[EuiPopover] Remove 2nd unnecessary anchor div wrapper #7311

cee-chen · 2023-10-24T19:12:01Z

Summary

closes #2022

My war against unnecessary <div> wrappers will never end 🤘 (Also this cleanup is nice not just for DOM, but also because it removes a bunch of unnecessary extra CSS)

NOTE: Much of the massive diffs are from snapshot changes, and a lot of them go away if you hide whitespace changes. As always, please follow along by commit

Props deprecations:

2 props that were being applied to the now-removed div (anchorClassName and buttonRef) have been moved to the 1st parent div temporarily during the deprecation period (3 months).

Props table:

VSCode jsdoc hint:

Consumer planning

Note that this is almost guaranteed to produce a metric butt-ton of snapshot churn in Kibana, which is why I'd like to combo this with #7310 if possible (to get all the snapshot churn out of the way at once).

There are 8 non-snapshot references to .euiPopover__anchor in Kibana that will need to be updated (link)
There are 2 CSS usages in Cloud that will need to be updated (but can only be updated once they upgrade to the EUI release with this change)
There are no meaningful usages in other Elastic consumers

QA

Regression/smoke-testing

Confirm all popovers https://eui.elastic.co/#/layout/popover look & behave as expected/the same as production
https://eui.elastic.co/pr_7311/#/tabular-content/data-grid Confirm header cells look correct/as expected (right aligned chevrons)
https://eui.elastic.co/pr_7311/#/templates/super-date-picker Confirm the quick select button (in the prepend section) looks as expected (vertically centered)

General checklist

Browser QA
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
  ~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
  ~~- [ ] Checked in both light and dark modes~~
Docs site QA
- Props have proper autodocs ~~(using @default if default values are missing) and **playground toggles~~**
Code quality checklist
- Added or updated jest ~~and cypress tests~~
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
  ⚠️ I opted to not mark the third changelog item as a breaking change, as I personally strongly don't think our DOM markup should be a guaranteed API (and consumers should avoid fragilely querying the DOM whenever possible).
Designer checklist - N/A

- move to outremost div instead

- replace it with `popoverRef` (combined for now until removal)

- pull non-hook logic from `useCombinedRefs` for use within class components - write missing tests

- add display label to Emotion classNames for extra clarity

- use specific `data-test-subj`s instead of generic selectors, and use `simulate` instead of calling `onClick` directly

- make it visually clearer which props are deprecated

kibanamachine · 2023-10-24T21:16:53Z

Preview staging links for this PR:

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

1Copenut

👍 LGTM!

cee-chen · 2023-10-25T21:38:22Z

I'm going to hold off on merging this (and the other popover PR) until after next Monday's release. We have enough larger/breaking changes in main as it is, and fixes that are more important to get into Kibana sooner rather than fighting over potential popover CI failures

cee-chen · 2023-10-30T19:58:25Z

buildkite test this

elasticmachine · 2023-10-30T20:02:35Z

💔 Build Failed

Buildkite Build
Commit: ba1bc01

Failed CI Steps

History

💔 Build #940 failed ba1bc01
💚 Build #888 succeeded 6c62d89
💚 Build #887 succeeded 08c215a
💔 Build #885 failed c39817a

cee-chen · 2023-10-30T20:10:59Z

😬 So it looks like npm is having a very bad no good terrible day right now - I'm going to override the failing CI and force merge this because all I did was fix a very minor type merge conflict, CI was passing before just now

