[EuiPopover][EuiInputPopover] Misc fixes and CSS cleanup by cee-chen · Pull Request #7211 · elastic/eui

cee-chen · 2023-09-21T06:12:02Z

Summary

As always, I recommend following along by commit because this PR has a bunch of CSS cleanups as well as fixes.

Missing close animations on standard `EuiPopover`s

This broke in EuiPopover's Emotion conversion (#5977)

Before (popovers disappear immediately on close)	After (close should have a transition)

Fix `EuiInputPopover` flipping to horizontal anchors on screens without sufficient vertical space

This one's been broken since #7071 (my fault!)

Before	After

QA

See above screenshots, feel free to repro on staging vs production

General checklist

Browser QA
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
  ~~- [ ] Checked in both light and dark modes~~
  ~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
Docs site QA - N/A, skipped
Code quality checklist
- Added or updated jest ~~and cypress tests~~
Release checklist
- A changelog entry exists and is marked appropriately.

+ convert `calc()` to `mathWithUnits` + fix missing opacity transition on drag/drop popovers

- that don't have enough enough vertical room to be top/bottom anchored

- has been broken since EuiPopover was converted to Emotion; was working with previous Sass - the transition needs to be permanently present (not just on open) for it to still work on close

- by making our conditional JS cleaner, we don't have to constantly override existing tranform/filter CSS + clarify comments/var names

- now that the default styles no longer have any transforms, this offset isn't needed whatsoever for anchored/input popovers + clean up tests to use specific assertions

- EuiInputPopovers were anchoring to the left/right when they should not be

cee-chen · 2023-09-21T18:13:30Z

Assigned this to @1Copenut because I've been giving Bree a lot of PRs to review lately, but if you have some spare time and feel like this reviewing this feel free @breehall!

1Copenut

👍 LGTM! I learned a new trick about using useMemo() to cache styles between re-renders.

cee-chen · 2023-09-22T15:52:01Z

using useMemo() to cache styles between re-renders.

Just to add a bit more context here, perf wise the memoization is probably negligible, I wanted it more for the the early return syntax (which I personally find much easier to read than else ifs and also usually it's less of a typing headache). But yeah it's nice that we can get a little microperf boost while doing so!

cee-chen · 2023-09-22T15:55:08Z

src/components/popover/popover.tsx

        arrowBuffer: 10,
      },
      returnBoundingBox: this.props.attachToAnchor,
+      allowCrossAxis: !this.props.attachToAnchor,


@1Copenut FYI, I was thinking about this overnight and instead of tying this prop specifically to attachToAnchor, I think I'm going to make a new top-level EuiPopover prop for it and have EuiInputPopover (only) use the prop. There might be a scenario where attached anchors should flex to the cross axes, it's just that input popovers specifically should not

@cee-chen That sounds like a good use case to split this to a top level prop.

4420a7a

Skipped a dedicated docs example for it now as it feels relatively edge case 🤔 attachToAnchor doesn't have a docs section either fwiw, feels like this prop is similar in intention

kibanamachine · 2023-09-22T21:13:29Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7211/
Storybook: https://eui.elastic.co/pr_7211/storybook

elasticmachine · 2023-09-22T21:15:27Z

💚 Build Succeeded

Buildkite Build
Commit: 4420a7a

History

💚 Build #671 succeeded 1449880
💚 Build #667 succeeded d5ccb72

`v88.3.0`⏩`v88.5.0` closes #151514 --- ## [`88.5.0`](https://github.com/elastic/eui/tree/v88.5.0) - Updated `EuiCallOut` with a new `onDismiss` prop ([#7156](elastic/eui#7156)) - Added a new `renderCustomToolbar` prop to `EuiDataGrid`, which allows custom rendering of the toolbar. ([#7190](elastic/eui#7190)) - Added a new `allowResetButton` prop to `toolbarVisibility.showDisplaySelector` of `EuiDataGrid`, which allows hiding the "Reset to default" button from the display settings popover. ([#7190](elastic/eui#7190)) - Added a new `additionalDisplaySettings` prop to `toolbarVisibility.showDisplaySelector` of `EuiDataGrid`, which allows rendering extra settings inside the display settings popover. ([#7190](elastic/eui#7190)) - Updated `EuiDataGrid`'s toolbar display settings button icon ([#7190](elastic/eui#7190)) - Updated `EuiTextTruncate` with significantly improved iteration performance. Removed `measurementRenderAPI` prop, as `EuiTextTruncation` now only uses more performant canvas render API ([#7210](elastic/eui#7210)) - Updated `EuiPopover` with a new configurable `repositionToCrossAxis` prop ([#7211](elastic/eui#7211)) - Updated `EuiDatePicker` to support `compressed` input styling ([#7218](elastic/eui#7218)) - Added `gradient` and `palette` icon glyphs. ([#7220](elastic/eui#7220)) **Bug fixes** - Fixed `EuiPopover`'s missing animations on popover close ([#7211](elastic/eui#7211)) - Fixed `EuiInputPopover` anchoring to the wrong side and missing shadows on smaller screens ([#7211](elastic/eui#7211)) - Fixed `EuiSuperDatePicker` icon spacing on the quick select button ([#7217](elastic/eui#7217)) - Fixed a missing type in `EuiMarkdownEditor`'s default processing plugins ([#7221](elastic/eui#7221)) ## [`88.4.1`](https://github.com/elastic/eui/tree/v88.4.1) **Bug fixes** - Fixed missing `className`s on `EuiTextTruncate` ([#7212](elastic/eui#7212)) - Fixed `title`s on `EuiComboBox` dropdown options to always be present ([#7212](elastic/eui#7212)) - Fixed `EuiComboBox` truncation issues when search is an empty space ([#7212](elastic/eui#7212)) ## [`88.4.0`](https://github.com/elastic/eui/tree/v88.4.0) - Updated `EuiComboBox` to allow configuring text truncation behavior via `truncationProps`. These props can be set on the entire combobox as well as on on individual dropdown options. ([#7028](elastic/eui#7028)) - Updated `EuiInMemoryTable` with a new `searchFormat` prop (defaults to `eql`). When setting this prop to `text`, the built-in search bar will ignore EQL syntax and allow searching for plain strings with special characters and symbols. ([#7175](elastic/eui#7175)) **Bug fixes** - `EuiComboBox` now always shows the highlighted search text, even on truncated text ([#7028](elastic/eui#7028)) - Fixed missing i18n in `EuiSearchBar`'s default placeholder and aria-label text ([#7175](elastic/eui#7175)) - Fixed the inline compressed styles of `EuiDescriptionListTitle` to use a taller line-height for readability ([#7185](elastic/eui#7185)) - Fixed `EuiComboBox` to correctly truncate selected items when displayed as pills and plain text ([#7193](elastic/eui#7193)) **Accessibility** - Added `aria-current` attribute to `EuiTablePagination` ([#7186](elastic/eui#7186)) **CSS-in-JS conversions** - Converted `EuiDroppable` and `EuiDraggable` to Emotion; Removed `$euiDragAndDropSpacing` Sass variables ([#7187](elastic/eui#7187)) --------- Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com> Co-authored-by: Jan Monschke <jan.monschke@elastic.co> Co-authored-by: Thomas Watson <watson@elastic.co>

