[EuiFieldNumber] Clean up native `:invalid` detection #6741

cee-chen · 2023-04-27T21:00:37Z

Summary

For some arcane React reason, our EuiFieldNumber change in 37c35ea (that added detection of native browser :invalid state) is causing extra rerender cycles and redux sync issues with the Lens team, which is reproducibly causing a failure/bug in the latest Kibana upgrade: elastic/kibana#155208

After a bit of debugging with Drew and determination of what can and can't be fixed on their end, I spiked out reverting the isNativeInvalid functionality and confirmed that seemed to fix the issue on my local Kibana. I then experimented with rewriting the native detection logic to try and reduce vectors that could be causing the extra rerender, and... I think it worked??

Honestly, I'm not sure why this is happening. It could be the switch to onKeyUp, it could be the removal of refs (implying useCombinedRefs was somehow problematic? 😕), it could have been the requestAnimationFrame. The code is definitely cleaner now though, so all in all I think it's a win?... 😅

QA

Check out Upgrade EUI to v77.2.2 kibana#155208
Run yarn build-pack
Go through the testing in Kibana steps to point the EUI at the built tgz
Go to local Kibana
- Create a Lens visualization
- Drag @timestamp to the middle
- Click the "Add layer" button
- Select Annotations
- Click the annotation "Events" button
- Under "Placement type", select "Custom query"
- Under the empty "Annotation query", type *
- Confirm the * on the non-popover-input does not go away
- Close the popover
- Under "Target date field", select any other option
- Confirm the annotations do not disappear from the page

General checklist

CI/all previous tests pass as before (invalid functionality was retained)
Checked in Chrome, Safari, Edge, and Firefox
A changelog entry exists and is marked appropriately

- switching to onKeyUp instead of onKeyDown allows us to use `e.target` without it being stale, & also removes the need for a rAF

- so that the `aria-invalid` prop only appears when invalid - consumers passing `isInvalid={false}` should still see `aria-invalid="false"`

kibanamachine · 2023-04-27T21:29:46Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6741/

1Copenut

👍 LGTM!

cee-chen · 2023-04-27T21:50:58Z

Me pressing merge on this PR:

kibanamachine · 2023-04-27T22:16:20Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6741/

## Summary `eui@77.1.1` ⏩ `eui@77.2.2` Closes elastic/eui#6724 --- ## [`77.2.2`](https://github.com/elastic/eui/tree/v77.2.2) - Updated `EuiFocusTrap` to support the `gapMode` prop configuration (now defaults to `padding`) ([#6744](elastic/eui#6744)) **Bug fixes** - Fixed the `scrollLock` property on `EuiFocusTrap` (and other components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to no longer block scrolling on nested portalled content, such as combobox dropdowns ([#6744](elastic/eui#6744)) ## [`77.2.1`](https://github.com/elastic/eui/tree/v77.2.1) **Bug fixes** - Fixed `EuiFieldNumber`'s native browser validity detection causing extra unnecessary rerenders ([#6741](elastic/eui#6741)) ## [`77.2.0`](https://github.com/elastic/eui/tree/v77.2.0) - Updated `EuiFieldNumber` to detect native browser invalid state and show an invalid icon ([#6704](elastic/eui#6704)) - Improved the input widths of `EuiRange` and `EuiDualRange` when `showInput={true}` to account for invalid icons ([#6704](elastic/eui#6704)) - Improved the `isInvalid` styling of `EuiDualRange` when `showInput="inputWithPopover"` ([#6704](elastic/eui#6704)) - Updated `EuiFormControlLayoutIcons` to render left icons in expected DOM order ([#6705](elastic/eui#6705)) - Updated `EuiDatePickerRange`'s `isInvalid` state to match other range inputs ([#6705](elastic/eui#6705)) - Updated `EuiSuperDatePicker`'s `isInvalid` state to match other range inputs ([#6705](elastic/eui#6705)) **Bug fixes** - Fixed `EuiValidatableControl` to correctly display `isInvalid` states on mount ([#6705](elastic/eui#6705)) - Fixed an issue with `EuiSearchBar` where quoted phrases were not quoted when generating an Elasticsearch query. ([#6714](elastic/eui#6714)) --------- Co-authored-by: Constance Chen <constance.chen.3@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Constance Chen <constance.chen@elastic.co>

