feat: Theming Iris Grid#1568
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1568 +/- ##
==========================================
+ Coverage 46.45% 46.47% +0.01%
==========================================
Files 571 572 +1
Lines 36030 36060 +30
Branches 9020 9026 +6
==========================================
+ Hits 16738 16759 +21
- Misses 19240 19249 +9
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
mofojed
left a comment
There was a problem hiding this comment.
Not sure why some of the grid tests are failing - looks like the colour isn't matching up. Double check that the correct colour is being used, if it is a new colour then update the snapshots (though I'm confused why can open a simple table passes but rollup/goto don't).
|
|
||
| :export { | ||
| grid-bg: $gray-900; | ||
| grid-bg: var(--dh-color-grid-bg); |
There was a problem hiding this comment.
Many of the variables above are no longer used (e.g. $selection-color).
Also the new selection color doesn't match the old color; I think that may be intentional? Confirm with @dsmmcken , but that's one reason the goto row tests are failing.
| --dh-color-grid-tree-line: var(--dh-color-gray-700); | ||
| --dh-color-grid-tree-marker: var(--dh-color-gray-800); | ||
| --dh-color-grid-tree-marker-hover: var(--dh-color-gray-900); |
There was a problem hiding this comment.
These don't match what they were before:
tree-line-color: $gray-500;
tree-marker-color: $gray-300;
tree-marker-hover-color: $gray-100;
If that's intentional, then need to update snapshots. Seems odd we're going in the reverse on the gray scale though (the scale may not be the same anymore? Dunno).
There was a problem hiding this comment.
I've updated the tree variables to be closer to the original for now. Seems to have fixed these tests
14df256 to
4e2d70e
Compare


Testing
Current mapping of Bootstrap gray palette to new

--dh-color-grayvars based on Don's POC. The Bootstrap variables will get updated in a future PR to pull from the new vars, but this is how things line up right now:BREAKING CHANGE: Enterprise will need ThemeProvider for the css variables to be available