From 2f1f7e20c152ac27ebd6d0ae37740cf7c0eab00e Mon Sep 17 00:00:00 2001 From: Cassandra Choi Date: Fri, 17 Feb 2023 16:47:21 -0500 Subject: [PATCH 1/4] feat(cdk/a11y): implement expansion methods --- src/cdk/a11y/key-manager/tree-key-manager.ts | 56 ++++++++++++++++++-- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/src/cdk/a11y/key-manager/tree-key-manager.ts b/src/cdk/a11y/key-manager/tree-key-manager.ts index ef3592b2a180..0cbc403a8792 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager.ts @@ -320,15 +320,65 @@ export class TreeKeyManager { /** * If the item is already expanded, we collapse the item. Otherwise, we will focus the parent. */ - private _collapseCurrentItem() {} + private _collapseCurrentItem() { + if (!this._activeItem) { + return; + } + + if (!this._isCurrentItemExpanded()) { + this._activeItem.collapse(); + } else { + const parent = this._activeItem.getParent(); + if (!parent) { + return; + } + this._setActiveItem(parent as T); + } + } /** * If the item is already collapsed, we expand the item. Otherwise, we will focus the first child. */ - private _expandCurrentItem() {} + private _expandCurrentItem() { + if (!this._activeItem) { + return; + } + + if (!this._isCurrentItemExpanded()) { + this._activeItem.expand(); + } else { + coerceObservable(this._activeItem.getChildren()) + .pipe(take(1)) + .subscribe(children => { + const firstChild = children[0]; + if (!firstChild) { + return; + } + this._setActiveItem(firstChild as T); + }); + } + } /** For all items that are the same level as the current item, we expand those items. */ - private _expandAllItemsAtCurrentItemLevel() {} + private _expandAllItemsAtCurrentItemLevel() { + if (!this._activeItem) { + return; + } + + const parent = this._activeItem.getParent(); + let itemsToExpand; + if (!parent) { + itemsToExpand = observableOf(this._items.filter(item => item.getParent() === null)); + } else { + itemsToExpand = coerceObservable(parent.getChildren()); + } + + itemsToExpand.pipe(take(1)).subscribe(items => { + for (const item of items) { + item.expand(); + } + }); + } private _activateCurrentItem() { this._activeItem?.activate(); From 4fb2a91faebd7d62e0948cac6c53c9dd9653a72c Mon Sep 17 00:00:00 2001 From: Cassandra Choi Date: Fri, 17 Feb 2023 16:47:21 -0500 Subject: [PATCH 2/4] feat(cdk/a11y): add tests and fixes for expand/collapse interactions --- .../a11y/key-manager/tree-key-manager.spec.ts | 300 +++++++++++++++++- src/cdk/a11y/key-manager/tree-key-manager.ts | 19 +- 2 files changed, 315 insertions(+), 4 deletions(-) diff --git a/src/cdk/a11y/key-manager/tree-key-manager.spec.ts b/src/cdk/a11y/key-manager/tree-key-manager.spec.ts index a45feaedf4ee..5d9cb4803330 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager.spec.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager.spec.ts @@ -14,7 +14,7 @@ import {createKeyboardEvent} from '../../testing/private'; import {QueryList} from '@angular/core'; import {take} from 'rxjs/operators'; import {TreeKeyManager, TreeKeyManagerItem} from './tree-key-manager'; -import {Observable, of as observableOf} from 'rxjs'; +import {Observable, of as observableOf, Subscription} from 'rxjs'; class FakeBaseTreeKeyManagerItem { _isExpanded = false; @@ -66,6 +66,12 @@ interface ItemConstructorTestContext { | FakeObservableTreeKeyManagerItem; } +interface ExpandCollapseKeyEventTestContext { + direction: 'ltr' | 'rtl'; + expandKeyEvent: () => KeyboardEvent; + collapseKeyEvent: () => KeyboardEvent; +} + describe('TreeKeyManager', () => { let fakeKeyEvents: { downArrow: KeyboardEvent; @@ -430,6 +436,298 @@ describe('TreeKeyManager', () => { expect(fakeKeyEvents.upArrow.defaultPrevented).toBe(true); }); }); + + describe('expand/collapse key events', () => { + const parameters: ExpandCollapseKeyEventTestContext[] = [ + { + direction: 'ltr', + expandKeyEvent: () => fakeKeyEvents.rightArrow, + collapseKeyEvent: () => fakeKeyEvents.leftArrow, + }, + { + direction: 'rtl', + expandKeyEvent: () => fakeKeyEvents.leftArrow, + collapseKeyEvent: () => fakeKeyEvents.rightArrow, + }, + ]; + + for (const param of parameters) { + describe(`in ${param.direction} mode`, () => { + beforeEach(() => { + keyManager = new TreeKeyManager({ + items: itemList, + horizontalOrientation: param.direction, + }); + for (const item of itemList) { + item._isExpanded = false; + } + }); + + it('with nothing active, expand key does not expand any items', () => { + expect(itemList.toArray().map(item => item.isExpanded())) + .withContext('item expansion state, for all items') + .toEqual(itemList.toArray().map(_ => false)); + + keyManager.onKeydown(param.expandKeyEvent()); + + expect(itemList.toArray().map(item => item.isExpanded())) + .withContext('item expansion state, for all items, after expand event') + .toEqual(itemList.toArray().map(_ => false)); + }); + + it('with nothing active, collapse key does not collapse any items', () => { + for (const item of itemList) { + item._isExpanded = true; + } + expect(itemList.toArray().map(item => item.isExpanded())) + .withContext('item expansion state, for all items') + .toEqual(itemList.toArray().map(_ => true)); + + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(itemList.toArray().map(item => item.isExpanded())) + .withContext('item expansion state, for all items') + .toEqual(itemList.toArray().map(_ => true)); + }); + + it('with nothing active, expand key does not change the active item index', () => { + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toEqual(-1); + + keyManager.onKeydown(param.expandKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after expand event') + .toEqual(-1); + }); + + it('with nothing active, collapse key does not change the active item index', () => { + for (const item of itemList) { + item._isExpanded = true; + } + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toEqual(-1); + + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after collapse event') + .toEqual(-1); + }); + + describe('if the current item is expanded', () => { + let spy: jasmine.Spy; + let subscription: Subscription; + + beforeEach(() => { + keyManager.onClick(parentItem); + parentItem._isExpanded = true; + + spy = jasmine.createSpy('change spy'); + subscription = keyManager.change.subscribe(spy); + }); + + afterEach(() => { + subscription.unsubscribe(); + }); + + it('when the expand key is pressed, moves to the first child', () => { + keyManager.onKeydown(param.expandKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one expand key event.') + .toBe(1); + expect(spy).not.toHaveBeenCalledWith(parentItem); + expect(spy).toHaveBeenCalledWith(childItem); + }); + + it( + 'when the expand key is pressed, and the first child is disabled, ' + + 'moves to the first non-disabled child', + () => { + childItem.isDisabled = true; + + keyManager.onKeydown(param.expandKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one expand key event.') + .toBe(3); + expect(spy).not.toHaveBeenCalledWith(parentItem); + expect(spy).not.toHaveBeenCalledWith(childItem); + expect(spy).toHaveBeenCalledWith(childItemWithNoChildren); + }, + ); + + it( + 'when the expand key is pressed, and all children are disabled, ' + + 'does not change the active item', + () => { + childItem.isDisabled = true; + childItemWithNoChildren.isDisabled = true; + + keyManager.onKeydown(param.expandKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one expand key event.') + .toBe(0); + expect(spy).not.toHaveBeenCalled(); + }, + ); + + it('when the collapse key is pressed, collapses the item', () => { + expect(parentItem.isExpanded()) + .withContext('active item initial expansion state') + .toBe(true); + + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(parentItem.isExpanded()) + .withContext('active item expansion state, after collapse key') + .toBe(false); + }); + + it('when the collapse key is pressed, does not change the active item', () => { + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toBe(0); + + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one collapse key event.') + .toBe(0); + expect(spy).not.toHaveBeenCalled(); + }); + }); + + describe('if the current item is expanded, and there are no children', () => { + let spy: jasmine.Spy; + let subscription: Subscription; + + beforeEach(() => { + keyManager.onClick(childItemWithNoChildren); + childItemWithNoChildren._isExpanded = true; + + spy = jasmine.createSpy('change spy'); + subscription = keyManager.change.subscribe(spy); + }); + + afterEach(() => { + subscription.unsubscribe(); + }); + + it('when the expand key is pressed, does not change the active item', () => { + keyManager.onKeydown(param.expandKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one expand key event.') + .toBe(3); + expect(spy).not.toHaveBeenCalled(); + }); + }); + + describe('if the current item is collapsed, and has a parent item', () => { + let spy: jasmine.Spy; + let subscription: Subscription; + + beforeEach(() => { + keyManager.onClick(childItem); + childItem._isExpanded = false; + + spy = jasmine.createSpy('change spy'); + subscription = keyManager.change.subscribe(spy); + }); + + afterEach(() => { + subscription.unsubscribe(); + }); + + it('when the expand key is pressed, expands the current item', () => { + expect(childItem.isExpanded()) + .withContext('active item initial expansion state') + .toBe(false); + + keyManager.onKeydown(param.expandKeyEvent()); + + expect(childItem.isExpanded()) + .withContext('active item expansion state, after expand key') + .toBe(true); + }); + + it('when the expand key is pressed, does not change active item', () => { + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toBe(1); + + keyManager.onKeydown(param.expandKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one collapse key event.') + .toBe(1); + expect(spy).not.toHaveBeenCalled(); + }); + + it('when the collapse key is pressed, moves the active item to the parent', () => { + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toBe(1); + + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one collapse key event.') + .toBe(0); + }); + + it('when the collapse key is pressed, and the parent is disabled, does nothing', () => { + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toBe(1); + + parentItem.isDisabled = true; + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one collapse key event.') + .toBe(1); + }); + }); + + describe('if the current item is collapsed, and has no parent items', () => { + let spy: jasmine.Spy; + let subscription: Subscription; + + beforeEach(() => { + keyManager.onClick(parentItem); + parentItem._isExpanded = false; + + spy = jasmine.createSpy('change spy'); + subscription = keyManager.change.subscribe(spy); + }); + + afterEach(() => { + subscription.unsubscribe(); + }); + + it('when the collapse key is pressed, does nothing', () => { + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, initial') + .toBe(0); + + keyManager.onKeydown(param.collapseKeyEvent()); + + expect(keyManager.getActiveItemIndex()) + .withContext('active item index, after one collapse key event.') + .toBe(0); + expect(spy).not.toHaveBeenCalledWith(parentItem); + }); + }); + }); + } + }); }); } }); diff --git a/src/cdk/a11y/key-manager/tree-key-manager.ts b/src/cdk/a11y/key-manager/tree-key-manager.ts index 0cbc403a8792..0563a9388d35 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager.ts @@ -325,11 +325,11 @@ export class TreeKeyManager { return; } - if (!this._isCurrentItemExpanded()) { + if (this._isCurrentItemExpanded()) { this._activeItem.collapse(); } else { const parent = this._activeItem.getParent(); - if (!parent) { + if (!parent || this._isItemDisabled(parent)) { return; } this._setActiveItem(parent as T); @@ -350,7 +350,7 @@ export class TreeKeyManager { coerceObservable(this._activeItem.getChildren()) .pipe(take(1)) .subscribe(children => { - const firstChild = children[0]; + const firstChild = children.find(child => !this._isItemDisabled(child)); if (!firstChild) { return; } @@ -359,6 +359,19 @@ export class TreeKeyManager { } } + private _isCurrentItemExpanded() { + if (!this._activeItem) { + return false; + } + return typeof this._activeItem.isExpanded === 'boolean' + ? this._activeItem.isExpanded + : this._activeItem.isExpanded(); + } + + private _isItemDisabled(item: TreeKeyManagerItem) { + return typeof item.isDisabled === 'boolean' ? item.isDisabled : item.isDisabled?.(); + } + /** For all items that are the same level as the current item, we expand those items. */ private _expandAllItemsAtCurrentItemLevel() { if (!this._activeItem) { From 8b4251c037005db7868a10867b3748670def85b5 Mon Sep 17 00:00:00 2001 From: Cassandra Choi Date: Fri, 17 Feb 2023 16:47:21 -0500 Subject: [PATCH 3/4] feat(cdk/a11y): actually fix build --- src/cdk/a11y/key-manager/tree-key-manager.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/cdk/a11y/key-manager/tree-key-manager.ts b/src/cdk/a11y/key-manager/tree-key-manager.ts index 0563a9388d35..6da1a6e4478e 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager.ts @@ -18,7 +18,15 @@ import { UP_ARROW, } from '@angular/cdk/keycodes'; import {QueryList} from '@angular/core'; -import {isObservable, Observable, Subject} from 'rxjs'; +import {isObservable, of as observableOf, Observable, Subject} from 'rxjs'; +import {take} from 'rxjs/operators'; + +function coerceObservable(data: T | Observable): Observable { + if (!isObservable(data)) { + return observableOf(data); + } + return data; +} /** Represents an item within a tree that can be passed to a TreeKeyManager. */ export interface TreeKeyManagerItem { From bbfea9094fb516f04486ff12c537ca224e145b42 Mon Sep 17 00:00:00 2001 From: Cassandra Choi Date: Fri, 17 Feb 2023 16:47:21 -0500 Subject: [PATCH 4/4] feat(cdk/a11y): use skipPredicate instead of disabled --- src/cdk/a11y/key-manager/tree-key-manager.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cdk/a11y/key-manager/tree-key-manager.ts b/src/cdk/a11y/key-manager/tree-key-manager.ts index 6da1a6e4478e..3d39eaabdc24 100644 --- a/src/cdk/a11y/key-manager/tree-key-manager.ts +++ b/src/cdk/a11y/key-manager/tree-key-manager.ts @@ -116,8 +116,7 @@ export class TreeKeyManager { * Predicate function that can be used to check whether an item should be skipped * by the key manager. By default, disabled items are skipped. */ - private _skipPredicateFn = (item: T) => - typeof item.isDisabled === 'boolean' ? item.isDisabled : !!item.isDisabled?.(); + private _skipPredicateFn = (item: T) => this._isItemDisabled(item); /** Function to determine equivalent items. */ private _trackByFn: (item: T) => unknown = (item: T) => item; @@ -337,7 +336,7 @@ export class TreeKeyManager { this._activeItem.collapse(); } else { const parent = this._activeItem.getParent(); - if (!parent || this._isItemDisabled(parent)) { + if (!parent || this._skipPredicateFn(parent as T)) { return; } this._setActiveItem(parent as T); @@ -358,7 +357,7 @@ export class TreeKeyManager { coerceObservable(this._activeItem.getChildren()) .pipe(take(1)) .subscribe(children => { - const firstChild = children.find(child => !this._isItemDisabled(child)); + const firstChild = children.find(child => !this._skipPredicateFn(child as T)); if (!firstChild) { return; }