Skip to content

Conversation

jelbourn
Copy link
Member

Many components generate element IDs, which are used primarily for accessible labeling. The IDs all use an incrementing number concatenated with some stirng specific to that component. This change add Angular's APP_ID into these IDs in order to avoid ID collisions in cases where there are multiple instances of Angular running on the same page.

@jelbourn jelbourn added the target: patch This PR is targeted for the next patch release label Jun 14, 2024
@jelbourn jelbourn requested a review from a team as a code owner June 14, 2024 20:43
@jelbourn jelbourn requested review from amysorto and andrewseguin and removed request for a team June 14, 2024 20:43
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

Should we have a common helper for this so in the future we only have one place we'd need to change?

@jelbourn
Copy link
Member Author

Yeah, I could see having something like

let nextId = 0;

@Injectable()
class IdGenerator {
  appId = inject(APP_ID);
  generateId(prefix: string) {
    return `${prefix}${this.appId}${nextId++}`;
  }
}

I guess in cdk/a11y ? Not sure there's a more appropriate place for it

@crisbeto
Copy link
Member

Yeah I think cdk/a11y would be appropriate for it.

@jelbourn jelbourn added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 18, 2024
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jun 18, 2024
@jelbourn
Copy link
Member Author

Updated the PR to include an IdGenerator class (in its own commit) and then use that class in most of the ID generation. I intentionally left stepper and tab-group because they do something slightly different

@jelbourn jelbourn added the merge: preserve commits When the PR is merged, a rebase and merge should be performed label Jun 18, 2024
@jelbourn jelbourn force-pushed the app-id-ids branch 4 times, most recently from 2327d53 to 2bbe914 Compare June 18, 2024 22:26
Adds a tiny utility class for generating unique element IDs.
Many components generate element IDs, which are used primarily for
accessible labeling. The IDs all use an incrementing number concatenated
with some stirng specific to that component. This change add Angular's
`APP_ID` into these IDs in order to avoid ID collisions in cases where
there are multiple instances of Angular running on the same page.
@crisbeto
Copy link
Member

Closing in favor of #29948.

@crisbeto crisbeto closed this Nov 19, 2024
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

detected: feature PR contains a feature commit merge: preserve commits When the PR is merged, a rebase and merge should be performed target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants