Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cdk listbox: multipleselect support and active descendant #19929

Merged
merged 23 commits into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1f14151
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
378ca05
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
d5a4b19
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
e11ce60
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
bf9dadb
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
a2e8339
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
f248a82
build: Added required files to listbox directory.
nielsr98 Jun 11, 2020
f1b291b
build: added listbox option directive and renamed listbox directive f…
nielsr98 Jun 11, 2020
057c921
feat(listbox): added support for non-multiple listbox and aria active…
nielsr98 Jul 9, 2020
5e67e9a
fix(listbox): formatted BUILD.bazel.
nielsr98 Jul 9, 2020
637f1b5
fix(listbox): fixed lint errors.
nielsr98 Jul 9, 2020
c7fb815
fix(listbox): removed unused variable and fix lint error.
nielsr98 Jul 9, 2020
0678a25
fix(listbox): removed unused import and fix docs style.
nielsr98 Jul 9, 2020
e27aaa9
fix(listbox): removed leftover console logs, fixed formatting, and ch…
nielsr98 Jul 10, 2020
4efbd05
refactor(listbox): removed toggleViaKeyboard.
nielsr98 Jul 10, 2020
571d5eb
refactor(listbox): changed listbox keyboard handling to use internal …
nielsr98 Jul 10, 2020
15fdf79
refactor(listbox): subscribe to stream of option selection change emi…
nielsr98 Jul 13, 2020
c417d73
nit(listbox): member naming.
nielsr98 Jul 13, 2020
18c9faa
fix(listbox): coverted query list to array before iteration.
nielsr98 Jul 13, 2020
59205ef
nit(listbox): fixed jsdoc format.
nielsr98 Jul 13, 2020
c5ac3d5
refactor(listbox): improved jsdocs, changed change events to interfaces.
nielsr98 Jul 14, 2020
6b51456
fix(listbox): added semicolons.
nielsr98 Jul 14, 2020
5790b14
refactor(listbox): add jsdoc comments and made event objects inline.
nielsr98 Jul 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cdk-experimental/listbox/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ ng_module(
module_name = "@angular/cdk-experimental/listbox",
deps = [
"//src/cdk/a11y",
"//src/cdk/collections",
"//src/cdk/keycodes",
],
)
Expand Down
302 changes: 298 additions & 4 deletions src/cdk-experimental/listbox/listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,21 +308,271 @@ describe('CdkOption', () => {
expect(listboxInstance._listKeyManager.activeItem).toEqual(optionInstances[2]);
expect(listboxInstance._listKeyManager.activeItemIndex).toBe(2);
});

it('should update selected option on click event', () => {
let selectedOptions = optionInstances.filter(option => option.selected);

expect(selectedOptions.length).toBe(0);
expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse();
expect(optionInstances[0].selected).toBeFalse();
expect(fixture.componentInstance.changedOption).toBeUndefined();

dispatchMouseEvent(optionElements[0], 'click');
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(1);
expect(optionElements[0].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[0].selected).toBeTrue();
expect(fixture.componentInstance.changedOption).toBeDefined();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id);
});
});

describe('multiselectable tests', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
describe('multiselectable tests', () => {
describe('with multiple selection', () => {

Prefer naming describe sub-blocks like this so that the end result reads like a sentence, e.g.

listbox with muliple selection should select all options using the select all method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, noted.

let fixture: ComponentFixture<ListboxMultiselect>;

let testComponent: ListboxMultiselect;

let listbox: DebugElement;
let listboxInstance: CdkListbox;

let options: DebugElement[];
let optionInstances: CdkOption[];
let optionElements: HTMLElement[];

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [CdkListboxModule],
declarations: [ListboxMultiselect],
}).compileComponents();
}));

beforeEach(async(() => {
fixture = TestBed.createComponent(ListboxMultiselect);
fixture.detectChanges();

testComponent = fixture.debugElement.componentInstance;

listbox = fixture.debugElement.query(By.directive(CdkListbox));
listboxInstance = listbox.injector.get<CdkListbox>(CdkListbox);

options = fixture.debugElement.queryAll(By.directive(CdkOption));
optionInstances = options.map(o => o.injector.get<CdkOption>(CdkOption));
optionElements = options.map(o => o.nativeElement);
}));

it('should select all options using the select all method', () => {
let selectedOptions = optionInstances.filter(option => option.selected);
testComponent.isMultiselectable = true;
fixture.detectChanges();

expect(selectedOptions.length).toBe(0);
expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse();
expect(optionInstances[0].selected).toBeFalse();
expect(fixture.componentInstance.changedOption).toBeUndefined();

listboxInstance.setAllSelected(true);
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(4);

for (const option of optionElements) {
expect(option.getAttribute('aria-selected')).toBe('true');
}

expect(fixture.componentInstance.changedOption).toBeDefined();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[3].id);
});

it('should deselect previously selected when multiple is false', () => {
let selectedOptions = optionInstances.filter(option => option.selected);

expect(selectedOptions.length).toBe(0);
expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse();
expect(optionInstances[0].selected).toBeFalse();
expect(fixture.componentInstance.changedOption).toBeUndefined();

dispatchMouseEvent(optionElements[0], 'click');
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(1);
expect(optionElements[0].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[0].selected).toBeTrue();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id);

dispatchMouseEvent(optionElements[2], 'click');
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(1);
expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse();
expect(optionInstances[0].selected).toBeFalse();
expect(optionElements[2].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[2].selected).toBeTrue();

/** Expect first option to be most recently changed because it was deselected. */
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id);
});

