-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Cdk listbox dev app #20010
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 dev app #20010
Changes from 18 commits
d3fbbfc
5066335
d0133cd
1e84c2d
e22b3f2
a9b2519
b1da11c
0af787c
7ad3615
86510e4
41b62da
83db6b5
71d9806
e30d8e5
169ee9c
817c6df
f7454da
127355d
72287d4
1b24b51
5278f78
d765094
033f30a
02dc398
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ import { | |
QueryList | ||
} from '@angular/core'; | ||
import {ActiveDescendantKeyManager, Highlightable, ListKeyManagerOption} from '@angular/cdk/a11y'; | ||
import {END, ENTER, HOME, SPACE} from '@angular/cdk/keycodes'; | ||
import {DOWN_ARROW, END, ENTER, HOME, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; | ||
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion'; | ||
import {SelectionChange, SelectionModel} from '@angular/cdk/collections'; | ||
import {defer, merge, Observable, Subject} from 'rxjs'; | ||
|
@@ -33,11 +33,12 @@ let nextId = 0; | |
'(focus)': 'activate()', | ||
'(blur)': 'deactivate()', | ||
'[id]': 'id', | ||
'[attr.aria-selected]': '_selected || null', | ||
'[attr.aria-selected]': 'selected || null', | ||
'[attr.tabindex]': '_getTabIndex()', | ||
'[attr.aria-disabled]': '_isInteractionDisabled()', | ||
'[class.cdk-option-disabled]': '_isInteractionDisabled()', | ||
'[class.cdk-option-active]': '_active' | ||
'[class.cdk-option-active]': '_active', | ||
'[class.cdk-option-selected]': 'selected' | ||
} | ||
}) | ||
export class CdkOption implements ListKeyManagerOption, Highlightable { | ||
|
@@ -171,15 +172,17 @@ export class CdkOption implements ListKeyManagerOption, Highlightable { | |
host: { | ||
'role': 'listbox', | ||
'(keydown)': '_keydown($event)', | ||
'[attr.aria-disabled]': '_disabled', | ||
'[attr.aria-multiselectable]': '_multiple', | ||
'[attr.tabindex]': 'tabIndex', | ||
'[attr.aria-disabled]': 'disabled', | ||
'[attr.aria-multiselectable]': 'multiple', | ||
'[attr.aria-activedescendant]': '_getAriaActiveDescendant()' | ||
} | ||
}) | ||
export class CdkListbox implements AfterContentInit, OnDestroy, OnInit { | ||
|
||
_listKeyManager: ActiveDescendantKeyManager<CdkOption>; | ||
_selectionModel: SelectionModel<CdkOption>; | ||
tabIndex = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be prefixed with an underscore? (similarly to how we do it within the selection list) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah probably to stay consistent. |
||
|
||
readonly optionSelectionChanges: Observable<OptionSelectionChangeEvent> = defer(() => { | ||
const options = this._options; | ||
|
@@ -256,7 +259,10 @@ export class CdkListbox implements AfterContentInit, OnDestroy, OnInit { | |
|
||
private _initKeyManager() { | ||
this._listKeyManager = new ActiveDescendantKeyManager(this._options) | ||
.withWrap().withVerticalOrientation().withTypeAhead(); | ||
.withWrap() | ||
.withVerticalOrientation() | ||
.withTypeAhead() | ||
.withAllowedModifierKeys(['shiftKey']); | ||
|
||
this._listKeyManager.change.pipe(takeUntil(this._destroyed)).subscribe(() => { | ||
this._updateActiveOption(); | ||
|
@@ -284,6 +290,7 @@ export class CdkListbox implements AfterContentInit, OnDestroy, OnInit { | |
|
||
const manager = this._listKeyManager; | ||
const {keyCode} = event; | ||
const previousActiveIndex = manager.activeItemIndex; | ||
|
||
if (keyCode === HOME || keyCode === END) { | ||
event.preventDefault(); | ||
|
@@ -297,6 +304,12 @@ export class CdkListbox implements AfterContentInit, OnDestroy, OnInit { | |
} else { | ||
manager.onKeydown(event); | ||
} | ||
|
||
/** Will select an option if shift was pressed while navigating to the option */ | ||
const isArrow = (keyCode === UP_ARROW || keyCode === DOWN_ARROW); | ||
if (isArrow && event.shiftKey && previousActiveIndex !== this._listKeyManager.activeItemIndex) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the thinking behind toggling options based on arrow keys with shift rather than requiring a separate space/enter keypress? Was this from the aria authoring practices? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it would work with a separate space/enter keypress. In the authoring practices it says that selection via SHIFT + arrow key is an optional way to do selection. |
||
this._toggleActiveOption(); | ||
devversion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
/** Emits a selection change event, called when an option has its selected state changed. */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
load("//tools:defaults.bzl", "ng_module") | ||
|
||
package(default_visibility = ["//visibility:public"]) | ||
|
||
ng_module( | ||
name = "cdk-experimental-listbox", | ||
srcs = glob(["**/*.ts"]), | ||
assets = [ | ||
"cdk-listbox-demo.html", | ||
"cdk-listbox-demo.css", | ||
], | ||
deps = [ | ||
"//src/cdk-experimental/listbox", | ||
"@npm//@angular/router", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* @license | ||
* Copyright Google LLC All Rights Reserved. | ||
* | ||
* Use of this source code is governed by an MIT-style license that can be | ||
* found in the LICENSE file at https://angular.io/license | ||
*/ | ||
|
||
import {NgModule} from '@angular/core'; | ||
import {CommonModule} from '@angular/common'; | ||
import {RouterModule} from '@angular/router'; | ||
import {CdkListboxModule} from '@angular/cdk-experimental/listbox'; | ||
|
||
import {CdkListboxDemo} from './cdk-listbox-demo'; | ||
|
||
@NgModule({ | ||
imports: [ | ||
CdkListboxModule, | ||
CommonModule, | ||
RouterModule.forChild([{path: '', component: CdkListboxDemo}]), | ||
], | ||
declarations: [CdkListboxDemo], | ||
}) | ||
export class CdkListboxDemoModule {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
.example-listbox { | ||
list-style-type: none; | ||
border: 1px solid black; | ||
cursor: default; | ||
padding: 0; | ||
} | ||
|
||
.cdk-option-selected { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you change these styles to only target css classes prefixed with |
||
background: cornflowerblue; | ||
} | ||
|
||
.cdk-option-active { | ||
background: lightgrey; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<ul cdkListbox class="example-listbox" [multiple]="multiSelectable" [useActiveDescendant]="activeDescendant"> | ||
<li class="example-option" cdkOption>Apple</li> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this class a noop right now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes I forgot to remove it after I changed the way I did the styling. |
||
<li cdkOption>Orange</li> | ||
<li cdkOption>Grapefruit</li> | ||
<li cdkOption>Peach</li> | ||
</ul> | ||
|
||
<button (click)="toggleMultiple()">Toggle Multiple</button> | ||
<br> | ||
<button (click)="toggleActiveDescendant()">Toggle Active Descendant</button> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/** | ||
* @license | ||
* Copyright Google LLC All Rights Reserved. | ||
* | ||
* Use of this source code is governed by an MIT-style license that can be | ||
* found in the LICENSE file at https://angular.io/license | ||
*/ | ||
|
||
import {Component} from '@angular/core'; | ||
|
||
@Component({ | ||
templateUrl: 'cdk-listbox-demo.html', | ||
styleUrls: ['cdk-listbox-demo.css'], | ||
}) | ||
export class CdkListboxDemo { | ||
multiSelectable = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: This file should use 2 spaces for indentation. We should set up a formatter at some point again: #19879 |
||
activeDescendant = true; | ||
|
||
toggleMultiple() { | ||
this.multiSelectable = !this.multiSelectable; | ||
} | ||
|
||
toggleActiveDescendant() { | ||
this.activeDescendant = !this.activeDescendant; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should pass the modifiers directly to
createKeyboardEvent
(so we avoid thedefineProperty
call here).