feat(table): support dynamic column definitions #5545
Conversation
googlebot
added
the
cla: yes
src/cdk/table/table.ts
Outdated
| */ | ||
| private _columnDefinitionsByName = new Map<string, CdkColumnDef>(); | ||
| /** Map of all the user's defined columns (header and data cell template) identified by name. */ | ||
| private _columnDefinitionsMap = new Map<string, CdkColumnDef>(); |
There was a problem hiding this comment.
Why change this? I liked the old name (since I generally dislike Hungarian notation)
There was a problem hiding this comment.
Unfamiliar with ByName over Map but I don't really have a preference
There was a problem hiding this comment.
"by name" is referring to how the map is indexed.
src/cdk/table/table.ts
Outdated
| * Check if the header or rows have changed what columns they want to display. If there is a diff, | ||
| * then re-render that section. | ||
| */ | ||
| private _checkColumnsChange() { |
There was a problem hiding this comment.
_renderUpdatedColumns or _renderColumnUpdates?
src/cdk/table/table.ts
Outdated
| ngAfterContentInit() { | ||
| // TODO(andrewseguin): Throw an error if two columns share the same name | ||
| /** Update the map containing the content's column definitions. */ | ||
| private _refreshColumnDefinitionsMap() { |
src/demo-app/table/table-demo.scss
Outdated
| @@ -22,6 +22,18 @@ | |||
| .demo-row-highlight-even { background: #ff0099; } | |||
| .demo-row-highlight-odd { background: #83f52c; } | |||
|
|
|||
| .mat-card { | |||
There was a problem hiding this comment.
Should prefix with a demo- style
andrewseguin
force-pushed
the
table-dynamic-columndef
branch
from
d58b272
to
2c21531
Compare
|
Made changes, ready for review |
src/cdk/table/table.ts
Outdated
| } | ||
|
|
||
| ngDoCheck() { | ||
| if (this._isViewInitialized && this.dataSource && !this._renderChangeSubscription) { |
There was a problem hiding this comment.
Can you add a comment that explains what you're doing with DoCheck and why it's necessary?
|
Added comment about ngDoCheck |
andrewseguin
force-pushed
the
table-dynamic-columndef
branch
from
ddf1887
to
685c5ef
Compare
|
@andrewseguin is this PR still good? Not sure if it was tied to your other PR WRT change detection |
|
It's still good, just will need a tweak or two when rebased with the other PR |
|
@andrewseguin there's one lint error, can add |
andrewseguin
force-pushed
the
table-dynamic-columndef
branch
from
685c5ef
to
bca05cb
Compare
andrewseguin
added
action: merge
andrewseguin
force-pushed
the
table-dynamic-columndef
branch
from
bca05cb
to
338daa3
Compare
andrewseguin
force-pushed
the
table-dynamic-columndef
branch
from
338daa3
to
9f3aa98
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Move the base row's column diff check to the table's change detection to control timing better. The table should have its content checked before reviewing the header/data row columns in case column definitions have been added/removed.