Skip to content

Commit 6e94aba

Browse files
committed
fix(cdk-experimental/ui-patterns): fixing toolbar interactions when disabled
1 parent 59d6299 commit 6e94aba

File tree

5 files changed

+77
-71
lines changed

5 files changed

+77
-71
lines changed

src/cdk-experimental/radio-group/radio-group.ts

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import {
1919
model,
2020
signal,
2121
WritableSignal,
22-
OnInit,
2322
OnDestroy,
2423
} from '@angular/core';
2524
import {RadioButtonPattern, RadioGroupPattern} from '../ui-patterns';
@@ -162,21 +161,27 @@ export class CdkRadioGroup<V> {
162161
this.pattern.setDefaultState();
163162
}
164163
});
164+
165+
afterRenderEffect(() => {
166+
if (this.toolbar) {
167+
const radioButtons = this._cdkRadioButtons();
168+
// If the group is disabled and the toolbar is set to skip disabled items,
169+
// the radio buttons should not be part of the toolbar's navigation.
170+
if (this.disabled() && this.toolbar.skipDisabled()) {
171+
radioButtons.forEach(radio => this.toolbar!.deregister(radio));
172+
} else {
173+
radioButtons.forEach(radio => this.toolbar!.register(radio));
174+
}
175+
}
176+
});
165177
}
166178

167179
onFocus() {
168180
this._hasFocused.set(true);
169181
}
170182

171-
toolbarButtonRegister(radio: CdkRadioButton<V>) {
172-
// only register if the group is not disabled or the toolbar does not skip disabled
173-
if (this.toolbar && (!this.disabled() || !this.toolbar.skipDisabled())) {
174-
this.toolbar.register(radio);
175-
}
176-
}
177-
178183
toolbarButtonDeregister(radio: CdkRadioButton<V>) {
179-
if (this.toolbar && (!this.disabled() || !this.toolbar.skipDisabled())) {
184+
if (this.toolbar) {
180185
this.toolbar.deregister(radio);
181186
}
182187
}
@@ -196,7 +201,7 @@ export class CdkRadioGroup<V> {
196201
'[id]': 'pattern.id()',
197202
},
198203
})
199-
export class CdkRadioButton<V> implements OnInit, OnDestroy {
204+
export class CdkRadioButton<V> implements OnDestroy {
200205
/** A reference to the radio button element. */
201206
private readonly _elementRef = inject(ElementRef);
202207

@@ -230,12 +235,6 @@ export class CdkRadioButton<V> implements OnInit, OnDestroy {
230235
element: this.element,
231236
});
232237

233-
ngOnInit() {
234-
if (this._cdkRadioGroup.toolbar) {
235-
this._cdkRadioGroup.toolbarButtonRegister(this);
236-
}
237-
}
238-
239238
ngOnDestroy() {
240239
if (this._cdkRadioGroup.toolbar) {
241240
this._cdkRadioGroup.toolbarButtonDeregister(this);

src/cdk-experimental/toolbar/toolbar.ts

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export class CdkToolbar<V> {
8888

8989
/** Sorted UIPatterns of the child widgets */
9090
items = computed(() =>
91-
[...this._cdkWidgets()].sort(sortDirectives).map(button => button.pattern),
91+
[...this._cdkWidgets()].sort(sortDirectives).map(widget => widget.pattern),
9292
);
9393

9494
/** Whether the toolbar is vertically or horizontally oriented. */
@@ -110,7 +110,6 @@ export class CdkToolbar<V> {
110110
pattern: ToolbarPattern<V> = new ToolbarPattern<V>({
111111
...this,
112112
activeIndex: signal(0),
113-
// items: this.items1, // might not need?
114113
textDirection: this.textDirection,
115114
focusMode: this.focusMode,
116115
});
@@ -128,18 +127,30 @@ export class CdkToolbar<V> {
128127
this.pattern.setDefaultState();
129128
}
130129
});
130+
131+
afterRenderEffect(() => {
132+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
133+
const violations = this.pattern.validate();
134+
for (const violation of violations) {
135+
console.error(violation);
136+
}
137+
}
138+
});
131139
}
132140

133141
register(widget: CdkRadioButtonInterface<V> | CdkToolbarWidget) {
134-
this._cdkWidgets().add(widget);
135-
136-
this._cdkWidgets.set(new Set(this._cdkWidgets()));
142+
const widgets = this._cdkWidgets();
143+
if (!widgets.has(widget)) {
144+
widgets.add(widget);
145+
this._cdkWidgets.set(new Set(widgets));
146+
}
137147
}
138148

139149
deregister(widget: CdkRadioButtonInterface<V> | CdkToolbarWidget) {
140-
this._cdkWidgets().delete(widget);
141-
142-
this._cdkWidgets.set(new Set(this._cdkWidgets()));
150+
const widgets = this._cdkWidgets();
151+
if (widgets.delete(widget)) {
152+
this._cdkWidgets.set(new Set(widgets));
153+
}
143154
}
144155
}
145156

