Clicking a table column does not make the text of the selected column highlighted.
Closed this issue · 6 comments
Description
Not very sure whether this is a bug or as designed.
When clicking a table, the text of the whole selected table will be highlighted
When clicking a value, the text of the value will be highlighted
When clicking a table row, the text of the whole row will be highlighted
When clicking a rendered sole array or object, the text of the whole selected part will be highlighted
The ONLY difference is selecting a table column. If we select a table column, only the background is highlighted, but the style of the text still remains plain
Expected behavior
When the table column is selected, its texts should be also highlighted (bold), not just the background color.
Current Version
- NPM: 10.8.2
- Node: v20.17.0
- Yarn: 4.4.1
- react-json-grid: 0.9.1
- Browser: Edge 129.0.2792.89
- OS: Debian GNU/Linux 12 (bookworm)
This is actually limitation of colgroup & col tag. Basically when a column is selected, to apply styles to all cells in that column I used col tag to change its background color and make font bold, but col doesn't contains actual text its td tag that contains it, that's why text is not being bold.
There is no easy fix to this, I will have to remove colgroup and have a loop to apply styles to all cells in that column. Or just remove bolding of text when clicked completely.
Not sure what I should do.
I feel like removing bolding of text on click completely, as json-grid also not doing it.
Thank you for letting me know that! I think the current behavior is not bad. For some style, the style of the highlighted background does not look very obvious compared to the row color. For example:
Removing the bold text will make the selected row not distinctive to the other rows. Compared to adjusting the highlight color of each theme, I think preserving the current behavior may be better.
Certainly, it is merely my own suggestion. Please determine the solution by yourself. Thank you!
Edited
For somehow, I think I found the reason why the background highlight was not distinctive. It seems that the highlight applied to row/cell will override the background color of <tr> while the highlight set by <colgroup> will not.
In this figure, I selected one column and deliberately changed the style of the nearby <td> to obj highlight by using the inspection. Therefore, I think maybe another feasible solution is to make the row/cell highlight color unified to the column highlight color.
Edited again
I was aware of the fact that the style of striped rows configured on <tr> is provided by the global stylesheet of my Docusaurus project. Therefore, you will not see this issue in your demo. Maybe my concern is not critical because the global styles should be maintained by myself.
I made my own solution for this issue. It changes the internal state type of highlightedElement and does not use <colgroup> to highlight the column anymore.
Please determine whether to accept my solution by yourself. If you prefer to removing the bold text style or choosing another solution, please reject my PR. Thank you!
Edited
By the way, I notice that the version number in package-lock.json is still 0.9.0. To make the PR minimal, I did not change that file, either. Maybe it should be corrected before you publish the next version.
Your solution is same as my first suggestion "remove colgroup and have a loop to apply styles to all cells in that column", and you implemented that really well.
But I still feel having this library simple as it is. Having a loop on a big table to remove and add class on cells will have performance issue. And bolding text on select doesn't seems to be that important, for my library that is.
Thanks for the PR, but I will have to reject that one.
If you want you can raise a PR for removing the font-weight property from the highlight class.
Thank you! I will not raise another PR to do that because I found that I may need to do some extra works for fixing the tests, which I am not very familiar with. If you have made your solution, please close this issue.