Skip to content

Commit f42a500

Browse files
committed
fixup! refactor(cdk-experimental/ui-patterns): add toolbar widget group to decouple toolbar and radio group
1 parent 626a505 commit f42a500

File tree

10 files changed

+132
-80
lines changed

10 files changed

+132
-80
lines changed

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,12 @@ export function mapSignal<T, V>(
9696
'(pointerdown)': 'pattern.onPointerdown($event)',
9797
'(focusin)': 'onFocus()',
9898
},
99-
hostDirectives: [CdkToolbarWidgetGroup],
99+
hostDirectives: [
100+
{
101+
directive: CdkToolbarWidgetGroup,
102+
inputs: ['disabled'],
103+
},
104+
],
100105
})
101106
export class CdkRadioGroup<V> {
102107
/** A reference to the radio group element. */
@@ -118,22 +123,22 @@ export class CdkRadioGroup<V> {
118123
protected items = computed(() => this._cdkRadioButtons().map(radio => radio.pattern));
119124

120125
/** Whether the radio group is vertically or horizontally oriented. */
121-
orientation = input<'vertical' | 'horizontal'>('vertical');
126+
readonly orientation = input<'vertical' | 'horizontal'>('vertical');
122127

123128
/** Whether disabled items in the group should be skipped when navigating. */
124-
skipDisabled = input(true, {transform: booleanAttribute});
129+
readonly skipDisabled = input(true, {transform: booleanAttribute});
125130

126131
/** The focus strategy used by the radio group. */
127-
focusMode = input<'roving' | 'activedescendant'>('roving');
132+
readonly focusMode = input<'roving' | 'activedescendant'>('roving');
128133

129134
/** Whether the radio group is disabled. */
130-
disabled = input(false, {transform: booleanAttribute});
135+
readonly disabled = input(false, {transform: booleanAttribute});
131136

132137
/** Whether the radio group is readonly. */
133-
readonly = input(false, {transform: booleanAttribute});
138+
readonly readonly = input(false, {transform: booleanAttribute});
134139

135140
/** The value of the currently selected radio button. */
136-
value = model<V | null>(null);
141+
readonly value = model<V | null>(null);
137142

138143
/** The internal selection state for the radio group. */
139144
private readonly _value = mapSignal<V | null, V[]>(this.value, {
@@ -183,16 +188,10 @@ export class CdkRadioGroup<V> {
183188
});
184189

185190
afterRenderEffect(() => {
186-
if (!this._hasFocused() && !this._hasToolbar()) {
191+
if (!this._hasFocused() && this.pattern instanceof RadioGroupPattern) {
187192
this.pattern.setDefaultState();
188193
}
189194
});
190-
191-
afterRenderEffect(() => {
192-
if (this._hasToolbar()) {
193-
this._cdkToolbarWidgetGroup.disabled.set(this.disabled());
194-
}
195-
});
196195
}
197196

198197
onFocus() {

src/cdk-experimental/toolbar/toolbar.spec.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {afterRenderEffect, Component, inject, signal} from '@angular/core';
1+
import {Component, inject, signal} from '@angular/core';
22
import {ComponentFixture, TestBed} from '@angular/core/testing';
33
import {By} from '@angular/platform-browser';
44
import {Direction} from '@angular/cdk/bidi';
@@ -68,12 +68,15 @@ describe('CdkToolbar', () => {
6868
function updateToolbar(
6969
config: {
7070
disabled?: boolean;
71+
widgetGroupDisabled?: boolean;
7172
orientation?: 'horizontal' | 'vertical';
7273
wrap?: boolean;
7374
skipDisabled?: boolean;
7475
} = {},
7576
) {
7677
if (config.disabled !== undefined) testComponent.disabled.set(config.disabled);
78+
if (config.widgetGroupDisabled !== undefined)
79+
testComponent.widgetGroupDisabled.set(config.widgetGroupDisabled);
7780
if (config.orientation !== undefined) testComponent.orientation.set(config.orientation);
7881
if (config.wrap !== undefined) testComponent.wrap.set(config.wrap);
7982
if (config.skipDisabled !== undefined) testComponent.skipDisabled.set(config.skipDisabled);
@@ -145,7 +148,7 @@ describe('CdkToolbar', () => {
145148
describe('LTR', () => {
146149
beforeEach(() => {
147150
setupTestToolbar('ltr');
148-
testWidgetGroupInstance.disabled.set(true);
151+
updateToolbar({widgetGroupDisabled: true});
149152
});
150153

151154
describe('vertical orientation', () => {
@@ -233,8 +236,7 @@ describe('CdkToolbar', () => {
233236
describe('RTL', () => {
234237
beforeEach(() => {
235238
setupTestToolbar('rtl');
236-
testWidgetGroupInstance.disabled.set(true);
237-
updateToolbar({orientation: 'horizontal'});
239+
updateToolbar({widgetGroupDisabled: true, orientation: 'horizontal'});
238240
});
239241

240242
describe('horizontal orientation', () => {
@@ -282,7 +284,9 @@ describe('CdkToolbar', () => {
282284
describe('LTR', () => {
283285
beforeEach(() => {
284286
setupTestToolbar('ltr');
285-
click(testWidgetGroupInstance.cdkToolbarWidgetGroup.element());
287+
const widgetGroupElement = testWidgetGroupInstance.cdkToolbarWidgetGroup.element();
288+
click(widgetGroupElement);
289+
testWidgetGroupInstance.lastAction.set(undefined);
286290
});
287291

288292
describe('vertical orientation', () => {
@@ -380,7 +384,9 @@ describe('CdkToolbar', () => {
380384
describe('RTL', () => {
381385
beforeEach(() => {
382386
setupTestToolbar('rtl');
383-
click(testWidgetGroupInstance.cdkToolbarWidgetGroup.element());
387+
const widgetGroupElement = testWidgetGroupInstance.cdkToolbarWidgetGroup.element();
388+
click(widgetGroupElement);
389+
testWidgetGroupInstance.lastAction.set(undefined);
384390
updateToolbar({orientation: 'horizontal'});
385391
});
386392

@@ -400,27 +406,28 @@ describe('CdkToolbar', () => {
400406
});
401407

402408
@Component({
403-
template: `inner UI patterns`,
409+
template: 'a black box',
404410
selector: 'testWidgetGroup',
405-
hostDirectives: [CdkToolbarWidgetGroup],
411+
hostDirectives: [
412+
{
413+
directive: CdkToolbarWidgetGroup,
414+
inputs: ['disabled'],
415+
},
416+
],
406417
})
407418
class TestToolbarWidgetGroup {
408419
readonly cdkToolbarWidgetGroup = inject(CdkToolbarWidgetGroup);
409-
readonly disabled = signal(false);
410420
readonly lastAction = signal<string | undefined>(undefined);
411421

412422
constructor() {
413-
afterRenderEffect(() => {
414-
this.cdkToolbarWidgetGroup.disabled.set(this.disabled());
415-
});
416423
this.cdkToolbarWidgetGroup.controls.set({
424+
isOnFirstItem: () => false,
425+
isOnLastItem: () => false,
417426
next: wrap => {
418427
this.lastAction.set(wrap ? 'nextWithWrap' : 'next');
419-
return {leaveGroup: !wrap};
420428
},
421429
prev: wrap => {
422430
this.lastAction.set(wrap ? 'prevWithWrap' : 'prev');
423-
return {leaveGroup: !wrap};
424431
},
425432
first: () => {
426433
this.lastAction.set('first');
@@ -455,7 +462,7 @@ class TestToolbarWidgetGroup {
455462
>
456463
<button cdkToolbarWidget data-value="widget">Widget Button 1</button>
457464
<button cdkToolbarWidget data-value="widget" [disabled]="true">Widget Button 2</button>
458-
<testWidgetGroup></testWidgetGroup>
465+
<testWidgetGroup [disabled]="widgetGroupDisabled()"></testWidgetGroup>
459466
<button cdkToolbarWidget data-value="widget">Widget Button 3</button>
460467
</div>
461468
`,
@@ -464,6 +471,7 @@ class TestToolbarWidgetGroup {
464471
class TestToolbarComponent {
465472
orientation = signal<'vertical' | 'horizontal'>('horizontal');
466473
disabled = signal(false);
474+
widgetGroupDisabled = signal(false);
467475
wrap = signal(true);
468476
skipDisabled = signal(true);
469477
}

src/cdk-experimental/toolbar/toolbar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ export class CdkToolbarWidgetGroup<V> implements OnInit, OnDestroy {
250250
readonly element = computed(() => this._elementRef.nativeElement);
251251

252252
/** Whether the widget group is disabled. */
253-
readonly disabled = signal(false);
253+
readonly disabled = input(false, {transform: booleanAttribute});
254254

255255
/** The controls that can be performed on the widget group. */
256256
readonly controls = signal<ToolbarWidgetGroupControls | undefined>(undefined);

src/cdk-experimental/ui-patterns/behaviors/list-navigation/list-navigation.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ describe('List Navigation', () => {
4646
expect(nav.inputs.activeItem()).toBe(nav.inputs.items()[1]);
4747
});
4848

49+
it('should peek next item', () => {
50+
const nav = getNavigation();
51+
expect(nav.peekNext()).toBe(nav.inputs.items()[1]);
52+
expect(nav.inputs.activeItem()).toBe(nav.inputs.items()[0]);
53+
});
54+
4955
it('should wrap', () => {
5056
const nav = getNavigation({wrap: signal(true)});
5157
nav.next(); // 0 -> 1
@@ -124,6 +130,13 @@ describe('List Navigation', () => {
124130
expect(nav.inputs.activeItem()).toBe(nav.inputs.items()[1]);
125131
});
126132

133+
it('should peek previous item', () => {
134+
const nav = getNavigation();
135+
nav.goto(nav.inputs.items()[2]);
136+
expect(nav.peekPrev()).toBe(nav.inputs.items()[1]);
137+
expect(nav.inputs.activeItem()).toBe(nav.inputs.items()[2]);
138+
});
139+
127140
it('should wrap', () => {
128141
const nav = getNavigation({wrap: signal(true)});
129142
nav.prev(); // 0 -> 4

src/cdk-experimental/ui-patterns/behaviors/list-navigation/list-navigation.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,21 @@ export class ListNavigation<T extends ListNavigationItem> {
3838
return this._advance(1);
3939
}
4040

41+
/** Peeks the next item in the list. */
42+
peekNext(): T | undefined {
43+
return this._peek(1);
44+
}
45+
4146
/** Navigates to the previous item in the list. */
4247
prev(): boolean {
4348
return this._advance(-1);
4449
}
4550

51+
/** Peeks the previous item in the list. */
52+
peekPrev(): T | undefined {
53+
return this._peek(-1);
54+
}
55+
4656
/** Navigates to the first item in the list. */
4757
first(): boolean {
4858
const item = this.inputs.items().find(i => this.inputs.focusManager.isFocusable(i));
@@ -62,6 +72,12 @@ export class ListNavigation<T extends ListNavigationItem> {
6272

6373
/** Advances to the next or previous focusable item in the list based on the given delta. */
6474
private _advance(delta: 1 | -1): boolean {
75+
const item = this._peek(delta);
76+
return item ? this.goto(item) : false;
77+
}
78+
79+
/** Peeks the next or previous focusable item in the list based on the given delta. */
80+
private _peek(delta: 1 | -1): T | undefined {
6581
const items = this.inputs.items();
6682
const itemCount = items.length;
6783
const startIndex = this.inputs.focusManager.activeIndex();
@@ -73,10 +89,10 @@ export class ListNavigation<T extends ListNavigationItem> {
7389
// when the index goes out of bounds.
7490
for (let i = step(startIndex); i !== startIndex && i < itemCount && i >= 0; i = step(i)) {
7591
if (this.inputs.focusManager.isFocusable(items[i])) {
76-
return this.goto(items[i]);
92+
return items[i];
7793
}
7894
}
7995

80-
return false;
96+
return;
8197
}
8298
}

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,20 @@ describe('ToolbarRadioGroup Pattern', () => {
124124
radioGroup.inputs.activeItem.set(radioButtons[0]);
125125
});
126126

127+
it('should correctly report when on the first item', () => {
128+
radioGroup.inputs.activeItem.set(radioButtons[0]);
129+
expect(radioGroup.isOnFirstItem()).toBe(true);
130+
radioGroup.inputs.activeItem.set(radioButtons[1]);
131+
expect(radioGroup.isOnFirstItem()).toBe(false);
132+
});
133+
134+
it('should correctly report when on the last item', () => {
135+
radioGroup.inputs.activeItem.set(radioButtons[4]);
136+
expect(radioGroup.isOnLastItem()).toBe(true);
137+
radioGroup.inputs.activeItem.set(radioButtons[3]);
138+
expect(radioGroup.isOnLastItem()).toBe(false);
139+
});
140+
127141
it('should handle "next" control', () => {
128142
radioGroup.next(false);
129143
expect(radioGroup.inputs.activeItem()).toBe(radioButtons[1]);
@@ -184,19 +198,6 @@ describe('ToolbarRadioGroup Pattern', () => {
184198
expect(radioGroup.inputs.activeItem()).toBe(radioButtons[0]);
185199
});
186200

187-
it('should leave group on "next" at last item', () => {
188-
radioGroup.inputs.activeItem.set(radioButtons[4]);
189-
const result = radioGroup.next(false);
190-
expect(result?.leaveGroup).toBe(true);
191-
expect(radioGroup.inputs.activeItem()).toBe(undefined);
192-
});
193-
194-
it('should leave group on "prev" at first item', () => {
195-
const result = radioGroup.prev(false);
196-
expect(result?.leaveGroup).toBe(true);
197-
expect(radioGroup.inputs.activeItem()).toBe(undefined);
198-
});
199-
200201
it('should wrap on "next" with wrap', () => {
201202
radioGroup.inputs.activeItem.set(radioButtons[4]);
202203
radioGroup.next(true);

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

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,36 +37,28 @@ export class ToolbarRadioGroupPattern<V>
3737
/** Noop. The toolbar handles pointerdown events. */
3838
override onPointerdown(_: PointerEvent): void {}
3939

40+
/** Whether the radio group is currently on the first item. */
41+
isOnFirstItem() {
42+
return this.listBehavior.navigationBehavior.peekPrev() === undefined;
43+
}
44+
45+
/** Whether the radio group is currently on the last item. */
46+
isOnLastItem() {
47+
return this.listBehavior.navigationBehavior.peekNext() === undefined;
48+
}
49+
4050
/** Navigates to the next radio button in the group. */
4151
next(wrap: boolean) {
4252
this.wrap.set(wrap);
43-
const item = this.inputs.activeItem();
4453
this.listBehavior.next();
45-
46-
const leaveGroup = item === this.inputs.activeItem();
47-
if (leaveGroup) {
48-
this.inputs.activeItem.set(undefined);
49-
}
5054
this.wrap.set(false);
51-
return {
52-
leaveGroup,
53-
};
5455
}
5556

5657
/** Navigates to the previous radio button in the group. */
5758
prev(wrap: boolean) {
5859
this.wrap.set(wrap);
59-
const item = this.inputs.activeItem();
6060
this.listBehavior.prev();
61-
62-
const leaveGroup = item === this.inputs.activeItem();
63-
if (leaveGroup) {
64-
this.inputs.activeItem.set(undefined);
65-
}
6661
this.wrap.set(false);
67-
return {
68-
leaveGroup,
69-
};
7062
}
7163

7264
/** Navigates to the first radio button in the group. */

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@ import type {ToolbarPattern} from './toolbar';
1313

1414
/** An interface that allows sub patterns to expose the necessary controls for the toolbar. */
1515
export interface ToolbarWidgetGroupControls {
16+
/** Whether the widget group is currently on the first item. */
17+
isOnFirstItem(): boolean;
18+
19+
/** Whether the widget group is currently on the last item. */
20+
isOnLastItem(): boolean;
21+
1622
/** Navigates to the next widget in the group. */
17-
next(wrap: boolean): {leaveGroup: boolean} | undefined;
23+
next(wrap: boolean): void;
1824

1925
/** Navigates to the previous widget in the group. */
20-
prev(wrap: boolean): {leaveGroup: boolean} | undefined;
26+
prev(wrap: boolean): void;
2127

2228
/** Navigates to the first widget in the group. */
2329
first(): void;
@@ -78,8 +84,10 @@ export class ToolbarWidgetGroupPattern<V> implements ListItem<V> {
7884

7985
/** Default toolbar widget group controls when no controls provided. */
8086
private readonly _defaultControls: ToolbarWidgetGroupControls = {
81-
next: wrap => ({leaveGroup: !wrap}),
82-
prev: wrap => ({leaveGroup: !wrap}),
87+
isOnFirstItem: () => true,
88+
isOnLastItem: () => true,
89+
next: () => {},
90+
prev: () => {},
8391
first: () => {},
8492
last: () => {},
8593
unfocus: () => {},

0 commit comments

Comments
 (0)