Skip to content

Commit cd9c7da

Browse files
authored
fix(aria/menu): make MenuItem value optional (#33715)
1 parent 3cc2d3b commit cd9c7da

9 files changed

Lines changed: 180 additions & 34 deletions

File tree

goldens/aria/menu/index.api.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export class Menu<V> implements OnDestroy {
2121
readonly expansionDelay: _angular_core.InputSignal<number>;
2222
readonly id: _angular_core.InputSignal<string>;
2323
readonly _items: Signal<MenuItem<V>[]>;
24-
readonly itemSelected: _angular_core.OutputEmitterRef<V>;
24+
readonly itemSelected: _angular_core.OutputEmitterRef<V | undefined>;
2525
// (undocumented)
2626
ngOnDestroy(): void;
2727
readonly parent: _angular_core.WritableSignal<MenuItem<V> | MenuTrigger<V> | undefined>;
@@ -50,7 +50,7 @@ export class MenuBar<V> implements OnDestroy {
5050
readonly element: HTMLElement;
5151
// (undocumented)
5252
readonly _items: SignalLike<MenuItem<V>[]>;
53-
readonly itemSelected: _angular_core.OutputEmitterRef<V>;
53+
readonly itemSelected: _angular_core.OutputEmitterRef<V | undefined>;
5454
// (undocumented)
5555
ngOnDestroy(): void;
5656
readonly _pattern: MenuBarPattern<V>;
@@ -93,9 +93,9 @@ export class MenuItem<V> implements OnInit, OnDestroy {
9393
readonly role: _angular_core.InputSignal<"menuitem" | "menuitemcheckbox" | "menuitemradio">;
9494
readonly searchTerm: _angular_core.ModelSignal<string>;
9595
readonly submenu: _angular_core.InputSignal<Menu<V> | undefined>;
96-
readonly value: _angular_core.InputSignal<V>;
96+
readonly value: _angular_core.InputSignal<V | undefined>;
9797
// (undocumented)
98-
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<MenuItem<any>, "[ngMenuItem]", ["ngMenuItem"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; "value": { "alias": "value"; "required": true; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "searchTerm": { "alias": "searchTerm"; "required": false; "isSignal": true; }; "role": { "alias": "role"; "required": false; "isSignal": true; }; "submenu": { "alias": "submenu"; "required": false; "isSignal": true; }; }, { "searchTerm": "searchTermChange"; }, never, never, true, never>;
98+
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<MenuItem<any>, "[ngMenuItem]", ["ngMenuItem"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; "value": { "alias": "value"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "searchTerm": { "alias": "searchTerm"; "required": false; "isSignal": true; }; "role": { "alias": "role"; "required": false; "isSignal": true; }; "submenu": { "alias": "submenu"; "required": false; "isSignal": true; }; }, { "searchTerm": "searchTermChange"; }, never, never, true, never>;
9999
// (undocumented)
100100
static ɵfac: _angular_core.ɵɵFactoryDeclaration<MenuItem<any>, never>;
101101
}

goldens/aria/private/index.api.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,9 @@ export class ListboxPattern<V> {
345345
}
346346

347347
// @public
348-
export interface MenuBarInputs<V> extends ListInputs<MenuItemPattern<V>, V> {
348+
export interface MenuBarInputs<V> extends ListInputs<MenuItemPattern<V>, V | undefined> {
349349
items: SignalLike<MenuItemPattern<V>[]>;
350-
itemSelected?: (value: V) => void;
350+
itemSelected?: (value: V | undefined) => void;
351351
textDirection: SignalLike<'ltr' | 'rtl'>;
352352
}
353353

@@ -365,7 +365,7 @@ export class MenuBarPattern<V> {
365365
readonly inputs: MenuBarInputs<V>;
366366
readonly isFocused: WritableSignalLike<boolean>;
367367
readonly keydownManager: SignalLike<KeyboardEventManager<KeyboardEvent>>;
368-
readonly listBehavior: List<MenuItemPattern<V>, V>;
368+
readonly listBehavior: List<MenuItemPattern<V>, V | undefined>;
369369
next(): void;
370370
onClick(event: MouseEvent): void;
371371
onFocusIn(): void;
@@ -380,24 +380,25 @@ export class MenuBarPattern<V> {
380380
}
381381

382382
// @public
383-
export interface MenuInputs<V> extends Omit<ListInputs<MenuItemPattern<V>, V>, 'value'> {
383+
export interface MenuInputs<V> extends Omit<ListInputs<MenuItemPattern<V>, V | undefined>, 'value'> {
384384
expansionDelay: SignalLike<number>;
385385
id: SignalLike<string>;
386386
items: SignalLike<MenuItemPattern<V>[]>;
387-
itemSelected?: (value: V) => void;
387+
itemSelected?: (value: V | undefined) => void;
388388
parent: SignalLike<MenuTriggerPattern<V> | MenuItemPattern<V> | undefined>;
389389
textDirection: SignalLike<'ltr' | 'rtl'>;
390390
}
391391

392392
// @public
393-
export interface MenuItemInputs<V> extends Omit<ListItem<V>, 'index' | 'selectable'> {
393+
export interface MenuItemInputs<V> extends Omit<ListItem<V>, 'index' | 'selectable' | 'value'> {
394394
parent: SignalLike<MenuPattern<V> | MenuBarPattern<V> | undefined>;
395395
role: SignalLike<'menuitem' | 'menuitemradio' | 'menuitemcheckbox'>;
396396
submenu: SignalLike<MenuPattern<V> | undefined>;
397+
value?: SignalLike<V | undefined>;
397398
}
398399

399400
// @public
400-
export class MenuItemPattern<V> implements ListItem<V> {
401+
export class MenuItemPattern<V> implements ListItem<V | undefined> {
401402
constructor(inputs: MenuItemInputs<V>);
402403
readonly active: SignalLike<boolean>;
403404
close(opts?: {
@@ -424,7 +425,7 @@ export class MenuItemPattern<V> implements ListItem<V> {
424425
readonly selectable: SignalLike<boolean>;
425426
readonly submenu: SignalLike<MenuPattern<V> | undefined>;
426427
readonly tabIndex: SignalLike<-1 | 0>;
427-
readonly value: SignalLike<V>;
428+
readonly value: SignalLike<V | undefined>;
428429
}
429430

430431
// @public
@@ -449,7 +450,7 @@ export class MenuPattern<V> {
449450
readonly isFocused: WritableSignalLike<boolean>;
450451
readonly keydownManager: SignalLike<KeyboardEventManager<KeyboardEvent>>;
451452
last(): void;
452-
readonly listBehavior: List<MenuItemPattern<V>, V>;
453+
readonly listBehavior: List<MenuItemPattern<V>, V | undefined>;
453454
next(): void;
454455
onClick(event: MouseEvent): void;
455456
onFocusIn(): void;

src/aria/menu/menu-bar.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export class MenuBar<V> implements OnDestroy {
106106
private readonly _itemPatterns = computed(() => this._items().map(i => i._pattern));
107107

108108
/** A callback function triggered when a menu item is selected. */
109-
readonly itemSelected = output<V>();
109+
readonly itemSelected = output<V | undefined>();
110110

111111
constructor() {
112112
this._pattern = new MenuBarPattern({
@@ -116,7 +116,7 @@ export class MenuBar<V> implements OnDestroy {
116116
focusMode: () => 'roving',
117117
orientation: () => 'horizontal',
118118
selectionMode: () => 'explicit',
119-
itemSelected: (value: V) => this.itemSelected.emit(value),
119+
itemSelected: (value: V | undefined) => this.itemSelected.emit(value),
120120
activeItem: signal(undefined),
121121
element: computed(() => this._elementRef.nativeElement),
122122
});

src/aria/menu/menu-item.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export class MenuItem<V> implements OnInit, OnDestroy {
6565
readonly id = input(inject(_IdGenerator).getId('ng-menu-item-', true));
6666

6767
/** The value of the menu item. */
68-
readonly value = input.required<V>();
68+
readonly value = input<V | undefined>(undefined);
6969

7070
/** Whether the menu item is disabled. */
7171
readonly disabled = input<boolean>(false);

src/aria/menu/menu.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,34 @@ describe('Standalone Menu Pattern', () => {
579579
expect(consoleSpy).toHaveBeenCalledWith("Duplicate value 'item0' detected inside ngMenu.");
580580
});
581581

582+
it('should not warn when items inside ngMenu have no value', () => {
583+
TestBed.resetTestingModule();
584+
TestBed.configureTestingModule({
585+
imports: [MenuWithOptionalValues],
586+
});
587+
const optionalValuesFixture = TestBed.createComponent(MenuWithOptionalValues);
588+
optionalValuesFixture.detectChanges();
589+
590+
expect(consoleSpy).not.toHaveBeenCalled();
591+
});
592+
593+
it('should emit undefined on itemSelected when selecting an item without a value', async () => {
594+
TestBed.resetTestingModule();
595+
TestBed.configureTestingModule({
596+
imports: [MenuWithOptionalValues],
597+
});
598+
const optionalValuesFixture = TestBed.createComponent(MenuWithOptionalValues);
599+
optionalValuesFixture.detectChanges();
600+
const instance = optionalValuesFixture.componentInstance;
601+
spyOn(instance, 'itemSelected');
602+
603+
const items = optionalValuesFixture.debugElement.queryAll(By.directive(MenuItem));
604+
const firstItem = items[0].nativeElement as HTMLElement;
605+
firstItem.click();
606+
607+
expect(instance.itemSelected).toHaveBeenCalledWith(undefined);
608+
});
609+
582610
it('should warn when ngMenuItem is outside ngMenu or ngMenuBar', () => {
583611
TestBed.resetTestingModule();
584612
TestBed.configureTestingModule({
@@ -1372,6 +1400,20 @@ class ShuffledMenuBarExample {
13721400
})
13731401
class MenuWithDuplicateValues {}
13741402

1403+
@Component({
1404+
template: `
1405+
<div ngMenu (itemSelected)="itemSelected($event)">
1406+
<div ngMenuItem>Item 0</div>
1407+
<div ngMenuItem>Item 1</div>
1408+
</div>
1409+
`,
1410+
imports: [Menu, MenuItem],
1411+
changeDetection: ChangeDetectionStrategy.Eager,
1412+
})
1413+
class MenuWithOptionalValues {
1414+
itemSelected(value: string | undefined) {}
1415+
}
1416+
13751417
@Component({
13761418
template: `
13771419
<div ngMenuItem value="item0">Item 0</div>

src/aria/menu/menu.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export class Menu<V> implements OnDestroy {
144144
readonly tabIndex = computed(() => this._pattern.tabIndex());
145145

146146
/** A callback function triggered when a menu item is selected. */
147-
readonly itemSelected = output<V>();
147+
readonly itemSelected = output<V | undefined>();
148148

149149
/** The delay in milliseconds before expanding sub-menus on hover. */
150150
readonly expansionDelay = input<number>(100); // Arbitrarily chosen.
@@ -160,7 +160,7 @@ export class Menu<V> implements OnDestroy {
160160
selectionMode: () => 'explicit',
161161
activeItem: signal(undefined),
162162
element: computed(() => this._elementRef.nativeElement),
163-
itemSelected: (value: V) => this.itemSelected.emit(value),
163+
itemSelected: (value: V | undefined) => this.itemSelected.emit(value),
164164
});
165165

166166
afterRenderEffect({

src/aria/private/menu/menu.spec.ts

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {signal, WritableSignalLike} from '../behaviors/signal-like/signal-like';
9+
import {signal, SignalLike, WritableSignalLike} from '../behaviors/signal-like/signal-like';
1010
import {MenuPattern, MenuBarPattern, MenuItemPattern, MenuTriggerPattern} from './menu';
1111
import {createKeyboardEvent} from '@angular/cdk/testing/private';
1212
import {ModifierKeys} from '@angular/cdk/testing';
@@ -1011,4 +1011,98 @@ describe('Menu Bar Pattern', () => {
10111011
});
10121012
});
10131013
});
1014+
1015+
describe('MenuItemPattern with optional value', () => {
1016+
it('should default value to undefined when omitted', () => {
1017+
const item = new MenuItemPattern({
1018+
id: signal('item-1'),
1019+
disabled: signal(false),
1020+
searchTerm: signal('Item'),
1021+
parent: signal(undefined),
1022+
element: signal(document.createElement('div')),
1023+
submenu: signal(undefined),
1024+
role: signal('menuitem'),
1025+
});
1026+
1027+
expect(item.value()).toBeUndefined();
1028+
});
1029+
1030+
function createTestMenu(items: SignalLike<MenuItemPattern<any>[]>) {
1031+
return new MenuPattern({
1032+
id: signal('menu-1'),
1033+
items,
1034+
parent: signal(undefined),
1035+
textDirection: signal('ltr'),
1036+
expansionDelay: signal(100),
1037+
disabled: signal(false),
1038+
activeItem: signal(undefined),
1039+
typeaheadDelay: signal(500),
1040+
wrap: signal(true),
1041+
softDisabled: signal(true),
1042+
multi: signal(false),
1043+
focusMode: signal('activedescendant'),
1044+
orientation: signal('vertical'),
1045+
selectionMode: signal('explicit'),
1046+
element: signal(document.createElement('div')),
1047+
});
1048+
}
1049+
1050+
it('should not report duplicate violations for items without a value', () => {
1051+
const items = signal<MenuItemPattern<any>[]>([]);
1052+
const menu = createTestMenu(items);
1053+
1054+
items.set([
1055+
new MenuItemPattern({
1056+
id: signal('item-1'),
1057+
disabled: signal(false),
1058+
searchTerm: signal('Item 1'),
1059+
parent: signal(menu),
1060+
element: signal(document.createElement('div')),
1061+
submenu: signal(undefined),
1062+
role: signal('menuitem'),
1063+
}),
1064+
new MenuItemPattern({
1065+
id: signal('item-2'),
1066+
disabled: signal(false),
1067+
searchTerm: signal('Item 2'),
1068+
parent: signal(menu),
1069+
element: signal(document.createElement('div')),
1070+
submenu: signal(undefined),
1071+
role: signal('menuitem'),
1072+
}),
1073+
]);
1074+
1075+
expect(menu.validate()).toEqual([]);
1076+
});
1077+
1078+
it('should report duplicate violations for items with the same defined value', () => {
1079+
const items = signal<MenuItemPattern<any>[]>([]);
1080+
const menu = createTestMenu(items);
1081+
1082+
items.set([
1083+
new MenuItemPattern({
1084+
id: signal('item-1'),
1085+
value: signal('copy'),
1086+
disabled: signal(false),
1087+
searchTerm: signal('Copy'),
1088+
parent: signal(menu),
1089+
element: signal(document.createElement('div')),
1090+
submenu: signal(undefined),
1091+
role: signal('menuitem'),
1092+
}),
1093+
new MenuItemPattern({
1094+
id: signal('item-2'),
1095+
value: signal('copy'),
1096+
disabled: signal(false),
1097+
searchTerm: signal('Copy Again'),
1098+
parent: signal(menu),
1099+
element: signal(document.createElement('div')),
1100+
submenu: signal(undefined),
1101+
role: signal('menuitem'),
1102+
}),
1103+
]);
1104+
1105+
expect(menu.validate()).toEqual(["Duplicate value 'copy' detected inside ngMenu."]);
1106+
});
1107+
});
10141108
});

0 commit comments

Comments
 (0)