cee-chen added 10 commits September 20, 2023 20:37

DRY out transition/animation CSS

2874622

+ convert `calc()` to `mathWithUnits` + fix missing opacity transition on drag/drop popovers

[misc] DRY out translateDistance

fb8749a

Fix missing popover shadows on attached popovers

21cb032

- that don't have enough enough vertical room to be top/bottom anchored

Fix missing transition on popover close

d94dcd8

- has been broken since EuiPopover was converted to Emotion; was working with previous Sass - the transition needs to be permanently present (not just on open) for it to still work on close

Clean up unnecessary CSS overrides

8b042b0

- by making our conditional JS cleaner, we don't have to constantly override existing tranform/filter CSS + clarify comments/var names

[misc nit] move unnecessary definition/destructure

fc1ced6

Fix EuiInputPopover offset

b5cd3c7

- now that the default styles no longer have any transforms, this offset isn't needed whatsoever for anchored/input popovers + clean up tests to use specific assertions

Fix attached popovers to not cross axes when space is insufficient

65e5ea0

- EuiInputPopovers were anchoring to the left/right when they should not be

Update downstream popover snapshots

d5ccb72

changelog

1449880

cee-chen marked this pull request as ready for review September 21, 2023 18:07

cee-chen requested a review from a team as a code owner September 21, 2023 18:07

cee-chen requested a review from 1Copenut September 21, 2023 18:07

cee-chen force-pushed the popover-fixes branch from ba16d87 to 1449880 Compare September 21, 2023 18:11

1Copenut approved these changes Sep 22, 2023

View reviewed changes

cee-chen commented Sep 22, 2023

View reviewed changes

[EuiPopover] Add new configurable repositionToCrossAxis prop

4420a7a

cee-chen enabled auto-merge (squash) September 22, 2023 20:45

cee-chen merged commit 3a653d4 into elastic:main Sep 22, 2023

cee-chen deleted the popover-fixes branch September 22, 2023 21:15

cee-chen mentioned this pull request Sep 25, 2023

Upgrade EUI to v88.5.0 elastic/kibana#166868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiPopover][EuiInputPopover] Misc fixes and CSS cleanup#7211

[EuiPopover][EuiInputPopover] Misc fixes and CSS cleanup#7211
cee-chen merged 11 commits intoelastic:mainfrom
cee-chen:popover-fixes

cee-chen commented Sep 21, 2023 •

edited

Loading

Uh oh!

cee-chen commented Sep 21, 2023

Uh oh!

1Copenut left a comment

Uh oh!

cee-chen commented Sep 22, 2023

Uh oh!

cee-chen Sep 22, 2023

Uh oh!

1Copenut Sep 22, 2023

Uh oh!

cee-chen Sep 22, 2023

Uh oh!

kibanamachine commented Sep 22, 2023

Uh oh!

elasticmachine commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

cee-chen commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Missing close animations on standard EuiPopovers

Fix EuiInputPopover flipping to horizontal anchors on screens without sufficient vertical space

QA

General checklist

Uh oh!

cee-chen commented Sep 21, 2023

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen commented Sep 22, 2023

Uh oh!

cee-chen Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

1Copenut Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Sep 22, 2023

Uh oh!

elasticmachine commented Sep 22, 2023

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Sep 21, 2023 •

edited

Loading

Missing close animations on standard `EuiPopover`s

Fix `EuiInputPopover` flipping to horizontal anchors on screens without sufficient vertical space