[EuiRange][EuiDualRange] Respond to resizing/layout changes #7442

cee-chen · 2024-01-03T00:46:29Z

Summary

This PR adds a resize observer to both EuiRange and EuiDualRange components, instead of only obtaining the trackWidth/rangeWidth on ref mount. TBH, I probably should have taken this approach in #7305 in the first place, as it allows us to simplify a good chunk of logic in EuiDualRange

Before	After

QA

Go to https://eui.elastic.co/pr_7442/#/forms/range-sliders
Scroll down to the first demo on the page
Confirm that the range ticks are correctly positioned on accordion option
Scroll down to the last demo on the page (kitchen sink)
Click the Display toggles buttons and toggle both to fullWidth
Confirm that both range ticks, highlights, and thumbnails update correctly on width change

General checklist

Revert [REVERT ME] commit
Browser QA
- Checked in Chrome, Safari, Edge, and Firefox
  ~~- [ ] Checked in both light and dark modes~~
  - [ ] Checked in mobile
  ~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
Docs site QA - N/A, bugfix
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)~~
Designer checklist - N/A

+ update parent range components to use the `onResize` callback instead of a ref callback

- the internal resize observer handles all of it, no need for all this extra shenanigans - probably should've taken this approach in the first place - remove `onResize` class ref - should no longer be needed in Kibana

- Honestly not sure how useful this is, but better to have something than nothing - we should eventually replace with with visual snapshot tests

cee-chen · 2024-01-03T01:23:22Z

src/components/form/range/dual_range.test.tsx

-    // Whether we like it or not, at least 2 Kibana instances are using EuiDualRange
-    // `ref`s to access the `onResize` instance method (search for `rangeRef.current?.onResize`)
-    // If we switch EuiDualRange to a function component, we'll need to use `useImperativeHandle`
-    // to allow Kibana to continue calling `onResize`


I double checked this usage in Kibana, and there's just 1 left, and it should be completely removable now that we've added our own resize observer

cee-chen · 2024-01-03T01:23:53Z

src/components/form/range/dual_range.tsx

-                }
-              />
-            </>
+          {isDraggable && this.isValid && (


This is primarily just indentation changes - I recommend viewing the diff with whitespace changes turned off

cee-chen · 2024-01-03T01:25:32Z

src/components/form/range/range.spec.tsx

+  };
+
+  // TODO: These should likely be visual snapshot regression tests instead
+  const assertRangePositions = () => {


I don't love these tests and personally think they're pretty tedious/fragile and not much better than snapshots, but they'll hopefully at least help us catch any weird positioning regressions like #7304. IMO, ideally we'd replace them with visual regression tests instead

1Copenut

👍 LGTM! I tested the QA parameters in the four evergreen browsers and tested on Chorme using the mobile preview and scrolled it to different widths to see how it'd react.

This reverts commit 7206b15.

cee-chen · 2024-01-03T18:49:15Z

Woohoo! Thanks Trevor!

kibanamachine · 2024-01-03T19:12:07Z

Preview staging links for this PR:

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

elasticmachine · 2024-01-03T19:12:10Z

💚 Build Succeeded

Buildkite Build
Commit: 4672131

History

💚 Build #1203 succeeded ecb814a
💚 Build #1202 succeeded 67360ae

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([#7435](elastic/eui#7435))

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([elastic#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([elastic#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([elastic#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([elastic#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([elastic#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([elastic#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([elastic#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([elastic#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([elastic#7435](elastic/eui#7435))

cee-chen added 4 commits January 2, 2024 14:43

[REVERT ME] issue repro

7206b15

Add built in resize observer to internal range slider component

700a618

+ update parent range components to use the `onResize` callback instead of a ref callback

[EuiDualRange] remove now-unnecessary resize/width logic

8488c13

- the internal resize observer handles all of it, no need for all this extra shenanigans - probably should've taken this approach in the first place - remove `onResize` class ref - should no longer be needed in Kibana

Add regression test for expected positioning

67360ae

- Honestly not sure how useful this is, but better to have something than nothing - we should eventually replace with with visual snapshot tests

cee-chen changed the title ~~[EuiRange][EuiDualRange] Respond to layout changes/~~ [EuiRange][EuiDualRange] Respond to resizing/layout changes Jan 3, 2024

changelog

ecb814a

cee-chen marked this pull request as ready for review January 3, 2024 01:22

cee-chen requested a review from a team as a code owner January 3, 2024 01:22

cee-chen commented Jan 3, 2024

View reviewed changes

1Copenut approved these changes Jan 3, 2024

View reviewed changes

Revert "[REVERT ME] issue repro"

4672131

This reverts commit 7206b15.

cee-chen enabled auto-merge (squash) January 3, 2024 18:50

cee-chen merged commit 971fea2 into elastic:main Jan 3, 2024
7 checks passed

cee-chen deleted the range-resize-fix branch January 3, 2024 19:12

cee-chen mentioned this pull request Jan 8, 2024

Upgrade EUI to v92.0.0 elastic/kibana#174487

Merged

e40pud mentioned this pull request Jan 10, 2024

[Security Solution] Default Risk score slide bar values are overlaid on Rule Creation page elastic/kibana#161456

Closed

vitaliidm mentioned this pull request Apr 12, 2024

[Security Solution]Anomaly score threshold slide bar values are overlaid on Rule Creation page when creating a new Machine Learning rule type elastic/kibana#173669

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiRange][EuiDualRange] Respond to resizing/layout changes #7442

[EuiRange][EuiDualRange] Respond to resizing/layout changes #7442

cee-chen commented Jan 3, 2024 •

edited

Loading

cee-chen Jan 3, 2024

cee-chen Jan 3, 2024

cee-chen Jan 3, 2024

1Copenut left a comment

cee-chen commented Jan 3, 2024

kibanamachine commented Jan 3, 2024

elasticmachine commented Jan 3, 2024

[EuiRange][EuiDualRange] Respond to resizing/layout changes #7442

[EuiRange][EuiDualRange] Respond to resizing/layout changes #7442

Conversation

cee-chen commented Jan 3, 2024 • edited Loading

Summary

QA

General checklist

cee-chen Jan 3, 2024

Choose a reason for hiding this comment

cee-chen Jan 3, 2024

Choose a reason for hiding this comment

cee-chen Jan 3, 2024

Choose a reason for hiding this comment

1Copenut left a comment

Choose a reason for hiding this comment

cee-chen commented Jan 3, 2024

kibanamachine commented Jan 3, 2024

elasticmachine commented Jan 3, 2024

💚 Build Succeeded

History

cee-chen commented Jan 3, 2024 •

edited

Loading