fix(chart & table): make to allow highlight in case of numeric column #19938
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #19938 +/- ##
==========================================
- Coverage 66.53% 66.52% -0.02%
==========================================
Files 1714 1714
Lines 65044 65068 +24
Branches 6723 6724 +1
==========================================
+ Hits 43278 43287 +9
- Misses 20055 20069 +14
- Partials 1711 1712 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -71,7 +71,7 @@ export default styled.div` | |||
| cursor: pointer; | |||
| } | |||
| td.dt-is-filter:hover { | |||
| background-color: ${theme.colors.secondary.light4}; | |||
| background-color: ${theme.colors.secondary.light4} !important; | |||
There was a problem hiding this comment.
if this was working for non-integers, why do we need the important now?
There was a problem hiding this comment.
It is because table cell is using inline style when numeric cell and so I need to use important
There was a problem hiding this comment.
Does the inline style need to set a background color at all? If so, can we switch it to use className / Emotion?
... or the other way around, worst case.
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
Outdated
Show resolved
Hide resolved
diegomedina248
approved these changes
May 4, 2022
rusackas
approved these changes
May 4, 2022
hughhhh
pushed a commit
to hve-labs/superset
that referenced
this pull request
May 11, 2022
…apache#19938) * fix(chart & table): make to allow highlight in case of numeric column * fix(chart & table): make to use emitFilter directly * fix(chart & table): make to use styled component instead of inline style
philipher29
pushed a commit
to ValtechMobility/superset
that referenced
this pull request
Jun 9, 2022
…apache#19938) * fix(chart & table): make to allow highlight in case of numeric column * fix(chart & table): make to use emitFilter directly * fix(chart & table): make to use styled component instead of inline style
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Hovering highlight is not working for integer columns on a Table Chart with cross-filtering enabled.
Description
When cross-filtering is enabled, hovering an element on the Table Chart would highlight the cell. However the highlighting is not happening in case the cell is an integer.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE:
cross-filter-highlight.mov
AFTER:
screen-cross-filter.mov
TESTING INSTRUCTIONS
How to reproduce the bug
ADDITIONAL INFORMATION