[Emotion] Convert EuiContextMenu, EuiContextMenuPanel, and EuiContextMenuItem#7312
Merged
cee-chen merged 25 commits intoelastic:mainfrom Oct 25, 2023
Merged
[Emotion] Convert EuiContextMenu, EuiContextMenuPanel, and EuiContextMenuItem#7312cee-chen merged 25 commits intoelastic:mainfrom
cee-chen merged 25 commits intoelastic:mainfrom
Conversation
- not used anwhere as in either EUI or Kibana
- inline it since it's pretty basic
+ remove unused `--next`/`--previous` classes
…PanelTitle` - by dogfooding `EuiContextMenuItem` - this requires updating `EuiContextMenuItem`'s logic to be able to render a `div` if necessary - we can now remove the `padding` variables, since `EuiContextMenuItem` is now the only one that uses it
- due to the previous change, they'd otherwise render as `div`s and not register within the tab order
+ syntax cleanup - prefer `const` and `&&` over `let` and `if`
- we're applying these styles directly via Emotion in the child components instead of at the parent
- add `shouldRenderCustomStyles` - convert all tests to RTL - convert snapshots to assertions where it makes sense to do so
- Fix class assertion failures caused by new Emotion classes (convert `have.attr, class`->`have.class`) - Fix focus failures caused by items without `href`s or `onClick`s rendering as `div`s
- due to the way the component is currently set up with `onAnimationEnd`, setting a 0s animation duration is the easiest way to reduce motion
3c72253 to
7c2c46f
Compare
|
Preview staging links for this PR:
|
Collaborator
💚 Build Succeeded
|
1Copenut
approved these changes
Oct 25, 2023
Contributor
1Copenut
left a comment
There was a problem hiding this comment.
👍 LGTM! I ran through your manual QA criteria on all four evergreen browsers in light and dark themes. No issues except the slightly shorter context menu title as noted in the description.
This was referenced Oct 30, 2023
cee-chen
added a commit
to elastic/kibana
that referenced
this pull request
Nov 3, 2023
`v89.1.0`⏩`v90.0.0` The majority of changes in this PR come from: - **EuiContextMenu** being converted to Emotion (elastic/eui#7312). If your usage of `EuiContextMenu` was significantly affected, we recommend pulling down this PR and QAing it locally. - `defaultProps` being removed from some very widespread components, particularly **EuiButton**, in anticipation of React's upcoming deprecation. (elastic/eui@b7dc9b4) **NOTE**: This only affected Enzyme snapshots, and did not affect production behavior. [Commits](https://github.com/elastic/kibana/pull/170179/commits) have been broken up by component changes as well as types of changes. --- ## [`90.0.0`](https://github.com/elastic/eui/tree/v90.0.0) - Updated the `eventColor` prop on `EuiCommentEvent` to apply the color to the entire comment header. ([#7288](elastic/eui#7288)) - Updated `EuiBasicTable` and `EuiInMemoryTable` to support a new controlled selection API: `selection.selected` ([#7321](elastic/eui#7321)) **Bug fixes** - Fixed controlled `EuiFieldNumbers` not correctly updating native validity state ([#7291](elastic/eui#7291)) - Fixed `EuiListGroupItem` to pass `style` props to the wrapping `<li>` element alongside `className` and `css`. All other props will be passed to the underlying content. ([#7298](elastic/eui#7298)) - Fixed `EuiListGroupItem`'s non-transitioned transform on hover/focus ([#7298](elastic/eui#7298)) - Fixed `EuiDataGrid`s with `gridStyle.stripes` sometimes showing buggy row striping after being sorted ([#7301](elastic/eui#7301)) - Fixed `EuiDataGrid`'s `gridStyle.rowClasses` API to not conflict with `gridStyle.stripes` if dynamically updated ([#7301](elastic/eui#7301)) - Fixed `EuiDataGrid`'s `gridStyle.rowClasses` API to support multiple space-separated classes ([#7301](elastic/eui#7301)) - Fixed `EuiInputPopover` not calling `onPanelResize` callback prop ([#7305](elastic/eui#7305)) - Fixed `EuiDualRange` incorrectly positioning highlights when rendered with `showInput="inputWithPopover"` ([#7305](elastic/eui#7305)) - Fixed `EuiTabs` incorrectly wrapping text when it should instead either scroll or truncate ([#7309](elastic/eui#7309)) - `EuiContextMenu` now renders text colors correctly when used within an `EuiBottomBar` ([#7312](elastic/eui#7312)) - Fixed the width of `EuiSuperDatePicker`'s Absolute date picker ([#7313](elastic/eui#7313)) - Fixed `EuiDataGrid` cells visually cutting off overflowing content a little too quickly ([#7320](elastic/eui#7320)) **Deprecations** - Deprecated `EuiBasicTable` and `EuiInMemoryTable`'s ref `setSelection` API. Use the new `selection.selected` API instead. ([#7321](elastic/eui#7321)) **Breaking changes** - Removed `EuiPageTemplate_Deprecated`, `EuiPageSideBar_Deprecated`, and `EuiPageContent*_Deprecated` ([#7265](elastic/eui#7265)) - Removed the `ghost` color option from `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon`. Use an `<EuiThemeProvider colorMode="dark">` wrapper and `color="text"` instead. ([#7296](elastic/eui#7296)) **Dependency updates** - Updated `refractor` to v3.6.0 ([#7127](elastic/eui#7127)) - Updated `rehype-raw` to v5.1.0 ([#7127](elastic/eui#7127)) - Updated `vfile` to v4.2.1 ([#7127](elastic/eui#7127)) **Accessibility** - `EuiContextMenu` now correctly respects reduced motion preferences ([#7312](elastic/eui#7312)) - `EuiAccordion`s no longer attempt to focus child content when the accordion is externally opened via `forceState`, but will continue to focus expanded content when users click the toggle button. ([#7314](elastic/eui#7314)) **CSS-in-JS conversions** - Converted `EuiContextMenu`, `EuiContextMenuPanel`, and `EuiContextMenuItem` to Emotion; Removed `$euiContextMenuWidth` ([#7312](elastic/eui#7312))
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
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
closes #6392
Converting this component to CSS-in-JS also will also address elastic/kibana#169571 and elastic/kibana#169313 (comment) (once merged into Kibana main).
This is kind of a lot to review (especially tests, which I went in and edited judiciously), so I think I'd recommend sticking to QA only for reviewing this PR (as regression/QA testing is more important here)
Also while here, I removed an extra unnecessary
<div>wrapper withinEuiContextMenuItem. Div wrappers left me an orphan in a dark alley at a young age, and this is how I seek justice.QA
titles are now slightly less tall than before. This is expected, and due to changes inline-heightfrom neweuiTitleEmotion styles.HelpbuttonGeneral checklist
- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)Emotion checklist
Kibana usage
**/target, **/*.snap, **/*.storyshotfor less noise) foreui{Component}(case sensitive) to find:{euiComponent}-(case sensitive) to check for usage of modifier classes - NoneeuiBadgeclass on a div instead of simply using theEuiBadgecomponent)General
className(s)read as expected in snapshots and browsers[ ] Checked component playgroundNo playgroundUnit tests
shouldRenderCustomStyles()test was added and passes with parent component and any nestedchildProps(e.g.tooltipProps)Sass/Emotion conversion process
$euiSizetoeuiTheme.size.base)or convertedcomponent-specific Sass vars/mixinsto exported JS versions[ ] Simplifiedcalc()tomathWithUnitsif possible (if mixing different unit types, this may not be possible)[ ] Added an@warndeprecation message within theglobal_styling/mixins/{component}.scssfile[ ] All SCSS overrides have been removed from the Amsterdam themeNo overridesCSS tech debt
euiCanAnimategapproperty to add margin between items if using flex-inlineand-blocklogical properties (check inline styles as well as CSS)DOM Cleanup
euiComponent,euiComponent__child) - Note: removed an entire child node/div wrapperExtras/nice-to-have
[ ] Documentation pass[ ] Check for issues in the backlog that could be a quick fix for that component[ ] Optional component/code cleanup: consider splitting up the component into multiple children if it's overly verbose or difficult to reason about