@@ -157,37 +168,40 @@ export class CdkToolbar<V> {
157168
'class': 'cdk-toolbar-widget',
158169
'[class.cdk-active]': 'pattern.active()',
159170
'[attr.tabindex]': 'pattern.tabindex()',
160-
'[attr.aria-disabled]': 'pattern.disabled()',
171+
'[attr.inert]': 'hardDisabled() ? true : null',
172+
'[attr.disabled]': 'hardDisabled() ? true : null',
161173
'[id]': 'pattern.id()',
162174
},
163175
})
164176
export class CdkToolbarWidget implements OnInit, OnDestroy {
165-
/** A reference to the radio button element. */
177+
/** A reference to the widget element. */
166178
private readonly _elementRef = inject(ElementRef);
167179

168180
/** The parent CdkToolbar. */
169181
private readonly _cdkToolbar = inject(CdkToolbar);
170182

171-
/** A unique identifier for the radio button. */
183+
/** A unique identifier for the widget. */
172184
private readonly _generatedId = inject(_IdGenerator).getId('cdk-toolbar-widget-');
173185

174-
/** A unique identifier for the radio button. */
186+
/** A unique identifier for the widget. */
175187
protected id = computed(() => this._generatedId);
176188

177189
/** The parent Toolbar UIPattern. */
178190
protected parentToolbar = computed(() => this._cdkToolbar.pattern);
179191

180-
/** A reference to the radio button element to be focused on navigation. */
192+
/** A reference to the widget element to be focused on navigation. */
181193
element = computed(() => this._elementRef.nativeElement);
182194

183-
/** Whether the radio button is disabled. */
195+
/** Whether the widget is disabled. */
184196
disabled = input(false, {transform: booleanAttribute});
185197

198+
readonly hardDisabled = computed(() => this.pattern.disabled() && this.pattern.tabindex() < 0);
199+
186200
pattern = new ToolbarWidgetPattern({
187201
...this,
188202
id: this.id,
189203
element: this.element,
190-
disabled: computed(() => this._cdkToolbar.disabled() || this.disabled()),
204+
disabled: computed(() => this._cdkToolbar.disabled() || this.disabled() || false),
191205
parentToolbar: this.parentToolbar,
192206
});
193207

src/cdk-experimental/ui-patterns/radio-group/radio-button.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ interface RadioGroupLike<V> {
2626
focusManager: ListFocus<RadioButtonPattern<V> | GeneralWidget>;
2727
selection: ListSelection<RadioButtonPattern<V>, V>;
2828
navigation: ListNavigation<RadioButtonPattern<V> | GeneralWidget>;
29+
readonly: SignalLike<boolean>;
2930
}
3031

3132
/** Represents the required inputs for a radio button in a radio group. */

src/cdk-experimental/ui-patterns/radio-group/radio-group.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,7 @@ export class RadioGroupPattern<V> {
9999

100100
// If within a toolbar relinquish keyboard control
101101
if (this.inputs.toolbar()) {
102-
// when in activedescendant focus mode, allow toolbar to indicate the radio group to do selection
103-
if (this.readonly() || this.inputs.focusMode() === 'activedescendant') return manager;
104-
return manager
105-
.on(' ', () => this.selection.selectOne())
106-
.on('Enter', () => this.selection.selectOne());
102+
return manager;
107103
}
108104

109105
// Readonly mode allows navigation but not selection changes.

src/cdk-experimental/ui-patterns/toolbar/toolbar.ts

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -65,28 +65,9 @@ export class ToolbarPattern<V> {
6565
keydown = computed(() => {
6666
const manager = new KeyboardEventManager();
6767

68-
/** When a toolbar widget is active and disabled, prevent selection */
69-
if (
70-
this.focusManager.activeItem().disabled() &&
71-
this.focusManager.activeItem() instanceof ToolbarWidgetPattern
72-
) {
73-
return manager
74-
.on(' ', () => {})
75-
.on('Enter', () => {})
76-
.on(this.prevKey, () => this.navigation.prev())
77-
.on(this.nextKey, () => this.navigation.next())
78-
.on('Home', () => this.navigation.first())
79-
.on('End', () => this.navigation.last());
80-
}
81-
82-
/** When in Active Descendant mode, Toolbar controls which group receives selection */
83-
if (this.inputs.focusMode() === 'activedescendant') {
84-
manager
85-
.on(' ', () => this.toolbarSelectOverride())
86-
.on('Enter', () => this.toolbarSelectOverride());
87-
}
88-
8968
return manager
69+
.on(' ', () => this.toolbarSelectOverride())
70+
.on('Enter', () => this.toolbarSelectOverride())
9071
.on(this.prevKey, () => this.navigation.prev())
9172
.on(this.nextKey, () => this.navigation.next())
9273
.on('Home', () => this.navigation.first())
@@ -98,12 +79,13 @@ export class ToolbarPattern<V> {
9879

9980
/** If the active item is a Radio Button, indicate to the group the selection */
10081
if (activeItem instanceof RadioButtonPattern) {
101-
if (activeItem.group()) {
102-
activeItem.group()!!.selection.selectOne();
82+
const group = activeItem.group();
83+
if (group && !group.readonly()) {
84+
group.selection.selectOne();
10385
}
10486
} else {
10587
/** Item is a Toolbar Widget, manually select it */
106-
activeItem.element().click();
88+
if (activeItem.element()) activeItem.element().click();
10789
}
10890
}
10991

@@ -115,7 +97,7 @@ export class ToolbarPattern<V> {
11597
return manager.on(e => this.goto(e));
11698
});
11799

118-
/** Navigates to the radio button associated with the given pointer event. */
100+
/** Navigates to the widget associated with the given pointer event. */
119101
goto(event: PointerEvent) {
120102
const item = this._getItem(event);
121103

@@ -142,7 +124,7 @@ export class ToolbarPattern<V> {
142124
return undefined;
143125
}
144126

145-
// Assumes the target or its ancestor has role="radio" or button
127+
// Assumes the target or its ancestor has role="radio" or role="button"
146128
const element = e.target.closest('[role="button"], [role="radio"]');
147129
return this.inputs.items().find(i => i.element() === element);
148130
}
@@ -182,6 +164,21 @@ export class ToolbarPattern<V> {
182164
this.inputs.activeIndex.set(firstItem.index());
183165
}
184166
}
167+
/** Validates the state of the toolbar and returns a list of accessibility violations. */
168+
validate(): string[] {
169+
const violations: string[] = [];
170+
171+
if (this.inputs.skipDisabled()) {
172+
for (const item of this.inputs.items()) {
173+
if (item instanceof RadioButtonPattern && item.selected() && item.disabled()) {
174+
violations.push(
175+
"Accessibility Violation: A selected radio button inside the toolbar is disabled while 'skipDisabled' is true, making the selection unreachable via keyboard.",
176+
);
177+
}
178+
}
179+
}
180+
return violations;
181+
}
185182
}
186183

187184
export type ToolbarWidget = {
@@ -190,14 +187,14 @@ export type ToolbarWidget = {
190187
disabled: SignalLike<boolean>;
191188
};
192189

193-
/** Represents the required inputs for a radio button in a radio group. */
190+
/** Represents the required inputs for a toolbar widget in a toolbar. */
194191
export interface ToolbarWidgetInputs extends ListNavigationItem, ListFocusItem {
195-
/** A reference to the parent radio group. */
196-
parentToolbar: SignalLike<ToolbarPattern<null> | undefined>;
192+
/** A reference to the parent toolbar. */
193+
parentToolbar: SignalLike<ToolbarPattern<null>>;
197194
}
198195

199196
export class ToolbarWidgetPattern {
200-
/** A unique identifier for the radio button. */
197+
/** A unique identifier for the widget. */
201198
id: SignalLike<string>;
202199
/** The html element that should receive focus. */ // might not be needed
203200
readonly element: SignalLike<HTMLElement>;
@@ -207,9 +204,8 @@ export class ToolbarWidgetPattern {
207204
/** A reference to the parent toolbar. */
208205
parentToolbar: SignalLike<ToolbarPattern<null> | undefined>;
209206

210-
// expose tab index have tabindex -1 if roving manage 1/0
211-
/** The tabindex of the radio button. */
212-
tabindex = computed(() => this.parentToolbar()?.focusManager.getItemTabindex(this));
207+
/** The tabindex of the widgdet. */
208+
tabindex = computed(() => this.inputs.parentToolbar().focusManager.getItemTabindex(this));
213209

214210
/** The position of the widget within the group. */
215211
index = computed(
@@ -219,8 +215,8 @@ export class ToolbarWidgetPattern {
219215
.findIndex(i => i.id() === this.id()) ?? -1,
220216
);
221217

222-
/** Whether the radio button is currently the active one (focused). */
223-
active = computed(() => this.parentToolbar()?.focusManager.activeItem() === this);
218+
/** Whether the widhet is currently the active one (focused). */
219+
active = computed(() => this.inputs.parentToolbar().focusManager.activeItem() === this);
224220

225221
constructor(readonly inputs: ToolbarWidgetInputs) {
226222
this.id = inputs.id;

0 commit comments

Comments
 (0)