`v90.0.0`⏩`v91.0.0-backport.0` ⚠️ While this upgrade pings many teams and has a large code diff, **the majority of the changes are snapshots or tests-related** and do not touch source code, so should theoretically only need a code review and not dedicated QA. The changes in EUI that required a large swathe of these updates are: - **EuiPopover** removed an extra unnecessary `<div>` wrapper on its anchors, which affected many snapshots and a few CSS overrides, which should have been updated - **EuiButtonGroup** now renders `<button>` elements instead of `<input type="radio">` elements for single selection, which affected both snapshots and E2E tests - **EuiSuperDatePicker**'s absolute date input now requires an `Enter` keypress when parsing dates (affected E2E tests) - **EuiComboBox**, when rendered with `singleSelection={{ plainText: 'true' }}`, no longer renders a pill (i.e. text). This combobox type now behaves more like an `EuiFieldText`, where the selection is rendered via input `value` instead. This affected a high amount of E2E tests (both FTR and Cypress), both in terms of updating assertions and changing selections, but should **not** significantly affect user experience - see elastic/eui#7332 for more. --- ## [`v91.0.0-backport.0`](https://github.com/elastic/eui/tree/v91.0.0-backport.0) **This is a backport release only intended for use by Kibana.** - Added `esqlVis`, `pipeBreaks`, and `pipeNoBreaks` icon glyphs. - `EuiSelectable` now allows configurable text truncation via `listProps.truncationProps` ([#7388](elastic/eui#7388)) - `EuiTextTruncate` now supports a new `calculationDelayMs` prop for working around font loading or layout shifting scenarios ([#7388](elastic/eui#7388)) **Bug fixes** - Fixed a bug with `EuiSelectable`s with custom `truncationProps`, where scrollbar widths were not being accounted for ([#7392](elastic/eui#7392)) ## [`91.0.0`](https://github.com/elastic/eui/tree/v91.0.0) - Updated the background color of `EuiPopover`s in dark mode to increase visibility & contrast against other page/panel backgrounds ([#7310](elastic/eui#7310)) - Memoized `EuiDataGrid` to prevent unneeded re-renders ([#7324](elastic/eui#7324)) - Added a configurable `role` prop to `EuiAccordion` ([#7326](elastic/eui#7326)) - Added a configurable `role` prop to `EuiGlobalToastList` ([#7328](elastic/eui#7328)) - For greater flexibility, `EuiSuperDatePicker` now allows users to paste ISO 8601, RFC 2822, and Unix timestamps in the `Absolute` tab input, in addition to timestamps in the `dateFormat` prop ([#7331](elastic/eui#7331)) - Plain text `EuiComboBox`es now behave more like a normal text field/input. Backspacing will no longer delete the entire value, and selected values can now be double clicked and copied. ([#7332](elastic/eui#7332)) - `EuiDataGrid`'s display settings popover now allows users to clear the "Lines per row" input before typing in a new number ([#7338](elastic/eui#7338)) - Improved the UX of `EuiSuperDatePicker`'s Absolute tab for users manually typing in timestamps ([#7341](elastic/eui#7341)) - Updated `EuiI18n`s with multiple `tokens` to accept dynamic `values` ([#7341](elastic/eui#7341)) **Bug fixes** - Fixed `EuiComboBox`'s `onSearchChange` callback to pass the correct `hasMatchingOptions` value ([#7334](elastic/eui#7334)) - Fixed an `EuiSelectableTemplateSitewide` bug where the `popoverButton` behavior would break if passed a non-DOM React wrapper ([#7339](elastic/eui#7339)) **Deprecations** - `EuiPopover`: deprecated `anchorClassName`. Use `className` instead ([#7311](elastic/eui#7311)) - `EuiPopover`: deprecated `buttonRef`. Use `popoverRef` instead ([#7311](elastic/eui#7311)) - `EuiPopover`: removed extra `.euiPopover__anchor` div wrapper. Target `.euiPopover` instead if necessary ([#7311](elastic/eui#7311)) - Deprecated `EuiButtonGroup`'s `name` prop. This can safely be removed. ([#7325](elastic/eui#7325)) **Breaking changes** - Removed deprecated `euiPaletteComplimentary` - use `euiPaletteComplementary` Instead ([#7333](elastic/eui#7333)) **Accessibility** - Updated `type="single"` `EuiButtonGroup`s to render standard buttons instead of radio buttons under the hood, per recent a11y recommendations ([#7325](elastic/eui#7325)) - `EuiAccordion` now defaults to a less screenreader-noisy `group` role instead of `region`. If your accordion contains significant enough content to be a document landmark role, you may re-configure it back to `region`. ([#7326](elastic/eui#7326)) - Reduced screen reader noisiness when sorting `EuiDataGrid` columns via toolbar ([#7327](elastic/eui#7327)) - `EuiGlobalToastList` now defaults to a `log` role. If your toasts will always require immediate user action, consider (with caution) using the `alert` role instead. ([#7328](elastic/eui#7328)) **CSS-in-JS conversions** - Updated `$euiFontFamily` and `$euiCodeFontFamily` to match Emotion fonts ([#7332](elastic/eui#7332)) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com> Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>

cee-chen force-pushed the popover/dom-cleanup branch from a248648 to c39817a Compare October 24, 2023 19:16

cee-chen mentioned this pull request Oct 24, 2023

[Meta] Deprecation schedule #1469

Open

35 tasks

cee-chen force-pushed the popover/dom-cleanup branch from c39817a to 08c215a Compare October 24, 2023 19:51

cee-chen added 9 commits October 24, 2023 13:47

Deprecate anchorClassName prop

61a782e

- move to outremost div instead

Deprecate buttonRef prop

68b9079

- replace it with `popoverRef` (combined for now until removal)

[cleanup] DRY out multiple refs logic

ec7a1fc

- pull non-hook logic from `useCombinedRefs` for use within class components - write missing tests

Remove 2nd wrapping div

c4ed570

- add display label to Emotion classNames for extra clarity

Update downstream references/usages of deprecated props

16513a1

Update downstream CSS references

14b2f54

Update datagrid tests pointing at old anchor className

b9fb956

- use specific `data-test-subj`s instead of generic selectors, and use `simulate` instead of calling `onClick` directly

Update downstream snapshots

e833945

Improve props table for deprecated props

6c62d89

- make it visually clearer which props are deprecated

cee-chen force-pushed the popover/dom-cleanup branch from 08c215a to 6c62d89 Compare October 24, 2023 20:47

cee-chen marked this pull request as ready for review October 24, 2023 20:53

cee-chen requested a review from a team as a code owner October 24, 2023 20:53

1Copenut approved these changes Oct 25, 2023

View reviewed changes

Merge branch 'main' into popover/dom-cleanup

ba1bc01

cee-chen enabled auto-merge (squash) October 30, 2023 19:30

cee-chen disabled auto-merge October 30, 2023 20:11

cee-chen merged commit be84bc7 into elastic:main Oct 30, 2023
4 of 7 checks passed

cee-chen deleted the popover/dom-cleanup branch October 30, 2023 20:11

tkajtoch mentioned this pull request Nov 7, 2023

Upgrade EUI to v91.0.0 (with backports) elastic/kibana#170716

Merged

cee-chen mentioned this pull request Nov 27, 2023

WIP elastic/kibana#172021

Closed

cee-chen mentioned this pull request Jan 26, 2024

[EuiPopover] Deprecate anchorClassName and buttonRef #7488

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiPopover] Remove 2nd unnecessary anchor div wrapper #7311

[EuiPopover] Remove 2nd unnecessary anchor div wrapper #7311

cee-chen commented Oct 24, 2023 •

edited

Loading

kibanamachine commented Oct 24, 2023

1Copenut left a comment

cee-chen commented Oct 25, 2023

cee-chen commented Oct 30, 2023

elasticmachine commented Oct 30, 2023 •

edited

Loading

cee-chen commented Oct 30, 2023 •

edited

Loading

[EuiPopover] Remove 2nd unnecessary anchor div wrapper #7311

[EuiPopover] Remove 2nd unnecessary anchor div wrapper #7311

Conversation

cee-chen commented Oct 24, 2023 • edited Loading

Summary

Props deprecations:

Consumer planning

QA

General checklist

kibanamachine commented Oct 24, 2023

1Copenut left a comment

Choose a reason for hiding this comment

cee-chen commented Oct 25, 2023

cee-chen commented Oct 30, 2023

elasticmachine commented Oct 30, 2023 • edited Loading

💔 Build Failed

Failed CI Steps

History

cee-chen commented Oct 30, 2023 • edited Loading

cee-chen commented Oct 24, 2023 •

edited

Loading

elasticmachine commented Oct 30, 2023 •

edited

Loading

cee-chen commented Oct 30, 2023 •

edited

Loading