## Summary `eui@77.2.2` ⏩ `eui@79.0.1` 🦴 The primary changes in this upgrade are around the deprecated `EuiLoadingContent` being removed in favor of `EuiSkeletonText`. - Most instances have been a [direct swap of usage](327626a), but [some replacements were a bit more opinionated](e6ceb36) as I saw them as potential to take advantage of `EuiSkeletonText`'s syntactical sugar and screen reader announcements for when state switches to loaded. --- ## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1) **Bug fixes** - Fixed broken push `EuiFlyout` behavior ([#6764](elastic/eui#6764)) ## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0) - Updated all `EuiSkeleton` components with new props that allow for more control over screen reader live announcements: `announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps` ([#6752](elastic/eui#6752)) - Improved keyboard accessibility in `EuiPageHeader` by ensuring the right side menu items come into focus from left to right. ([#6753](elastic/eui#6753)) **Breaking changes** - Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton` components instead. ([#6754](elastic/eui#6754)) ## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0) - Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and `EuiSwitch` in their unchecked states to meet WCAG AA guidelines. ([#6729](elastic/eui#6729)) - Added React Testing Library `*ByTestSubject` custom commands to `within()`. RTL utilities can be imported from `@elastic/eui/lib/test/rtl`. ([#6737](elastic/eui#6737)) - Updated `EuiAvatar` to support a new letter `casing` prop that allow customizing text capitalization ([#6739](elastic/eui#6739)) - Updated `EuiFocusTrap` to support the `gapMode` prop configuration (now defaults to `padding`) ([#6744](elastic/eui#6744)) **Bug fixes** - Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL and query string generation ([#6717](elastic/eui#6717)) - Fixed `EuiFieldNumber`'s native browser validity detection causing extra unnecessary rerenders ([#6741](elastic/eui#6741)) - Fixed the `scrollLock` property on `EuiFocusTrap` (and other components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to no longer block scrolling on nested portalled content, such as combobox dropdowns ([#6744](elastic/eui#6744)) **Breaking changes** - `EuiAvatar`s with the default `user` type will now default to capitalizing all initials in uppercase ([#6739](elastic/eui#6739)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

cee-chen added 2 commits April 27, 2023 13:41

Fix EuiFieldNumber to no longer need a combined ref

29f1cf2

- switching to onKeyUp instead of onKeyDown allows us to use `e.target` without it being stale, & also removes the need for a rAF

[misc] Prefer undefined for isNativeInvalid

2dffd5f

- so that the `aria-invalid` prop only appears when invalid - consumers passing `isInvalid={false}` should still see `aria-invalid="false"`

cee-chen requested a review from 1Copenut April 27, 2023 21:00

changelog

4f2ffa9

cee-chen marked this pull request as ready for review April 27, 2023 21:46

1Copenut approved these changes Apr 27, 2023

View reviewed changes

cee-chen merged commit f628d41 into elastic:main Apr 27, 2023

cee-chen deleted the number-invalid-cleanup branch April 27, 2023 21:51

cee-chen mentioned this pull request Apr 27, 2023

Upgrade EUI to v77.2.2 elastic/kibana#155208

Merged

cee-chen mentioned this pull request Apr 27, 2023

[Lens] assist EUI with bug investigation to get to version 77.2.0 elastic/kibana#155890

Closed

This was referenced May 5, 2023

Upgrade EUI to v79.0.1 elastic/kibana#156925

Merged

[EuiFieldNumber] Default to step="any" and check native validity on blur #6760

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiFieldNumber] Clean up native `:invalid` detection #6741

[EuiFieldNumber] Clean up native `:invalid` detection #6741

cee-chen commented Apr 27, 2023 •

edited

Loading

kibanamachine commented Apr 27, 2023

1Copenut left a comment

cee-chen commented Apr 27, 2023

kibanamachine commented Apr 27, 2023

[EuiFieldNumber] Clean up native :invalid detection #6741

[EuiFieldNumber] Clean up native :invalid detection #6741

Conversation

cee-chen commented Apr 27, 2023 • edited Loading

Summary

QA

General checklist

kibanamachine commented Apr 27, 2023

1Copenut left a comment

Choose a reason for hiding this comment

cee-chen commented Apr 27, 2023

kibanamachine commented Apr 27, 2023

[EuiFieldNumber] Clean up native `:invalid` detection #6741

[EuiFieldNumber] Clean up native `:invalid` detection #6741

cee-chen commented Apr 27, 2023 •

edited

Loading