it('should allow multiple selection when multiple is true', () => {
let selectedOptions = optionInstances.filter(option => option.selected);
testComponent.isMultiselectable = true;

expect(selectedOptions.length).toBe(0);
expect(fixture.componentInstance.changedOption).toBeUndefined();

dispatchMouseEvent(optionElements[0], 'click');
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(1);
expect(optionElements[0].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[0].selected).toBeTrue();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id);

dispatchMouseEvent(optionElements[2], 'click');
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(2);
expect(optionElements[0].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[0].selected).toBeTrue();
expect(optionElements[2].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[2].selected).toBeTrue();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[2].id);
});

it('should deselect all options when multiple switches to false', () => {
let selectedOptions = optionInstances.filter(option => option.selected);
testComponent.isMultiselectable = true;

expect(selectedOptions.length).toBe(0);
expect(fixture.componentInstance.changedOption).toBeUndefined();

dispatchMouseEvent(optionElements[0], 'click');
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(1);
expect(optionElements[0].getAttribute('aria-selected')).toBe('true');
expect(optionInstances[0].selected).toBeTrue();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id);

testComponent.isMultiselectable = false;
fixture.detectChanges();

selectedOptions = optionInstances.filter(option => option.selected);
expect(selectedOptions.length).toBe(0);
expect(optionElements[0].hasAttribute('aria-selected')).toBeFalse();
expect(optionInstances[0].selected).toBeFalse();
expect(fixture.componentInstance.changedOption.id).toBe(optionInstances[0].id);
});
});

describe('aria active descendant tests', () => {
let fixture: ComponentFixture<ListboxActiveDescendant>;

let testComponent: ListboxActiveDescendant;

let listbox: DebugElement;
let listboxInstance: CdkListbox;
let listboxElement: HTMLElement;

let options: DebugElement[];
let optionInstances: CdkOption[];
let optionElements: HTMLElement[];

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [CdkListboxModule],
declarations: [ListboxActiveDescendant],
}).compileComponents();
}));

beforeEach(async(() => {
fixture = TestBed.createComponent(ListboxActiveDescendant);
fixture.detectChanges();

testComponent = fixture.debugElement.componentInstance;

listbox = fixture.debugElement.query(By.directive(CdkListbox));
listboxInstance = listbox.injector.get<CdkListbox>(CdkListbox);
listboxElement = listbox.nativeElement;

options = fixture.debugElement.queryAll(By.directive(CdkOption));
optionInstances = options.map(o => o.injector.get<CdkOption>(CdkOption));
optionElements = options.map(o => o.nativeElement);
}));

it('should update aria active descendant', () => {
expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse();

listboxInstance.setActiveOption(optionInstances[0]);
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue();
expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[0].id);

listboxInstance.setActiveOption(optionInstances[2]);
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue();
expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[2].id);
});

it('should update aria active descendant via arrow keys', () => {
expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse();

dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW);
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue();
expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[0].id);

dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW);
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeTrue();
expect(listboxElement.getAttribute('aria-activedescendant')).toBe(optionElements[1].id);
});

it('should place focus on options and not set active descendant', () => {
testComponent.isActiveDescendant = false;
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse();

dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW);
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse();
expect(document.activeElement).toEqual(optionElements[0]);
dispatchKeyboardEvent(listboxElement, 'keydown', DOWN_ARROW);
fixture.detectChanges();

expect(listboxElement.hasAttribute('aria-activedescendant')).toBeFalse();
expect(document.activeElement).toEqual(optionElements[1]);

});
});
});

@Component({
template: `
<div cdkListbox
[disabled]="isListboxDisabled"
(selectionChange)="onSelectionChange($event)">
[disabled]="isListboxDisabled"
(selectionChange)="onSelectionChange($event)">
<div cdkOption
[disabled]="isPurpleDisabled">
Purple</div>
Purple
</div>
<div cdkOption
[disabled]="isSolarDisabled">
Solar</div>
Solar
</div>
<div cdkOption>Arc</div>
<div cdkOption>Stasis</div>
</div>`
Expand All @@ -337,3 +587,47 @@ class ListboxWithOptions {
this.changedOption = event.option;
}
}

@Component({
template: `
<div cdkListbox
[multiple]="isMultiselectable"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extra space here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think the (selectionChange) on the next line needs a space.

(selectionChange)="onSelectionChange($event)">
<div cdkOption>Purple</div>
<div cdkOption>Solar</div>
<div cdkOption>Arc</div>
<div cdkOption>Stasis</div>
</div>`
})
class ListboxMultiselect {
changedOption: CdkOption;
isMultiselectable: boolean = false;

onSelectionChange(event: ListboxSelectionChangeEvent) {
this.changedOption = event.option;
}
}

@Component({
template: `
<div cdkListbox
[useActiveDescendant]="isActiveDescendant">
<div cdkOption>Purple</div>
<div cdkOption>Solar</div>
<div cdkOption>Arc</div>
<div cdkOption>Stasis</div>
</div>`
})
class ListboxActiveDescendant {
changedOption: CdkOption;
isActiveDescendant: boolean = true;
focusedOption: string;

onSelectionChange(event: ListboxSelectionChangeEvent) {
this.changedOption = event.option;
}

onFocus(option: string) {
this.focusedOption = option;
}
}
Loading