feat(stark-ui): table - add support for angular CDK selection model #1377
Conversation
|
+1 for a good night's sleep ;-) |
66d03a7 to
c3b8d7f
Compare
f5cbee4 to
e42aa14
Compare
| } | ||
|
|
||
| public set selection(selection: SelectionModel<object>) { | ||
| if (coerceBooleanProperty(this.multiSelect) || !!this.rowsSelectable) { |
There was a problem hiding this comment.
Shouldn't we make multiselect a boolean with a coerce setter?
There was a problem hiding this comment.
Previously, we had a method called isMultiSelectEnabled() which contained:
return coerceBooleanProperty(this.multiSelect);Maybe could we do this in another PR to improve the table component ? 😊
There was a problem hiding this comment.
Yes I agree, cause it's not really related to this issue.
| const i: number = this.displayedColumns.indexOf("select"); | ||
| this.displayedColumns.splice(i); | ||
| if (this.rowsSelectable) { | ||
| if (!this.displayedColumns.includes("select")) { |
There was a problem hiding this comment.
could you combine these if statements
There was a problem hiding this comment.
I'm not a big fan of combining those 2 if statements.
What would happen if rowsSelectable is set twice to true (true or "true" or 1 using any cast) ?
The "select" displayedColumn would be deleted even it's not the goal...
What do you think ?
There was a problem hiding this comment.
Ah yes, I missed that there is a third "else do nothing" clause and I kind of missed what the code was doing.
If I may suggest a small refactor. But that's mostly preference, so I won't block the PR for this.
const i = this.displayedColumns.indexOf("select");
// Show column
if (this.rowsSelectable && i === -1) {
this.displayedColumns.unshift("select");
}
// Hide column
if (!this.rowsSelectable && i >= 0) {
this.displayedColumns.splice(i);
}There was a problem hiding this comment.
For the rc.1, I'll do a PR to cleanup a bit the table component 😊
e060da3 to
19c1731
Compare
ISSUES CLOSED: #1366 Disabled changeDetection strategy in Stark Table from 'OnPush' to 'Default' in order to fix selection issues. Deprecated 'multiSelect', 'rowsSelectable' Input and 'selectChanged' Output in favor of the new 'selection' Input.
19c1731 to
a20da4f
Compare
ISSUES CLOSED: #1366
Disabled changeDetection strategy in Stark Table from 'OnPush' to 'Default' in order to
fix selection issues.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The developer cannot manage the Stark Table selection programmatically.
Issue Number: #1366
What is the new behavior?
New
selectionInput has been added to allow the developer to obtain all the features proposed by theselectionmodel object provided by/in angular/material.Does this PR introduce a breaking change?
Other information