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

rowClassNameGetter is not applying background color to the row #326

Closed
donaldtf opened this issue May 1, 2018 · 8 comments
Closed

rowClassNameGetter is not applying background color to the row #326

donaldtf opened this issue May 1, 2018 · 8 comments

Comments

@donaldtf
Copy link

donaldtf commented May 1, 2018

Expected Behavior

When using rowClassNameGetter, you should be able to change the background color of the table row by setting it in the class you return.

Current Behavior

Currently the background color is not applied properly when set on the class used in rowClassNameGetter, even though other css properties are.

Possible Solution

I think this should be a simple fix. If you add background-color: inherit to this class: .fixedDataTableCellGroupLayout/cellGroup > .public/fixedDataTableCell/main in this file: src/css/layout/fixedDataTableCellGroupLayout.css then it should allow background colors to propagate through.

Steps to Reproduce (for bugs)

Here is the code pen: https://codepen.io/anon/pen/erWOyg
As you can see, the highlight class is being applied to the row at index 0 because the font-weight has become bold, but the background color is not yellow as would be expected.

Context

I am trying to create a table search that highlights the rows that have a match. I want to add a highlight class in order to make matching rows stand out.

Your Environment

  • Version used: We are currently on 0.8.1, but it looks like this bug is still happening in the latest version that is being pulled in on code pen.
  • Browser Name and version: Chrome, Version: 66.0.3359.139
  • Operating System and version (desktop or mobile): MacOS High Sierra 10.13.4
@donaldtf
Copy link
Author

donaldtf commented May 1, 2018

I am happy to submit a PR with the fix if you would like me to.

@wcjordan
Copy link
Member

wcjordan commented May 1, 2018

Your class is getting added as expected, but since a row is only a container for cells, this isn't how you should apply background color. Instead I recommend updating your cell renderers and applying the background color within the cells.

@donaldtf
Copy link
Author

donaldtf commented May 1, 2018

I was thinking I may need to color individual cells. My question about that is that our table has dynamic columns that a user can add or take away. When they only have a few columns we still leave the table the same width, so we end up with some empty space on the right side of the table. When hovering on a row the hover effect is applied to these non-cells that are still part of the row, but if I color my row by coloring my cells I will not be able to color this extra area, right? Or is there a work around to color this area? I have updated the code pen to show this empty column at the end in case that doesn't make sense.

@wcjordan
Copy link
Member

wcjordan commented May 1, 2018

One way to deal with this is to add a flex column at the end of the table to take up that space and color appropriately. Another way is to apply the hover affect in the same way you're applying the cell coloring, so the hover effect only occurs over the desired cells. Does that fit with what you're thinking, or am I misunderstanding the hover effect you're looking for?

@donaldtf
Copy link
Author

donaldtf commented May 1, 2018

It sounds like either of those options would work well for my use case. Consistency between the two effects (hover highlight and search highlight) is what is most important I believe. Just curious, though, why it would be a bad idea to apply a background color to an entire row? Why include the ability to style a row at all then if styles should be applied at the cell level?

@wcjordan
Copy link
Member

wcjordan commented May 1, 2018

No clue, I didn't write the library, and it's styling outside of the cell renderer doesn't work too well when modified. Feel free to raise any suggestions on #60 and #61 if you have ideas or are interested in tackling anything for a future release.

@donaldtf
Copy link
Author

donaldtf commented May 1, 2018

Ok, awesome. Thanks for your help and all your work on this library!

@wcjordan
Copy link
Member

wcjordan commented May 1, 2018

Np, I'm going to close for now, but feel free to ask if you have any issues or follow up questions.

@wcjordan wcjordan closed this as completed May 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants