Skip to content

fix(api): updates the api to standard naming and adds docs for selection #8286

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

Merged
merged 3 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 33 additions & 2 deletions src/cdk/collections/collections.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
### Collections
The `collections` package provides a set of utilities for managing collections.

A set of utilities for managing collections.
### `SelectionModel`
`SelectionModel` is a utility for powering selection of one or more options from a list.
This model is used in components such as the selection list, table selections and chip lists.

#### Basic Usage
```javascript
const model = new SelectionModel(
true, // multiple selection or not
[2,1,3] // initial selected values
);

// select a value
model.select(4);
console.log(model.selected.length) //-> 4

// deselect a value
model.deselect(4);
console.log(model.selected.length) //-> 3

// toggle a value
model.toggle(4);
console.log(model.selected.length) //-> 4

// check for selection
console.log(model.isSelected(4)) //-> true

// sort the selections
console.log(model.sort()) //-> [1,2,3,4]

// listen for changes
model.changes.subscribe(s => console.log(s));
```
14 changes: 7 additions & 7 deletions src/cdk/collections/selection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('SelectionModel', () => {
it('should be able to select multiple options', () => {
const onChangeSpy = jasmine.createSpy('onChange spy');

model.onChange!.subscribe(onChangeSpy);
model.changed!.subscribe(onChangeSpy);
model.select(1);
model.select(2);

Expand All @@ -56,7 +56,7 @@ describe('SelectionModel', () => {
it('should be able to select multiple options at the same time', () => {
const onChangeSpy = jasmine.createSpy('onChange spy');

model.onChange!.subscribe(onChangeSpy);
model.changed!.subscribe(onChangeSpy);
model.select(1, 2);

expect(model.selected.length).toBe(2);
Expand Down Expand Up @@ -112,7 +112,7 @@ describe('SelectionModel', () => {

model.select(1);

model.onChange!.subscribe(spy);
model.changed!.subscribe(spy);

model.select(2);

Expand Down Expand Up @@ -144,7 +144,7 @@ describe('SelectionModel', () => {
model = new SelectionModel(true);
spy = jasmine.createSpy('SelectionModel change event');

model.onChange!.subscribe(spy);
model.changed!.subscribe(spy);
});

it('should emit an event when a value is selected', () => {
Expand All @@ -167,7 +167,7 @@ describe('SelectionModel', () => {
it('should not emit an event when preselecting values', () => {
model = new SelectionModel(false, [1]);
spy = jasmine.createSpy('SelectionModel initial change event');
model.onChange!.subscribe(spy);
model.changed!.subscribe(spy);

expect(spy).not.toHaveBeenCalled();
});
Expand All @@ -181,7 +181,7 @@ describe('SelectionModel', () => {
model = new SelectionModel(true, [1, 2, 3]);
spy = jasmine.createSpy('SelectionModel change event');

model.onChange!.subscribe(spy);
model.changed!.subscribe(spy);
});

it('should emit an event when a value is deselected', () => {
Expand Down Expand Up @@ -218,7 +218,7 @@ describe('SelectionModel', () => {
});

it('should not have an onChange stream if change events are disabled', () => {
expect(model.onChange).toBeFalsy();
expect(model.changed).toBeFalsy();
});

it('should still update the select value', () => {
Expand Down
13 changes: 10 additions & 3 deletions src/cdk/collections/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,14 @@ export class SelectionModel<T> {
}

/** Event emitted when the value has changed. */
onChange: Subject<SelectionChange<T>> | null = this._emitChanges ? new Subject() : null;
changed: Subject<SelectionChange<T>> | null = this._emitChanges ? new Subject() : null;
Copy link
Member

Choose a reason for hiding this comment

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

Didn't we land on change present-tense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We landed on myEventThingChange but the rest of them ended up past tense.

Copy link
Member

Choose a reason for hiding this comment

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

Which things are changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few I know of selectedChanged, _scrollDistanceChanged and _selectedIndexChanged. Let me know what you want to do.


/**
* Event emitted when the value has changed.
* @deprecated Use `changed` instead.
Copy link
Member

Choose a reason for hiding this comment

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

Need to add a deletion-target here (8.0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

* @deletion-target 8.0.0 To be changed to `changed`
*/
onChange: Subject<SelectionChange<T>> | null = this.changed;

constructor(
private _multiple = false,
Expand Down Expand Up @@ -129,8 +136,8 @@ export class SelectionModel<T> {
this._selected = null;

if (this._selectedToEmit.length || this._deselectedToEmit.length) {
if (this.onChange) {
this.onChange.next({
if (this.changed) {
this.changed.next({
source: this,
added: this._selectedToEmit,
removed: this._deselectedToEmit
Expand Down