Skip to content

Conversation

@ok7sai
Copy link
Member

@ok7sai ok7sai commented Oct 23, 2025

Or do we want to create a different id generator only used by aria?

@ok7sai ok7sai requested a review from a team as a code owner October 23, 2025 23:47
}

return `${prefix}${counters[prefix]++}`;
return `${prefix}${randomInfix ? this._infix + '-' : ''}${counters[prefix]++}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: I'm wondering if we should add the random number as a suffix. I remember that back in the day IDs starting with a number were problematic and in theory the prefix here can be an empty string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the prefix is an empty string then starting with the counter is going to have the same problem. Maybe we can add a letter to the random number like a1234?

@ok7sai ok7sai force-pushed the ng-aria-id-generator branch 3 times, most recently from e4ce50e to f2ef0e9 Compare October 24, 2025 18:11
@ok7sai
Copy link
Member Author

ok7sai commented Oct 24, 2025

Updated API golden.

@ok7sai ok7sai requested a review from crisbeto October 24, 2025 18:12
@ok7sai ok7sai force-pushed the ng-aria-id-generator branch from f2ef0e9 to 6271738 Compare October 27, 2025 20:11
@ok7sai ok7sai force-pushed the ng-aria-id-generator branch from 6271738 to 6a9cc78 Compare October 28, 2025 21:48
@ok7sai ok7sai removed the request for review from wagnermaciel October 28, 2025 22:21
@ok7sai ok7sai added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Oct 28, 2025
@ok7sai ok7sai merged commit 314b182 into angular:main Oct 28, 2025
28 checks passed
@ok7sai ok7sai deleted the ng-aria-id-generator branch October 28, 2025 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants