Skip to content

Commit fc07642

Browse files
committed
fix(cdk-experimental/ui-patterns): toolbar cleanup
1 parent da13f68 commit fc07642

File tree

5 files changed

+73
-47
lines changed

5 files changed

+73
-47
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ export class CdkRadioGroup<V> {
131131
value: this._value,
132132
activeItem: signal(undefined),
133133
textDirection: this.textDirection,
134-
toolbar: signal(undefined),
134+
toolbar: signal(undefined), // placeholder until Toolbar CDK is added
135135
});
136136

137137
/** Whether the radio group has received focus yet. */

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type ToolbarWidgetLike = {
4343
* Represents the properties exposed by a toolbar that need to be accessed by a radio group.
4444
* This exists to avoid circular dependency errors between the toolbar and radio button.
4545
*/
46-
interface ToolbarLike<V> {
46+
export interface ToolbarLike<V> {
4747
listBehavior: List<RadioButtonPattern<V> | ToolbarWidgetLike, V>;
4848
orientation: SignalLike<'vertical' | 'horizontal'>;
4949
disabled: SignalLike<boolean>;
@@ -66,7 +66,7 @@ export class RadioGroupPattern<V> {
6666
/** Whether the radio group is readonly. */
6767
readonly = computed(() => this.selectedItem()?.disabled() || this.inputs.readonly());
6868

69-
/** The tabindex of the radio group (if using activedescendant or if in toolbar). */
69+
/** The tabindex of the radio group. */
7070
tabindex = computed(() => (this.inputs.toolbar() ? -1 : this.listBehavior.tabindex()));
7171

7272
/** The id of the current active radio button (if using activedescendant). */

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {signal, WritableSignal} from '@angular/core';
10-
import {RadioGroupInputs, RadioGroupPattern} from './radio-group';
10+
import {RadioGroupInputs, RadioGroupPattern, ToolbarLike} from './radio-group';
1111
import {RadioButtonPattern} from './radio-button';
1212
import {createKeyboardEvent} from '@angular/cdk/testing/private';
1313
import {ModifierKeys} from '@angular/cdk/testing';
@@ -304,4 +304,49 @@ describe('RadioGroup Pattern', () => {
304304
expect(violations.length).toBe(1);
305305
});
306306
});
307+
308+
describe('toolbar', () => {
309+
let radioGroup: TestRadioGroup;
310+
let radioButtons: TestRadio[];
311+
let toolbar: ToolbarLike<string>;
312+
313+
beforeEach(() => {
314+
const patterns = getDefaultPatterns();
315+
radioGroup = patterns.radioGroup;
316+
radioButtons = patterns.radioButtons;
317+
toolbar = {
318+
listBehavior: radioGroup.listBehavior,
319+
orientation: radioGroup.orientation,
320+
disabled: radioGroup.disabled,
321+
};
322+
radioGroup.inputs.toolbar = signal(toolbar);
323+
});
324+
325+
it('should ignore keyboard navigation when within a toolbar', () => {
326+
const initialActive = radioGroup.inputs.activeItem();
327+
radioGroup.onKeydown(down());
328+
expect(radioGroup.inputs.activeItem()).toBe(initialActive);
329+
});
330+
331+
it('should ignore keyboard selection when within a toolbar', () => {
332+
expect(radioGroup.inputs.value()).toEqual([]);
333+
radioGroup.onKeydown(space());
334+
expect(radioGroup.inputs.value()).toEqual([]);
335+
radioGroup.onKeydown(enter());
336+
expect(radioGroup.inputs.value()).toEqual([]);
337+
});
338+
339+
it('should ignore pointer events when within a toolbar', () => {
340+
const initialActive = radioGroup.inputs.activeItem();
341+
expect(radioGroup.inputs.value()).toEqual([]);
342+
343+
const clickEvent = {
344+
target: radioButtons[1].element(),
345+
} as unknown as PointerEvent;
346+
radioGroup.onPointerdown(clickEvent);
347+
348+
expect(radioGroup.inputs.activeItem()).toBe(initialActive);
349+
expect(radioGroup.inputs.value()).toEqual([]);
350+
});
351+
});
307352
});

src/cdk-experimental/ui-patterns/toolbar/BUILD.bazel

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@ ts_project(
1010
deps = [
1111
"//:node_modules/@angular/core",
1212
"//src/cdk-experimental/ui-patterns/behaviors/event-manager",
13-
"//src/cdk-experimental/ui-patterns/behaviors/list-focus",
14-
"//src/cdk-experimental/ui-patterns/behaviors/list-navigation",
13+
"//src/cdk-experimental/ui-patterns/behaviors/list",
1514
"//src/cdk-experimental/ui-patterns/behaviors/signal-like",
1615
"//src/cdk-experimental/ui-patterns/radio-group",
17-
"//src/cdk/bidi",
1816
],
1917
)

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

Lines changed: 23 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -70,57 +70,40 @@ export class ToolbarPattern<V> {
7070
/** The keydown event manager for the toolbar. */
7171
keydown = computed(() => {
7272
const manager = new KeyboardEventManager();
73-
manager.options = {
74-
preventDefault: false,
75-
stopPropagation: true,
76-
};
73+
74+
const activeItem = this.inputs.activeItem();
75+
const isRadioButton = activeItem instanceof RadioButtonPattern;
76+
77+
if (isRadioButton) {
78+
manager
79+
.on(' ', () => this.selectRadioButton())
80+
.on('Enter', () => this.selectRadioButton())
81+
.on(this.altNextKey, () => activeItem?.group()?.listBehavior.next())
82+
.on(this.altPrevKey, () => activeItem?.group()?.listBehavior.prev());
83+
} else {
84+
manager.on(this.altNextKey, () => this.listBehavior.next());
85+
manager.on(this.altPrevKey, () => this.listBehavior.prev());
86+
}
7787

7888
return manager
79-
.on(' ', () => this.toolbarSelectOverride())
80-
.on('Enter', () => this.toolbarSelectOverride())
8189
.on(this.prevKey, () => this.listBehavior.prev())
8290
.on(this.nextKey, () => this.listBehavior.next())
83-
.on(this.altNextKey, () => {
84-
const activeItem = this.inputs.activeItem();
85-
/** When within a Radio Group, use its navigation */
86-
if (activeItem instanceof RadioButtonPattern && activeItem.group()) {
87-
activeItem.group()?.listBehavior.next();
88-
} else {
89-
this.listBehavior.next();
90-
}
91-
})
92-
.on(this.altPrevKey, () => {
93-
const activeItem = this.inputs.activeItem();
94-
/** When within a Radio Group, use its navigation */
95-
if (activeItem instanceof RadioButtonPattern && activeItem.group()) {
96-
activeItem.group()?.listBehavior.prev();
97-
} else {
98-
this.listBehavior.prev();
99-
}
100-
})
10191
.on('Home', () => this.listBehavior.first())
10292
.on('End', () => this.listBehavior.last());
10393
});
10494

105-
toolbarSelectOverride() {
106-
const activeItem = this.inputs.activeItem();
95+
selectRadioButton() {
96+
const activeItem = this.inputs.activeItem() as RadioButtonPattern<V>;
10797

108-
/** If the active item is a Radio Button, indicate to the group the selection */
109-
if (activeItem instanceof RadioButtonPattern) {
110-
const group = activeItem.group();
111-
if (group && !group.readonly() && !group.disabled()) {
112-
group.listBehavior.selectOne();
113-
}
98+
// activeItem must be a radio button
99+
const group = activeItem!.group();
100+
if (group && !group.readonly() && !group.disabled()) {
101+
group.listBehavior.selectOne();
114102
}
115103
}
116104

117105
/** The pointerdown event manager for the toolbar. */
118-
pointerdown = computed(() => {
119-
const manager = new PointerEventManager();
120-
121-
// Default behavior: navigate and select on click.
122-
return manager.on(e => this.goto(e));
123-
});
106+
pointerdown = computed(() => new PointerEventManager().on(e => this.goto(e)));
124107

125108
/** Navigates to the widget associated with the given pointer event. */
126109
goto(event: PointerEvent) {
@@ -129,8 +112,8 @@ export class ToolbarPattern<V> {
129112

130113
if (item instanceof RadioButtonPattern) {
131114
const group = item.group();
132-
if (group && !group.readonly() && !group.disabled()) {
133-
group.listBehavior.goto(item, {selectOne: true});
115+
if (group && !group.disabled()) {
116+
group.listBehavior.goto(item, {selectOne: !group.readonly()});
134117
}
135118
} else {
136119
this.listBehavior.goto(item);

0 commit comments

Comments
 (0)