[Emotion] Convert EuiDataGrid (Part 1) #7998
Merged
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
cee-chen
force-pushed
the
emotion/datagrid-misc
branch
from
1b3e985
to
d7050d8
Compare
- move it to its own folder, it's a fairly top-level component in the DOM (similar to the controls/toolbar) - simplify some of its rendering logic to a separate util (allows less conditional hooks shenanigans). Also allows us to remove the duplicate `useEuiTablePaginationDefaults()` logic, which is already done once at the topmost EuiDataGrid level - clean up types - no need to have internal props in the top-level types file
+ requires vanilla Emotion CSS/classNames + delete variables, not used anywhere else and can be contained here
+ convert inline styles to logical CSS, clean up CSS slightly
- unfortunately loki doesn't screenshot scrollbars by default, but this at least helps with quick manual cross-browser QA
- Emotion `css` prop actually requires `className` under the hood to merge classes
cee-chen
force-pushed
the
emotion/datagrid-misc
branch
from
d7050d8
to
48de274
Compare
cee-chen
changed the title
💚 Build Succeeded
|
mgadewoll
approved these changes
Sep 5, 2024
There was a problem hiding this comment.
[Pinning this here with loose relation]
The scrollbar tests across browsers went as expected (showing custom scrollbars or default ones) except for Safari which always shows the same static/non-overlaying ones it seemed. But that is Safari 🤷♀️
| @@ -187,6 +188,7 @@ export const EuiDataGridBodyCustomRender: FunctionComponent< | |||
| {...customGridBodyProps} | |||
| className={classNames( | |||
| 'euiDataGrid__customRenderBody', | |||
| className, | |||
There was a problem hiding this comment.
TIL. Thanks for the note on the required className 👍
|
Thank you so much for the quick review Lene! 🙇 |
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
Part of #6394. I'm splitting up EuiDataGrid into smaller chunks for ease of review/QA, but as always, I still recommend following along by commit.
Converts the following miscellaenous parts of EuiDataGrid to Emotion:
QA
Show scroll barssetting toAlwaysAutomatically)General checklist
- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)