Skip to content

Commit 5ebb693

Browse files
author
Yifan Ge
committed
Address review comments #2
1 parent 1926ee2 commit 5ebb693

File tree

5 files changed

+24
-32
lines changed

5 files changed

+24
-32
lines changed

src/cdk-experimental/selection/row-selection.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,17 @@ import {CdkSelection} from './selection';
2121
@Directive({
2222
selector: '[cdkRowSelection]',
2323
host: {
24-
'[class.cdk-selected]': '_selection.isSelected(this._value, this._index)',
25-
'[attr.aria-selected]': '_selection.isSelected(this._value, this._index)',
24+
'[class.cdk-selected]': '_selection.isSelected(this.value, this.index)',
25+
'[attr.aria-selected]': '_selection.isSelected(this.value, this.index)',
2626
},
2727
})
2828
export class CdkRowSelection<T> {
29-
@Input('cdkRowSelectionValue') _value: T;
29+
@Input('cdkRowSelectionValue') value: T;
3030

3131
@Input('cdkRowSelectionIndex')
32-
get index(): number|undefined {
33-
return this._index;
34-
}
35-
set index(index: number|undefined) {
36-
this._index = coerceNumberProperty(index);
37-
}
38-
_index?: number;
32+
get index(): number|undefined { return this._index; }
33+
set index(index: number|undefined) { this._index = coerceNumberProperty(index); }
34+
private _index?: number;
3935

4036
constructor(readonly _selection: CdkSelection<T>) {}
4137

src/cdk-experimental/selection/select-all.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {Directive, Inject, isDevMode, OnDestroy, OnInit, Optional, Self} from '@angular/core';
1010
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
11-
import {Observable, of as observableOf, ReplaySubject} from 'rxjs';
11+
import {Observable, of as observableOf, Subject} from 'rxjs';
1212
import {switchMap, takeUntil} from 'rxjs/operators';
1313

1414
import {CdkSelection} from './selection';
@@ -48,7 +48,7 @@ export class CdkSelectAll<T> implements OnDestroy, OnInit {
4848
/**
4949
* Toggles the select-all state.
5050
* @param event The click event if the toggle is triggered by a (mouse or keyboard) click. If
51-
* using with a native <input type="checkbox">, the parameter is required for the
51+
* using with a native `<input type="checkbox">`, the parameter is required for the
5252
* indeterminate state to work properly.
5353
*/
5454
toggle(event?: MouseEvent) {
@@ -64,7 +64,7 @@ export class CdkSelectAll<T> implements OnDestroy, OnInit {
6464
});
6565
}
6666

67-
private readonly _destroyed = new ReplaySubject(1);
67+
private readonly _destroyed = new Subject<void>();
6868

6969
constructor(
7070
@Optional() private readonly _selection: CdkSelection<T>,

src/cdk-experimental/selection/selection-column.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class CdkSelectionColumn<T> implements OnInit, OnDestroy {
6060

6161
this.syncColumnDefName();
6262
}
63-
_name: string;
63+
private _name: string;
6464

6565
@ViewChild(CdkColumnDef, {static: true}) private readonly _columnDef: CdkColumnDef;
6666
@ViewChild(CdkCellDef, {static: true}) private readonly _cell: CdkCellDef;

src/cdk-experimental/selection/selection-toggle.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import {
1818
Self
1919
} from '@angular/core';
2020
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
21-
import {BehaviorSubject, Observable, of as observableOf, ReplaySubject, Subject} from 'rxjs';
22-
import {distinctUntilChanged, startWith, switchMap, takeUntil} from 'rxjs/operators';
21+
import {Observable, of as observableOf, Subject} from 'rxjs';
22+
import {distinctUntilChanged, switchMap, takeUntil} from 'rxjs/operators';
2323

2424
import {CdkSelection} from './selection';
2525

@@ -39,16 +39,12 @@ import {CdkSelection} from './selection';
3939
})
4040
export class CdkSelectionToggle<T> implements OnDestroy, OnInit {
4141
/** The value that is associated with the toggle */
42-
@Input('cdkSelectionToggleValue') private _value: T;
42+
@Input('cdkSelectionToggleValue') value: T;
4343

4444
/** The index of the value in the list. Required when used with `trackBy` */
4545
@Input('cdkSelectionToggleIndex')
46-
get index(): number|undefined {
47-
return this._index;
48-
}
49-
set index(index: number|undefined) {
50-
this._index = coerceNumberProperty(index);
51-
}
46+
get index(): number|undefined { return this._index; }
47+
set index(index: number|undefined) { this._index = coerceNumberProperty(index); }
5248
private _index?: number;
5349

5450
/** The checked state of the selection toggle */
@@ -59,10 +55,10 @@ export class CdkSelectionToggle<T> implements OnDestroy, OnInit {
5955

6056
/** Toggles the selection */
6157
toggle() {
62-
this._selection.toggleSelection(this._value, this._index);
58+
this._selection.toggleSelection(this.value, this.index);
6359
}
6460

65-
private _destroyed = new ReplaySubject(1);
61+
private _destroyed = new Subject<void>();
6662

6763
constructor(
6864
@Optional() private _selection: CdkSelection<T>,
@@ -101,7 +97,7 @@ export class CdkSelectionToggle<T> implements OnDestroy, OnInit {
10197
}
10298

10399
private _isSelected(): boolean {
104-
return this._selection.isSelected(this._value, this._index);
100+
return this._selection.isSelected(this.value, this.index);
105101
}
106102

107103
static ngAcceptInputType_index: NumberInput;

src/cdk-experimental/selection/selection.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
Output,
2020
TrackByFunction
2121
} from '@angular/core';
22-
import {Observable, of as observableOf, ReplaySubject, Subscription} from 'rxjs';
22+
import {Observable, of as observableOf, Subject, Subscription} from 'rxjs';
2323
import {takeUntil} from 'rxjs/operators';
2424

2525
import {SelectableWithIndex, SelectionChange, SelectionSet} from './selection-set';
@@ -48,7 +48,7 @@ export class CdkSelection<T> implements OnInit, AfterContentChecked, CollectionV
4848
}
4949
private _dataSource: TableDataSource<T>;
5050

51-
@Input('trackBy') private _trackByFn: TrackByFunction<T>;
51+
@Input('trackBy') trackByFn: TrackByFunction<T>;
5252

5353
/** Whether to support multiple selection */
5454
@Input('cdkSelectionMultiple')
@@ -69,7 +69,7 @@ export class CdkSelection<T> implements OnInit, AfterContentChecked, CollectionV
6969
/** Subscription that listens for the data provided by the data source. */
7070
private _renderChangeSubscription: Subscription|null;
7171

72-
private _destroyed = new ReplaySubject<void>(1);
72+
private _destroyed = new Subject<void>();
7373

7474
private _selection: SelectionSet<T>;
7575

@@ -115,7 +115,7 @@ export class CdkSelection<T> implements OnInit, AfterContentChecked, CollectionV
115115
}
116116

