Skip to content
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

Grid - conditional cell styling #2588

Merged
merged 11 commits into from
Sep 19, 2018
Merged

Conversation

SAndreeva
Copy link
Contributor

Closes #1079 .

const defaultClasses = ['igx-grid__td igx-grid__td--fw'];

if (this.column.cellClasses) {
Object.entries(this.column.cellClasses).forEach(([name, cb]) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.entries is not supported in IE. Can we add a polyfill for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A polyfill was added with this PR #2279. We will take care to add a note for it in the documentation.

{ field: 'Phone', width: 150, cellClasses: this.cellClasses1 },
{ field: 'Fax', width: 150, cellClasses: this.cellClasses1 }
];
this.data = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you could use SampleTestData.contactInfoData() method here instead of hardcoding the data. The only difference I saw is the lack of numeric data in the SampleTestData. Adding one more numeric field shouldn't break any tests, I hope so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contactInfoData() returns a slightly different data. I will simply add one more method to expose mine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...or find another method.

@@ -308,17 +308,20 @@ export class IgxColumnComponent implements AfterContentInit {
@Input()
public headerClasses = '';
/**
* Sets/gets the class selector of the column cells.
* Sets a conditional class selector of the column cells.
* Accepts an object literal where keys are the name of the classes,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add 'containing key-value pairs' after 'object literal'.

@@ -0,0 +1,17 @@
:host::ng-deep {
.test2 {
color: black !important;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need !important?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when a cell fits two or more conditions, we may want to define which would take precedence...not of great importance, but a possible use case.

@SAndreeva SAndreeva added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Sep 19, 2018
@npaunov npaunov added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Sep 19, 2018
@kdinev kdinev merged commit d054290 into master Sep 19, 2018
@kdinev kdinev deleted the SAndreeva/conditional-cell-style branch September 19, 2018 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants