[Emotion] Convert EuiModal#6321
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
…ith `image-mask`
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
1 similar comment
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
| } | ||
| `; | ||
|
|
||
| export const euiAnimSlideInUp = (size: string) => keyframes` |
There was a problem hiding this comment.
I noticed there are a few components using this animation (e.g. EuiImage). So I moved the animation here.
There are more components using this animation. So I might follow-up with a PR.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
| padding-inline: ${euiTheme.size.l} ${euiTheme.size.xxl}; | ||
| padding-block: ${euiTheme.size.l} ${euiTheme.size.base}; | ||
| ${euiTheme.size.l}; |
There was a problem hiding this comment.
[Just thinking out loud] This padding conversion was a little hard to follow for me personally (particular the inline one cause its directions are flipped). At the same time I super appreciate that we're using inline and block over padding shorthand, which doesn't yet have a corresponding logical property 😍
I'm very tempted to add a logical util helper for this in our own codebase. Something that just automatically handles this for us, e.g. logicalShorthandCSS('padding', '1 2 3 4'). I might explore that in a separate PR and send it your way to see if you like it!
There was a problem hiding this comment.
Also just wanted to add - that third ${euiTheme.size.l}; line looks like an misplaced copy-paste - will go ahead and clean it up
| padding-block: ${euiTheme.size.m}; | ||
| padding-inline: ${euiTheme.size.l}; | ||
| justify-content: stretch; | ||
| gap: ${euiTheme.size.s}; |
There was a problem hiding this comment.
Assuming this new gap was a design change? Love it! ❤️
| // The below fixes scroll on Chrome and Safari | ||
| display: flex; | ||
| flex-direction: column; |
There was a problem hiding this comment.
[not a change request, just me thinking out loud] I wonder if this fix is still needed. Maybe worth investigating later in latest Chrome/Safari?
| } | ||
| `, | ||
| euiModal__closeIcon: css` | ||
| position: absolute; |
There was a problem hiding this comment.
Looks like we removed a background-color on this - just to confirm, it looks like that's because EuiButton's bg color was already overriding it in Amsterdam, correct? 👍
- to match `defaultMinWidth` modifier, used in EuiButtonDisplay (elastic#6373)
cfaec3d to
d73eaf2
Compare
cee-chen
left a comment
There was a problem hiding this comment.
🎉 This is looking good to me - QA'd all modal examples on Firefox, Safari, and Edge!
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6321/ |
|
I did a quick grep in Kibana for However, I kept coming across this weird reference to https://github.com/search?q=repo%3Aelastic%2Fkibana%20euiModal__flex&type=code I searched through EUI main and we definitely don't have that class set or defined anywhere so I have no idea what these folks are hooking into (I guess a really old version of EuiModal?? But some of those git blames are fairly recent 🙈). We could either try to remove the references totally (they're not working anyway), try to convert them to actual working selectors, or leave them alone (status quo). Any thoughts @miukimiu? |
|
@constancecchen we removed that class in https://github.com/elastic/eui/pull/6161/files#diff-4c5d8e9013780b8dc443258a62175a2548e5fce53325e705ff6b9aa4b9275d6e. I think we can convert them to actual working selectors. |
`eui@70.2.4` ⏩ `eui@70.4.0` - "Fixed EuiButtonGroup firing onChange twice" required changing some tests from `click` to `change` ___ ## [`70.4.0`](https://github.com/elastic/eui/tree/v70.4.0) - Updated `EuiTourStep.footerAction` type to accept `ReactNode[]` ([#6384](elastic/eui#6384)) - Vertically aligned all footer content so that `euiTourStepIndicator` is always centered ([#6384](elastic/eui#6384)) - Added `filterInCircle` glyph to `EuiIcon` ([#6385](elastic/eui#6385)) - Added `color` prop to `EuiBeacon` ([#6420](elastic/eui#6420)) - Added the `euiMaxBreakpoint` and `euiMinBreakpoint` CSS-in-JS utilities for creating min/max-width media queries ([#6431](elastic/eui#6431)) **Bug fixes** - Restores the previous match operator behaviour when the query value is split into multiple terms after analysis. ([#6409](elastic/eui#6409)) - Fixed missing slide-in animation on `EuiCollapsibleNav`s and left-side `EuiFlyout`s ([#6422](elastic/eui#6422)) - Fix bug in `EuiCard` where footer were not aligned to the bottom of the card ([#6424](elastic/eui#6424)) - Fixed multiple component media queries for consumers with custom theme breakpoints ([#6431](elastic/eui#6431)) ## [`70.3.0`](https://github.com/elastic/eui/tree/v70.3.0) - `EuiSearchBar` now automatically wraps special characters not used by query syntax in quotes ([#6356](elastic/eui#6356)) - Added `alignment` prop to `EuiBetaBadge` ([#6361](elastic/eui#6361)) - `EuiButton` now accepts `minWidth={false}` ([#6373](elastic/eui#6373)) **Bug fixes** - Fixed `EuiPageTemplate` not correctly passing the `component` prop to the inner main content wrapper. ([#6352](elastic/eui#6352)) - `EuiSkipLink` now correctly calls `onClick` even when `fallbackDestination` is invalid ([#6355](elastic/eui#6355)) - Permanently fixed `EuiModal` to not cause scroll-jumping issues on modal open ([#6360](elastic/eui#6360)) - Re-fixed `EuiPageSection` not correctly merging `contentProps.css` ([#6365](elastic/eui#6365)) - Fixed `EuiTab` not defaulting to size `m` ([#6366](elastic/eui#6366)) - Fixed the shadow sizes of `.eui-yScrollWithShadows` and `.eui-xScrollWithShadows` ([#6374](elastic/eui#6374)) - Fixed bug in `EuiCard` where the inner content in vertical cards was not growing 100% in width ([#6377](elastic/eui#6377)) - Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex` CSS gap change ([#6380](elastic/eui#6380)) - Fixed visual bug in nested `EuiFlexGroup`s, where the parent `EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not ([#6381](elastic/eui#6381)) **CSS-in-JS conversions** - Converted `EuiModal` to Emotion ([#6321](elastic/eui#6321)) **Fixes** - `EuiButton` no longer outputs unnecessary inline styles for `minWidth={0}` or `minWidth={false}` ([#6373](elastic/eui#6373)) - `EuiFacetButton` no longer reports type issues when passing props accepted by `EuiButton` ([#6373](elastic/eui#6373)) Co-authored-by: Constance Chen <constance.chen@elastic.co>
Summary
This PR converts
EuiModalto Emotion.QA
Remove or strikethrough items that do not apply to your PR.
General checklist
[ ] Added documentation[ ] Checked Code Sandbox works for any docs examples[ ] Checked for breaking changes and labeled appropriately[ ] Updated the Figma library counterpartEmotion conversion checklist
- [ ] Checked component playground (class components wrapped inwithEuiThemeneed to passtrueas the second argument to itspropUtilityForPlaygroundinsrc-docs/src/views/{component}/playground.js)shouldRenderCustomStyles()test was added and passes with parent component and any nestedchildProps(e.g.tooltipProps)- [ ] Removed anymount()ed snapshots in favor ofrender()or a more specific assertion- [ ] Converted all global Sass vars/mixins to JS (e.g.$euiSizetoeuiTheme.size.base)- [ ] Removed or converted component-specific Sass vars/mixins to exported JS versions, listed removals in changelog, and ranyarn compile-scssto update JSON filescalc()tomathWithUnitsif possible (if mixing different unit types, this may not be possible)- [ ] Added an@warndeprecation message within theglobal_styling/mixins/{component}.scssfilesrc/components/index.scsssrc/amsterdam/overrides/{component}.scssfiles (styles within should have been converted to the baseline Emotion styles)euiCanAnimategapproperty to add margin between items if using flex-inlineand-blocklogical properties (check inline styles as well as CSS)euiComponent,euiComponent__child){euiComponent}-(case sensitive) to check for source code usage of modifier classes- [ ] If usages exist, consider converting to adataattribute so that consumers still have something to hook into**/target, **/*.snap, **/*.storyshotfor less noise) foreui{Component}(case sensitive) to find:- [ ] Any Sass or CSS that will need to be updated, particularly if a component Sass var was deletedeuiBadgeclass on a div instead of simply using theEuiBadgecomponent).jsdocs files to TSfrom '../src', replace<React.Fragment>with<>)- [ ] 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