Skip to content

Commit

Permalink
fix(multiple): include APP_ID in element IDs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jelbourn committed Jun 18, 2024
1 parent 3a6490c commit 2327d53
Show file tree
Hide file tree
Showing 51 changed files with 449 additions and 353 deletions.
10 changes: 6 additions & 4 deletions src/cdk-experimental/combobox/combobox-popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, ElementRef, Inject, Input, OnInit} from '@angular/core';
import {IdGenerator} from '@angular/cdk/a11y';
import {Directive, ElementRef, inject, Inject, Input, OnInit} from '@angular/core';
import {AriaHasPopupValue, CDK_COMBOBOX, CdkCombobox} from './combobox';

let nextId = 0;

@Directive({
selector: '[cdkComboboxPopup]',
exportAs: 'cdkComboboxPopup',
Expand All @@ -24,6 +23,9 @@ let nextId = 0;
standalone: true,
})
export class CdkComboboxPopup<T = unknown> implements OnInit {
/** Generator for assigning unique IDs to DOM elements. */
private _idGenerator = inject(IdGenerator);

@Input()
get role(): AriaHasPopupValue {
return this._role;
Expand All @@ -42,7 +44,7 @@ export class CdkComboboxPopup<T = unknown> implements OnInit {
}
private _firstFocusElement: HTMLElement;

@Input() id = `cdk-combobox-popup-${nextId++}`;
@Input() id = this._idGenerator.getId('cdk-combobox-popup-');

constructor(
private readonly _elementRef: ElementRef<HTMLElement>,
Expand Down
1 change: 1 addition & 0 deletions src/cdk-experimental/table-scroll-container/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ng_module(
exclude = ["**/*.spec.ts"],
),
deps = [
"//src/cdk/a11y",
"//src/cdk/bidi",
"//src/cdk/platform",
"//src/cdk/table",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {CSP_NONCE, Directive, ElementRef, Inject, OnDestroy, OnInit, Optional} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {IdGenerator} from '@angular/cdk/a11y';
import {Directionality} from '@angular/cdk/bidi';
import {_getShadowRoot} from '@angular/cdk/platform';
import {
Expand All @@ -16,8 +15,17 @@ import {
StickySize,
StickyUpdate,
} from '@angular/cdk/table';

let nextId = 0;
import {DOCUMENT} from '@angular/common';
import {
CSP_NONCE,
Directive,
ElementRef,
inject,
Inject,
OnDestroy,
OnInit,
Optional,
} from '@angular/core';

/**
* Applies styles to the host element that make its scrollbars match up with
Expand All @@ -39,6 +47,9 @@ let nextId = 0;
standalone: true,
})
export class CdkTableScrollContainer implements StickyPositioningListener, OnDestroy, OnInit {
/** Generator for assigning unique IDs to DOM elements. */
private _idGenerator = inject(IdGenerator);

private readonly _uniqueClassName: string;
private _styleRoot!: Node;
private _styleElement?: HTMLStyleElement;
Expand All @@ -55,7 +66,7 @@ export class CdkTableScrollContainer implements StickyPositioningListener, OnDes
@Optional() private readonly _directionality?: Directionality,
@Optional() @Inject(CSP_NONCE) private readonly _nonce?: string | null,
) {
this._uniqueClassName = `cdk-table-scroll-container-${++nextId}`;
this._uniqueClassName = this._idGenerator.getId('cdk-table-scroll-container-');
_elementRef.nativeElement.classList.add(this._uniqueClassName);
}

Expand Down
13 changes: 8 additions & 5 deletions src/cdk/a11y/live-announcer/live-announcer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {DOCUMENT} from '@angular/common';
import {
Directive,
ElementRef,
inject,
Inject,
Injectable,
Input,
Expand All @@ -19,17 +20,19 @@ import {
Optional,
} from '@angular/core';
import {Subscription} from 'rxjs';
import {IdGenerator} from '../id-generator/id-generator';
import {
AriaLivePoliteness,
LiveAnnouncerDefaultOptions,
LIVE_ANNOUNCER_ELEMENT_TOKEN,
LIVE_ANNOUNCER_DEFAULT_OPTIONS,
LIVE_ANNOUNCER_ELEMENT_TOKEN,
LiveAnnouncerDefaultOptions,
} from './live-announcer-tokens';

let uniqueIds = 0;

@Injectable({providedIn: 'root'})
export class LiveAnnouncer implements OnDestroy {
/** Generator for assigning unique IDs to DOM elements. */
private _idGenerator = inject(IdGenerator);

private _liveElement: HTMLElement;
private _document: Document;
private _previousTimeout: number;
Expand Down Expand Up @@ -179,7 +182,7 @@ export class LiveAnnouncer implements OnDestroy {

liveEl.setAttribute('aria-atomic', 'true');
liveEl.setAttribute('aria-live', 'polite');
liveEl.id = `cdk-live-announcer-${uniqueIds++}`;
liveEl.id = this._idGenerator.getId('cdk-live-announcer-');

this._document.body.appendChild(liveEl);

Expand Down
1 change: 1 addition & 0 deletions src/cdk/accordion/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ng_module(
exclude = ["**/*.spec.ts"],
),
deps = [
"//src/cdk/a11y",
"//src/cdk/collections",
"@npm//@angular/core",
"@npm//rxjs",
Expand Down
21 changes: 11 additions & 10 deletions src/cdk/accordion/accordion-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,23 @@
* found in the LICENSE file at https://angular.io/license
*/

import {IdGenerator} from '@angular/cdk/a11y';
import {UniqueSelectionDispatcher} from '@angular/cdk/collections';
import {
Output,
booleanAttribute,
ChangeDetectorRef,
Directive,
EventEmitter,
Inject,
inject,
Input,
OnDestroy,
Optional,
ChangeDetectorRef,
Output,
SkipSelf,
Inject,
booleanAttribute,
} from '@angular/core';
import {UniqueSelectionDispatcher} from '@angular/cdk/collections';
import {CDK_ACCORDION, CdkAccordion} from './accordion';
import {Subscription} from 'rxjs';

/** Used to generate unique ID for each accordion item. */
let nextId = 0;
import {CDK_ACCORDION, CdkAccordion} from './accordion';

/**
* An basic directive expected to be extended and decorated as a component. Sets up all
Expand All @@ -40,6 +39,8 @@ let nextId = 0;
standalone: true,
})
export class CdkAccordionItem implements OnDestroy {
protected _idGenerator = inject(IdGenerator);

/** Subscription to openAll/closeAll events. */
private _openCloseAllSubscription = Subscription.EMPTY;
/** Event emitted every time the AccordionItem is closed. */
Expand All @@ -57,7 +58,7 @@ export class CdkAccordionItem implements OnDestroy {
@Output() readonly expandedChange: EventEmitter<boolean> = new EventEmitter<boolean>();

/** The unique AccordionItem id. */
readonly id: string = `cdk-accordion-child-${nextId++}`;
readonly id: string = this._idGenerator.getId('cdk-accordion-child-');

/** Whether the AccordionItem is expanded. */
@Input({transform: booleanAttribute})
Expand Down
12 changes: 7 additions & 5 deletions src/cdk/accordion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,19 @@
* found in the LICENSE file at https://angular.io/license
*/

import {IdGenerator} from '@angular/cdk/a11y';
import {
booleanAttribute,
Directive,
inject,
InjectionToken,
Input,
OnChanges,
OnDestroy,
SimpleChanges,
booleanAttribute,
} from '@angular/core';
import {Subject} from 'rxjs';

/** Used to generate unique ID for each accordion. */
let nextId = 0;

/**
* Injection token that can be used to reference instances of `CdkAccordion`. It serves
* as alternative token to the actual `CdkAccordion` class which could cause unnecessary
Expand All @@ -37,14 +36,17 @@ export const CDK_ACCORDION = new InjectionToken<CdkAccordion>('CdkAccordion');
standalone: true,
})
export class CdkAccordion implements OnDestroy, OnChanges {
/** Generator for assigning unique IDs to DOM elements. */
private _idGenerator = inject(IdGenerator);

/** Emits when the state of the accordion changes */
readonly _stateChanges = new Subject<SimpleChanges>();

/** Stream that emits true/false when openAll/closeAll is triggered. */
readonly _openCloseAllActions: Subject<boolean> = new Subject<boolean>();

/** A readonly id value to use for unique selection coordination. */
readonly id: string = `cdk-accordion-${nextId++}`;
readonly id: string = this._idGenerator.getId('cdk-accordion-');

/** Whether the accordion should allow multiple expanded accordion items simultaneously. */
@Input({transform: booleanAttribute}) multi: boolean = false;
Expand Down
48 changes: 25 additions & 23 deletions src/cdk/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,41 +6,43 @@
* found in the LICENSE file at https://angular.io/license
*/

import {
TemplateRef,
Injectable,
Injector,
OnDestroy,
Type,
StaticProvider,
Inject,
Optional,
SkipSelf,
ComponentRef,
} from '@angular/core';
import {BasePortalOutlet, ComponentPortal, TemplatePortal} from '@angular/cdk/portal';
import {of as observableOf, Observable, Subject, defer} from 'rxjs';
import {DialogRef} from './dialog-ref';
import {DialogConfig} from './dialog-config';
import {IdGenerator} from '@angular/cdk/a11y';
import {Directionality} from '@angular/cdk/bidi';
import {
ComponentType,
Overlay,
OverlayRef,
OverlayConfig,
ScrollStrategy,
OverlayContainer,
OverlayRef,
ScrollStrategy,
} from '@angular/cdk/overlay';
import {BasePortalOutlet, ComponentPortal, TemplatePortal} from '@angular/cdk/portal';
import {
ComponentRef,
Inject,
inject,
Injectable,
Injector,
OnDestroy,
Optional,
SkipSelf,
StaticProvider,
TemplateRef,
Type,
} from '@angular/core';
import {defer, Observable, of as observableOf, Subject} from 'rxjs';
import {startWith} from 'rxjs/operators';

import {DEFAULT_DIALOG_CONFIG, DIALOG_DATA, DIALOG_SCROLL_STRATEGY} from './dialog-injectors';
import {DialogConfig} from './dialog-config';
import {CdkDialogContainer} from './dialog-container';

/** Unique id for the created dialog. */
let uniqueId = 0;
import {DEFAULT_DIALOG_CONFIG, DIALOG_DATA, DIALOG_SCROLL_STRATEGY} from './dialog-injectors';
import {DialogRef} from './dialog-ref';

@Injectable({providedIn: 'root'})
export class Dialog implements OnDestroy {
/** Generator for assigning unique IDs to DOM elements. */
private _idGenerator = inject(IdGenerator);

private _openDialogsAtThisLevel: DialogRef<any, any>[] = [];
private readonly _afterAllClosedAtThisLevel = new Subject<void>();
private readonly _afterOpenedAtThisLevel = new Subject<DialogRef>();
Expand Down Expand Up @@ -114,7 +116,7 @@ export class Dialog implements OnDestroy {
DialogRef<R, C>
>;
config = {...defaults, ...config};
config.id = config.id || `cdk-dialog-${uniqueId++}`;
config.id = config.id || this._idGenerator.getId('cdk-dialog-');

if (
config.id &&
Expand Down
40 changes: 21 additions & 19 deletions src/cdk/drag-drop/directives/drop-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,34 @@
* found in the LICENSE file at https://angular.io/license
*/

import {NumberInput, coerceArray, coerceNumberProperty} from '@angular/cdk/coercion';
import {IdGenerator} from '@angular/cdk/a11y';
import {Directionality} from '@angular/cdk/bidi';
import {coerceArray, coerceNumberProperty, NumberInput} from '@angular/cdk/coercion';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {
booleanAttribute,
ChangeDetectorRef,
Directive,
ElementRef,
EventEmitter,
Inject,
inject,
Input,
OnDestroy,
Output,
Optional,
Directive,
ChangeDetectorRef,
Output,
SkipSelf,
Inject,
booleanAttribute,
} from '@angular/core';
import {Directionality} from '@angular/cdk/bidi';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {CDK_DROP_LIST, CdkDrag} from './drag';
import {CdkDragDrop, CdkDragEnter, CdkDragExit, CdkDragSortEvent} from '../drag-events';
import {CDK_DROP_LIST_GROUP, CdkDropListGroup} from './drop-list-group';
import {DropListRef} from '../drop-list-ref';
import {DragRef} from '../drag-ref';
import {DragDrop} from '../drag-drop';
import {DropListOrientation, DragAxis, DragDropConfig, CDK_DRAG_CONFIG} from './config';
import {merge, Subject} from 'rxjs';
import {startWith, takeUntil} from 'rxjs/operators';
import {DragDrop} from '../drag-drop';
import {CdkDragDrop, CdkDragEnter, CdkDragExit, CdkDragSortEvent} from '../drag-events';
import {DragRef} from '../drag-ref';
import {DropListRef} from '../drop-list-ref';
import {assertElementNode} from './assertions';

/** Counter used to generate unique ids for drop zones. */
let _uniqueIdCounter = 0;
import {CDK_DRAG_CONFIG, DragAxis, DragDropConfig, DropListOrientation} from './config';
import {CDK_DROP_LIST, CdkDrag} from './drag';
import {CDK_DROP_LIST_GROUP, CdkDropListGroup} from './drop-list-group';

/** Container that wraps a set of draggable items. */
@Directive({
Expand All @@ -55,6 +54,9 @@ let _uniqueIdCounter = 0;
},
})
export class CdkDropList<T = any> implements OnDestroy {
/** Generator for assigning unique IDs to DOM elements. */
private _idGenerator = inject(IdGenerator);

/** Emits when the list has been destroyed. */
private readonly _destroyed = new Subject<void>();

Expand Down Expand Up @@ -85,7 +87,7 @@ export class CdkDropList<T = any> implements OnDestroy {
* Unique ID for the drop zone. Can be used as a reference
* in the `connectedTo` of another `CdkDropList`.
*/
@Input() id: string = `cdk-drop-list-${_uniqueIdCounter++}`;
@Input() id: string = this._idGenerator.getId('cdk-drop-list-');

/** Locks the position of the draggable elements inside the container along the specified axis. */
@Input('cdkDropListLockAxis') lockAxis: DragAxis;
Expand Down
4 changes: 2 additions & 2 deletions src/cdk/listbox/listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ describe('CdkOption and CdkListbox', () => {
expect(optionIds.size).toBe(options.length);
for (let i = 0; i < options.length; i++) {
expect(options[i].id).toBe(optionEls[i].id);
expect(options[i].id).toMatch(/cdk-option-\d+/);
expect(options[i].id).toMatch(/cdk-option-\w+/);
}
expect(listbox.id).toEqual(listboxEl.id);
expect(listbox.id).toMatch(/cdk-listbox-\d+/);
expect(listbox.id).toMatch(/cdk-listbox-\w+/);
});

it('should not overwrite user given ids', () => {
Expand Down
Loading

0 comments on commit 2327d53

Please sign in to comment.