117117
ngOnInit() {
118-
this._selection = new SelectionSet<T>(this._multiple, this._trackByFn);
118+
this._selection = new SelectionSet<T>(this._multiple, this.trackByFn);
119119
this._selection.changed.pipe(takeUntil(this._destroyed)).subscribe((change) => {
120120
this.updateSelectAllState();
121121
this.change.emit(change);
@@ -139,7 +139,7 @@ export class CdkSelection<T> implements OnInit, AfterContentChecked, CollectionV
139139

140140
/** Toggles selection for a given value. `index` is required if `trackBy` is used. */
141141
toggleSelection(value: T, index?: number) {
142-
if (this._trackByFn && index == null && isDevMode()) {
142+
if (this.trackByFn && index == null && isDevMode()) {
143143
throw Error('CdkSelection: index required when trackBy is used');
144144
}
145145

@@ -168,7 +168,7 @@ export class CdkSelection<T> implements OnInit, AfterContentChecked, CollectionV
168168

169169
/** Checks whether a value is selected. `index` is required if `trackBy` is used. */
170170
isSelected(value: T, index?: number) {
171-
if (this._trackByFn && index == null && isDevMode()) {
171+
if (this.trackByFn && index == null && isDevMode()) {
172172
throw Error('CdkSelection: index required when trackBy is used');
173173
}
174174

0 commit comments

Comments
 (0)