[EuiTableRow] Convert to Emotion#7628
Merged
cee-chen merged 9 commits intoelastic:tableflipfrom Mar 28, 2024
Merged
Conversation
- just setting up the responsive context and conditional logic, no styles yet
- make color variables a utility fn to help reduce cruft/setup before the styles obj (there will be more coming)
+ remove now-unnecessary `:hover` override, euiPanel mixin + remove unnecessary extra `euiBottomShadowSmall` on expanded rows, that already gets inherited by `.euiTableRow` - TODO: `cellContentPadding` might need to be DRYed out with `euiTable`, but let's hold off on that for now
- note that we've put `position: relative` on all mobile rows as a default since so many states use it - so no need to apply it to only specific kinds of rows
-⚠️ Opinionatedly remove `$euiTableActionsBorderColor` in favor of just using the border color - I don't see a point at all in making this distinction - prefer a pseudo element over a linear gradient for rendering the 'border' - remove mixins and variables
- note: the `_` prefix before the animation util fn prevents it from getting a label in Emotion's className logic
+ add one mobile fix and one temporary !important workaround found from said QA
cee-chen
commented
Mar 27, 2024
cee-chen
commented
Mar 27, 2024
- table cells will use these shortly, so we might as well make it now
|
Preview staging links for this PR:
|
Collaborator
💚 Build Succeeded
History
|
mgadewoll
reviewed
Mar 28, 2024
Contributor
mgadewoll
left a comment
There was a problem hiding this comment.
I tried as best as possible to follow along what's happening for tables, but I might still miss some general understanding 😅 But overall the changes look good to me, I left a couple smaller notes.
mgadewoll
approved these changes
Mar 28, 2024
Contributor
mgadewoll
left a comment
There was a problem hiding this comment.
🚢 🐈⬛ Nice work on converting the code but also making it cleaner more readable ✨
Contributor
Author
|
Thanks as always for the awesome and thorough PR reviews Lene! 💖 |
cee-chen
added a commit
that referenced
this pull request
Mar 28, 2024
cee-chen
added a commit
that referenced
this pull request
Apr 1, 2024
1 task
cee-chen
added a commit
that referenced
this pull request
Apr 5, 2024
19 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
This doesn't convert all table row styles to Emotion, but it converts (what I consider) to be the majority of row-related CSS (enough to get a mostly functional Storybook up). There are still some styles that I consider belong to cells, rather than rows, which I will target in the next cell-focused PR.
As mentioned in #7625, staging itself will still look broken until all CSS is converted over (at which point I'll do another prod vs staging QA pass and also hopefully have more Storybooks written by then for easier QA).
@euiTableActionsBackgroundMobilemixin$euiTableActionsBorderColorvariable (+ updated mobile usages to use the theme-wide border color token, matching desktop border colors)$euiTableHoverColorvariable$euiTableSelectedColorvariable$euiTableHoverSelectedColorvariable$euiTableHoverClickableColorvariable$euiTableFocusClickableColorvariableQA
General checklist
- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] Added or updated jest and cypress tests- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)