-
Notifications
You must be signed in to change notification settings - Fork 860
[Visual Refresh] Make EuiDataGrid and EuiTable hover consistent
#8769
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
Conversation
9561a64 to
7e58e9a
Compare
|
Converting to draft because a cypress test is failing — |
|
Hey @acstll, I'm afraid I need some clarification on the questions. Are they related only to Amsterdam? Why then we change anything there? |
@ek-so it's all clear now! We shouldn't change anything visually on Amsterdam; this was not clear to me and all related questions come from that confusion, so sorry — also the question regarding
thanks for checking 🙏 |
7e58e9a to
0ebdcd5
Compare
|
Quick status update: I changed the implementation to use (new) component tokens instead of that bit of TS logic, in order to have Amsterdam remain exactly the same. Will do the following next:
|
8cd38f5 to
828355a
Compare
packages/eui/src/themes/amsterdam/global_styling/variables/_colors.ts
Outdated
Show resolved
Hide resolved
- borderStrongText - backgroundBaseInteractiveSelectHover
"clickable" styles are applied before "selected" so the latter, on hover, always wins
- borderStrongText and backgroundBaseInteractiveSelectHover - fix values in JSON
to demo new tokens backgroundBaseInteractiveSelect and backgroundBaseInteractiveSelectHover
add 6 new tokens for the "stripes" version, mainly to be able to have borealis and amsterdam have different styles
cc15c97 to
94c55b0
Compare
💚 Build SucceededHistory
cc @acstll |
💚 Build Succeeded
History
cc @acstll |
|
@acstll after this is merged, I would inform the discover team about it additionally, because they have a custom implementation in discover for this thing, and it doesn't look clean currently. We promised to do that as far as I recall, here. |
@ek-so I created a sub-task to track it (https://github.com/elastic/eui-private/issues/360), we'll be glad to help |
mgadewoll
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 Changes are looking great. Thanks for the update! 🎉
`103.1.0` ⏩ `104.0.2` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) ## Changes - updates flat tooltip prop usages on **EuiCopy** to use new `tooltipProp` ## Package updates ### `@elastic/eui` ## [`v104.0.2`](https://github.com/elastic/eui/releases/v104.0.2) **Bug fixes** - Fixed missing JSON token exports for `euiColorBackgroundBaseInteractiveSelectHover` and `euiColorBorderStrongText` ([#8819](elastic/eui#8819)) ## [`v104.0.1`](https://github.com/elastic/eui/releases/v104.0.1) **Bug fixes** - Fixed a wrong path in the module declaration for the JSON token exports that would trigger typescript errors ([#8818](elastic/eui#8818)) ## [`v104.0.0`](https://github.com/elastic/eui/releases/v104.0.0) - Added data vis text color tokens: ([#8793](elastic/eui#8793)) - `colors.vis.euiColorVisText0` - `colors.vis.euiColorVisText1` - `colors.vis.euiColorVisText2` - `colors.vis.euiColorVisText3` - `colors.vis.euiColorVisText4` - `colors.vis.euiColorVisText5` - `colors.vis.euiColorVisText6` - `colors.vis.euiColorVisText7` - `colors.vis.euiColorVisText8` - `colors.vis.euiColorVisText9` - Updated and aligned background hover styles for `EuiTable` and `EuiDataGrid` ([#8769](elastic/eui#8769)) **Deprecations** - Deprecated `euiPaletteForLightBackground` and `euiPaletteForDarkBackground` palettes. Use the newly added data vis color tokens instead. ([#8793](elastic/eui#8793)) **Breaking changes** - Removed tokens: ([#8793](elastic/eui#8793)) - `colors.vis.euiColorVisAsTextLight1` - `colors.vis.euiColorVisAsTextLight0` - `colors.vis.euiColorVisAsTextLight2` - `colors.vis.euiColorVisAsTextLight3` - `colors.vis.euiColorVisAsTextLight4` - `colors.vis.euiColorVisAsTextLight5` - `colors.vis.euiColorVisAsTextLight6` - `colors.vis.euiColorVisAsTextDark1` - `colors.vis.euiColorVisAsTextDark0` - `colors.vis.euiColorVisAsTextDark2` - `colors.vis.euiColorVisAsTextDark3` - `colors.vis.euiColorVisAsTextDark4` - `colors.vis.euiColorVisAsTextDark5` - `colors.vis.euiColorVisAsTextDark6` - Removed `xl` size from `EuiTabs` ([#8762](elastic/eui#8762)) - Added `tooltipProps` to `EuiCopy` which replaces spreading all props to `EuiToolTip` ([#8758](elastic/eui#8758)) <!--ONMERGE {"backportTargets":["8.19"]} ONMERGE--> --------- Co-authored-by: Elastic Machine <[email protected]>
`103.1.0` ⏩ `104.0.2` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) ## Changes - updates flat tooltip prop usages on **EuiCopy** to use new `tooltipProp` ## Package updates ### `@elastic/eui` ## [`v104.0.2`](https://github.com/elastic/eui/releases/v104.0.2) **Bug fixes** - Fixed missing JSON token exports for `euiColorBackgroundBaseInteractiveSelectHover` and `euiColorBorderStrongText` ([elastic#8819](elastic/eui#8819)) ## [`v104.0.1`](https://github.com/elastic/eui/releases/v104.0.1) **Bug fixes** - Fixed a wrong path in the module declaration for the JSON token exports that would trigger typescript errors ([elastic#8818](elastic/eui#8818)) ## [`v104.0.0`](https://github.com/elastic/eui/releases/v104.0.0) - Added data vis text color tokens: ([elastic#8793](elastic/eui#8793)) - `colors.vis.euiColorVisText0` - `colors.vis.euiColorVisText1` - `colors.vis.euiColorVisText2` - `colors.vis.euiColorVisText3` - `colors.vis.euiColorVisText4` - `colors.vis.euiColorVisText5` - `colors.vis.euiColorVisText6` - `colors.vis.euiColorVisText7` - `colors.vis.euiColorVisText8` - `colors.vis.euiColorVisText9` - Updated and aligned background hover styles for `EuiTable` and `EuiDataGrid` ([elastic#8769](elastic/eui#8769)) **Deprecations** - Deprecated `euiPaletteForLightBackground` and `euiPaletteForDarkBackground` palettes. Use the newly added data vis color tokens instead. ([elastic#8793](elastic/eui#8793)) **Breaking changes** - Removed tokens: ([elastic#8793](elastic/eui#8793)) - `colors.vis.euiColorVisAsTextLight1` - `colors.vis.euiColorVisAsTextLight0` - `colors.vis.euiColorVisAsTextLight2` - `colors.vis.euiColorVisAsTextLight3` - `colors.vis.euiColorVisAsTextLight4` - `colors.vis.euiColorVisAsTextLight5` - `colors.vis.euiColorVisAsTextLight6` - `colors.vis.euiColorVisAsTextDark1` - `colors.vis.euiColorVisAsTextDark0` - `colors.vis.euiColorVisAsTextDark2` - `colors.vis.euiColorVisAsTextDark3` - `colors.vis.euiColorVisAsTextDark4` - `colors.vis.euiColorVisAsTextDark5` - `colors.vis.euiColorVisAsTextDark6` - Removed `xl` size from `EuiTabs` ([elastic#8762](elastic/eui#8762)) - Added `tooltipProps` to `EuiCopy` which replaces spreading all props to `EuiToolTip` ([elastic#8758](elastic/eui#8758)) <!--ONMERGE {"backportTargets":["8.19"]} ONMERGE--> --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit efedad1) # Conflicts: # package.json # src/dev/license_checker/config.ts # yarn.lock
## Summary This PR updates styles in tables affecting background colors and hover states, aligning with current design spec. - PR with changes in EUI → elastic/eui#8769 - Additional context behind design decision → elastic/eui#8461 (comment) -⚠️ `2025-08-27` Design spec was [updated to add hover state for stripes](#229928 (comment)) → elastic/eui-private#360 (comment) 🔒 ## Changes - Remove custom background hover color for rows, in both data grids (main and document/table) to leverage EUI's defaults [bed13fe](895c91b) - Use new `backgroundBaseInteractiveSelect` token for background color of expanded rows (when the document flyout is open [9d104c8](b164866) ### Previous Changes (initial iteration, no longer part of the PR) - ~~Remove custom background hover color for rows, in both data grids (main and document/table) bff52d8~~ - ~~Use new `backgroundBaseInteractiveSelect` token for background color of expanded rows (when the document flyout is open) 5a1f1f5~~ - ~~Use new `components.dataGridRowBackgroundMarked` token for background color in surrounding documents page e5528f5~~ ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] ~~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~~ - [ ] ~~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~~ - [ ] ~~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~~ - [ ] ~~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~~ - [ ] ~~This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.~~ - [ ] ~~[Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed~~ - [ ] ~~The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)~~ - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks Only visual changes for colors on table rows, no performance regression risk. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
## Summary This PR updates styles in tables affecting background colors and hover states, aligning with current design spec. - PR with changes in EUI → elastic/eui#8769 - Additional context behind design decision → elastic/eui#8461 (comment) -⚠️ `2025-08-27` Design spec was [updated to add hover state for stripes](#229928 (comment)) → elastic/eui-private#360 (comment) 🔒 ## Changes - Remove custom background hover color for rows, in both data grids (main and document/table) to leverage EUI's defaults [bed13fe](895c91b) - Use new `backgroundBaseInteractiveSelect` token for background color of expanded rows (when the document flyout is open [9d104c8](b164866) ### Previous Changes (initial iteration, no longer part of the PR) - ~~Remove custom background hover color for rows, in both data grids (main and document/table) bff52d8~~ - ~~Use new `backgroundBaseInteractiveSelect` token for background color of expanded rows (when the document flyout is open) 5a1f1f5~~ - ~~Use new `components.dataGridRowBackgroundMarked` token for background color in surrounding documents page e5528f5~~ ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] ~~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~~ - [ ] ~~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~~ - [ ] ~~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~~ - [ ] ~~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~~ - [ ] ~~This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.~~ - [ ] ~~[Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed~~ - [ ] ~~The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)~~ - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks Only visual changes for colors on table rows, no performance regression risk. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
…9928) ## Summary This PR updates styles in tables affecting background colors and hover states, aligning with current design spec. - PR with changes in EUI → elastic/eui#8769 - Additional context behind design decision → elastic/eui#8461 (comment) -⚠️ `2025-08-27` Design spec was [updated to add hover state for stripes](elastic#229928 (comment)) → elastic/eui-private#360 (comment) 🔒 ## Changes - Remove custom background hover color for rows, in both data grids (main and document/table) to leverage EUI's defaults [bed13fe](elastic@895c91b) - Use new `backgroundBaseInteractiveSelect` token for background color of expanded rows (when the document flyout is open [9d104c8](elastic@b164866) ### Previous Changes (initial iteration, no longer part of the PR) - ~~Remove custom background hover color for rows, in both data grids (main and document/table) bff52d8~~ - ~~Use new `backgroundBaseInteractiveSelect` token for background color of expanded rows (when the document flyout is open) 5a1f1f5~~ - ~~Use new `components.dataGridRowBackgroundMarked` token for background color in surrounding documents page e5528f5~~ ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] ~~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~~ - [ ] ~~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~~ - [ ] ~~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~~ - [ ] ~~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~~ - [ ] ~~This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.~~ - [ ] ~~[Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed~~ - [ ] ~~The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)~~ - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks Only visual changes for colors on table rows, no performance regression risk. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
Summary
Closes https://github.com/elastic/eui-private/issues/301
Important
Separate PR to update
border-strong-*token values in Amsterdamis still underwayin draft modeready!Following the design spec, this PR introduces changes to
EuiTableandEuiDataGridEuiDataGridThe update adds a new
colors.backgroundBaseInteractiveSelectHovertoken, to differentiate the hover state for selected rows. And removes hover states completely for the "stripes" version of Data Grid.Demo videos (Borealis/light only)
Data Grid
data-grid-hover.mov
Table
table-hover.mov
Amsterdam
Styles for the Amsterdam theme were left untouched. There is only one small change we can consider a fix: for
EuiTablethe hover state color when there's anonClickhandler applies (it didn't).I have a few doubts regarding the adaptation for the legacy Amsterdam theme:No hover color forstripes=trueseems too breaking. I would keep it. @ek-so what do you think? (@mgadewoll I implemented this in the component code, not CSS; is there an accepted way to check if the theme is Amsterdam?)I think the newbackgroundBaseInteractiveSelectHovercolor is too different frombackgroundBaseInteractiveSelect🤔and the values for SCSS are not exactly matchingThe value forborderStrongPrimarywas updated to keep the selected outline "primary" blue (this should be safe, but I felt like I should mention it) — @ek-so you think this is also OK?QA
General checklist
Checked for accessibility including keyboard-only and screenreader modesProps have proper autodocs (using@defaultif default values are missing) and playground togglesChecked Code Sandbox works for any docs examplesAdded or updated jest and cypress testsIf applicable, added the breaking change issue label (and filled out the breaking change checklist)If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)