-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
fix(table): render cells even if data is falsy #7914
Conversation
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.
LGTM
const cells = rowData ? this._getCellTemplatesForRow(row) : []; | ||
|
||
cells.forEach(cell => { | ||
this._getCellTemplatesForRow(row).forEach(cell => { |
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.
Needs a unit test
Added a unit test |
src/cdk/table/table.spec.ts
Outdated
</cdk-table> | ||
` | ||
}) | ||
class EvenSimplerCdkTableApp { |
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.
TableWithBooleanDataSource
?
("EvenSimpler" doesn't tell you anything really)
src/cdk/table/table.spec.ts
Outdated
@@ -109,10 +109,10 @@ describe('CdkTable', () => { | |||
}); | |||
|
|||
it('should render cells even if row data is falsy', () => { | |||
const evenSimplerCdkTableAppFixture = TestBed.createComponent(EvenSimplerCdkTableApp); | |||
const booleanRowCdkTableAppFixture = TestBed.createComponent(EvenSimplerCdkTableApp); |
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.
Name of the component too?
EvenSimplerCdkTableApp
-> TableWithBooleanDataSource
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.
d'oh yes - my brain was thinking they were all renaming
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. |
This was an optimization assuming that undefined data was meant to create a row without cells. This is a faulty design decision and the logic is incorrect anyways (see issue,
0
and other falsy values caused empty rows).Supported workaround to rendering empty rows is to use the
when
predicate that can define a row without columns.Fixes #7902