fix(table): column class names should be css friendly #6173
Conversation
googlebot
added
the
cla: yes
src/cdk/table/cell.ts
Outdated
| * as part of a CSS class. Allows for the special characters - and _. | ||
| */ | ||
| getCssClassFriendlyName() { | ||
| return this.name.replace(/[^a-z0-9_-]/ig, ''); |
There was a problem hiding this comment.
I think you're doing more work than you need to by putting the logic here. I would do:
get name { return this._name; }
set name(value) {
this._name = value;
this.cssFriendlyName = (value | '').replace(/[^a-z0-9_-]/ig, '');
}There was a problem hiding this comment.
The non-normalized name is still used by the CdkTable and should match even with the special characters. This change only should affect the CSS classes
There was a problem hiding this comment.
Right, I'm not proposing changing that, that's why I was saving it in _name. I just think it makes sense to only update the cssFriendlyName when the name gets set, rather than whenever change detection runs.
There was a problem hiding this comment.
Ah whoops, totally missed that you were storing the value into a separate variable. Makes sense
src/cdk/table/cell.ts
Outdated
| * as part of a CSS class. Allows for the special characters - and _. | ||
| */ | ||
| getCssClassFriendlyName() { | ||
| return this.name.replace(/[^a-z0-9_-]/ig, ''); |
There was a problem hiding this comment.
Need to mention this replacement in the docs.
| @@ -97,6 +98,16 @@ describe('CdkTable', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should be able to apply class-friendly css class names for the column cells', () => { | |||
| const crazyColumnNameAppFixture = TestBed.createComponent(CrazyColumnNameCdkTableApp); | |||
There was a problem hiding this comment.
Add comment here like
// Test component has a column named "crazy-column-NAME-1!@#$%^-_&*()2"
src/cdk/table/cell.ts
Outdated
| * as part of a CSS class. Allows for the special characters - and _. | ||
| */ | ||
| getCssClassFriendlyName() { | ||
| return this.name.replace(/[^a-z0-9_-]/ig, ''); |
There was a problem hiding this comment.
What's your reasoning for replacing with empty string instead of either _ or -? Should include that in a comment here
There was a problem hiding this comment.
No reasoning, not sure of the standard here. Any preference if the invalid characters are replaced with _ or -?
There was a problem hiding this comment.
I see dashes more commonly in css
andrewseguin
force-pushed
the
table-normalize-column-classes
branch
from
0364eb1
to
644d40d
Compare
|
Test comment added; string is replaced by dashes; added styles section to cdk-table docs; using variable now instead of function |
|
Confirmed this is working in beta 10. |
|
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. |
Column classes are given to header and row cells. E.g. a column named
column-awill give cells a classnamecdk-column-column-a.If the user has a column with special characters, they should be stripped so that CSS selectors cannot be invalid.
This will provide a css class friendly name where it is only alphanumeric or includes special characters like - and _ (just because, if others should be included let me know)
Fixes #6065