-
Notifications
You must be signed in to change notification settings - Fork 6.8k
wip: cdk selection #11770
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
wip: cdk selection #11770
Changes from 1 commit
b45d46f
2bd4b3a
f48d574
c515ae8
ce1561b
1a3f875
59629aa
913c434
b9e7261
7888b82
81fd942
4749ee2
f348bb5
3fd4e4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package(default_visibility=["//visibility:public"]) | ||
load("@angular//:index.bzl", "ng_module") | ||
load("@build_bazel_rules_typescript//:defs.bzl", "ts_library", "ts_web_test") | ||
load("@io_bazel_rules_sass//sass:sass.bzl", "sass_binary") | ||
|
||
|
||
ng_module( | ||
name = "selection", | ||
srcs = glob(["**/*.ts"], exclude=["**/*.spec.ts"]), | ||
module_name = "@angular/cdk-experimental/selection", | ||
assets = [] + glob(["**/*.html"]), | ||
deps = [ | ||
"//src/cdk/coercion", | ||
"//src/cdk/collections", | ||
"@rxjs", | ||
], | ||
tsconfig = "//src/cdk-experimental:tsconfig-build.json", | ||
) | ||
|
||
|
||
ts_library( | ||
name = "selection_test_sources", | ||
testonly = 1, | ||
srcs = glob(["**/*.spec.ts"]), | ||
deps = [ | ||
":selection", | ||
"//src/cdk/collections", | ||
"//src/cdk/testing", | ||
"@rxjs", | ||
], | ||
tsconfig = "//src/cdk-experimental:tsconfig-build.json", | ||
) | ||
|
||
ts_web_test( | ||
name = "unit_tests", | ||
bootstrap = [ | ||
"//:web_test_bootstrap_scripts", | ||
], | ||
tags = ["manual"], | ||
|
||
# Do not sort | ||
deps = [ | ||
"//:tslib_bundle", | ||
"//:angular_bundles", | ||
"//:angular_test_bundles", | ||
"//test:angular_test_init", | ||
":selection_test_sources", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/** | ||
* @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 | ||
*/ | ||
|
||
export * from './public-api'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/** | ||
* @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 { CdkSelection } from './selection'; | ||
import { CdkSelectionToggle } from './selection-toggle'; | ||
|
||
|
||
@NgModule({ | ||
declarations: [CdkSelection, CdkSelectionToggle], | ||
exports: [CdkSelection, CdkSelectionToggle] | ||
}) | ||
export class CdkSelectionModule {} | ||
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. When this is merged into the cdk for real, I think it would make sense to add it to the existing |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
/** | ||
* @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 {Directive, Input, OnInit, OnDestroy, ElementRef, ChangeDetectorRef} from '@angular/core'; | ||
import {CdkSelection} from './selection'; | ||
import {Subject} from 'rxjs'; | ||
import {takeUntil} from 'rxjs/operators'; | ||
|
||
|
||
@Directive({ | ||
selector: '[cdkSelectionToggle]', | ||
host: { | ||
'class': 'cdk-selection-toggle', | ||
'role': 'option', | ||
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. Don't want to force a role; something like table wouldn't want to be 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, forgot to remove this. |
||
'tabindex': '-1', | ||
'[attr.aria-selected]': '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. Not sure this should use 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, forgot to remove this. |
||
'[class.cdk-selection-selected]': 'selected', | ||
'(mousedown)': '_onMouseDown($event)', | ||
'(click)': '_onToggle()', | ||
'(contextmenu)': '_onToggle()', | ||
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. Why 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. In things like Finder in mac, when you right click it selects the active item. I wanted it to follow that here. 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. Add comment with the explanation for |
||
'(keydown.enter)': '_onToggle()', | ||
} | ||
}) | ||
export class CdkSelectionToggle<T> implements OnInit, OnDestroy { | ||
|
||
/** Model of the item to toggle selection. */ | ||
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. Expand comment to explain the distinction between a single value or an array 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. How about |
||
@Input('cdkSelectionToggle') model: T | T[]; | ||
|
||
/** Whether the selection is disabled or not. */ | ||
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. Don't need the "or not" at the end of the comment (here and other boolean descriptions) |
||
@Input() disabled: boolean; | ||
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. Use |
||
|
||
/** Whether the toggle is selected or not. */ | ||
selected: boolean; | ||
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 an 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. No, similar to the sort you set the active states on the parent. This is just a internal state. Perhaps I should underscore it? |
||
|
||
/** The modifier that was invoked. */ | ||
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. "that was invoked" when?
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. In my original implementation I listened for this separately because in cases like a checkbox I would listen to the change event rather than click and this information would not be available on that change event. I figured as we carry this forward to other implementations like the checkbox having this at the root would simplify those scenarios. The modifier is public so the selection directive parent can read it out w/o having to pass it. Let me know your thoughts here. |
||
modifier: 'shift' | 'meta' | null; | ||
|
||
private _destroy = new Subject(); | ||
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.
|
||
|
||
constructor( | ||
public elementRef: ElementRef, | ||
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. Does the 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. Yes, its used in the parent directive to identify the element order. 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. Ack; it needs a user-facing JsDoc, then |
||
private _selection: CdkSelection<T>, | ||
private _cd: ChangeDetectorRef | ||
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.
|
||
) {} | ||
|
||
ngOnInit() { | ||
this._selection.selectionChange | ||
.pipe(takeUntil(this._destroy)) | ||
.subscribe(() => this._checkSelected()); | ||
|
||
this._checkSelected(); | ||
this._selection.register(this); | ||
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. Why do this in 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. Personally I try to keep the constructor to only setup functions and put event operations/etc on init. Let me know your thoughts. |
||
} | ||
|
||
ngOnDestroy() { | ||
this._destroy.next(); | ||
this._destroy.complete(); | ||
|
||
this._selection.deregister(this); | ||
} | ||
|
||
/** Mousedown even to capture modifiers. */ | ||
_onMouseDown(event: MouseEvent) { | ||
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. In general methods should be named for what they do rather than when they're called |
||
this._setModifiers(event); | ||
} | ||
|
||
/** Invoke toggle on the parent selection directive. */ | ||
_onToggle() { | ||
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.
|
||
if (!this.disabled) { | ||
this._selection.toggle(this); | ||
} | ||
} | ||
|
||
/** Set the modifiers for the mouse event. */ | ||
_setModifiers(event) { | ||
if (event.metaKey || event.ctrlKey) { | ||
this.modifier = 'meta'; | ||
} else if (event.shiftKey) { | ||
this.modifier = 'shift'; | ||
} | ||
|
||
// Clear the modifier if we don't use it | ||
setTimeout(() => this.modifier = null, 200); | ||
} | ||
|
||
/** Check whether the toggle's model(s) are selected and set state. */ | ||
_checkSelected() { | ||
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. The name 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. How about |
||
if (Array.isArray(this.model)) { | ||
let has = true; | ||
for (const model of this.model) { | ||
if (!this._selection._selectionModel.isSelected(model)) { | ||
has = false; | ||
break; | ||
} | ||
} | ||
this.selected = has; | ||
} else { | ||
this.selected = this._selection._selectionModel.isSelected(this.model); | ||
} | ||
|
||
this._cd.markForCheck(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
The `selection` package handles managing selection state on components | ||
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. FYI, I'll do an editing pass on this after it's submitted since it's easier than doing back and forth review of longer copy. |
||
with support for single and multi select with keyboard modifiers. | ||
|
||
Components such as buttons, checkboxes, etc can all be decorated with | ||
the `cdkSelectionToggle` directive. Once decorated, the directive | ||
will listen for mouse and keyboard events and coordinate the | ||
selection options with the parent `cdkSelection` directive. | ||
|
||
In the example below, the div container of the buttons is decorated with `cdkSelection`. To populate | ||
default selections pass an array of the selections to this option like: | ||
`[cdkSelection]="['yellow']"`. Nested beneath the selection directive is a button decorated | ||
with the `cdkSelectionToggle` directive with an argument of the item's value property. | ||
|
||
```html | ||
<ul cdkSelection cdkSelectionStrategy="single"> | ||
<li *ngFor="let item of items"> | ||
<button [cdkSelectionToggle]="item.value"> | ||
{{item.label}} | ||
</button> | ||
</li> | ||
</ul> | ||
``` | ||
|
||
### Strategies | ||
The selection directive has 3 different strategies for selection: | ||
|
||
- Single: Only one item can be selected at a time. | ||
- Multiple: Multiple items can be selected by clicking | ||
- Modifier Multiple: Multiple items can be select using keyboard modifiers such as shift or ctrl. | ||
|
||
### Clearable | ||
The ability to clear a selection that has been made can be prevented using the `cdkSelectionClearable`. | ||
This means that after a selection (or many selections) have been made users can deselect | ||
all but one of the selections. This concept can be related to radio buttons selection strategy. | ||
|
||
### Tracking | ||
When using complex objects with the selection toggle, custom tracking strategies | ||
can ensure correct identification of the values. The `cdkSelectionTrackBy` accepts | ||
a function that will be invoked with the values of the toggle. This can be used to | ||
return a identifying attribute for the object. | ||
|
||
```TS | ||
@Component({ | ||
template: ` | ||
<ul cdkSelection cdkSelectionStrategy="multiple" [cdkSelectionTrackBy]=""trackBy> | ||
<li *ngFor="let item of items"> | ||
<button [cdkSelectionToggle]="item"> | ||
{{item.label}} | ||
</button> | ||
</li> | ||
</ul> | ||
` | ||
}) | ||
export class MyComponent { | ||
trackBy(model) { | ||
return model.value; | ||
} | ||
} | ||
``` |
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.
Omit spaces in braces