Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiRange/EuiDualRange] Add alert icon when isInvalid and showInput #6704

Merged
merged 9 commits into from
Apr 11, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Apr 11, 2023

Summary

I strongly recommend following along by commit as this PR touches and cleans up various sub-components that EuiRange/EuiDualRange rely on.

Dynamic validation based on user input and browser/native :invalid state

1

2

Before/After

before after

before after

QA

General checklist

  • Checked in both light and dark modes
  • Checked in mobile - Note that EuiDualRange was not super great on very small mobile screens even before this PR
  • Checked in Chrome, Safari, Edge, and Firefox
  • Added or updated jest and cypress tests
  • Checked for accessibility including keyboard-only and screenreader modes
  • Updated the Figma library counterpart
  • A changelog entry exists and is marked appropriately

…nstead of text

- to more correctly matches the current Figma designs

+ add a screen reader label
@cee-chen cee-chen requested a review from breehall April 11, 2023 01:40
- color arrow + extend line under arrow and icons

+ fix background colors of icons
…utWithPopover"`

- the input was rendering the padding offset for the invalid icon, without actually rendering said icons (due to `controlOnly` prop)
…ting `aria-invalid`

Browsers natively set their own custom `validity` based on min/max/value/step/etc - we should hook into these and extend them (as opposed to overriding them)

+ switch Jest tests from Enzyme to RTL while here
…lRange

- account for `invalid` icon (which now automatically displays for out of range inputs)

- dynamically adjust width based on # of characters in actual input

- adjust width affordances based on `compressed`

- width changes are especially a readability improvement in Firefox
@kibanamachine
Copy link

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

Copy link
Contributor

@breehall breehall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ This looks good to me! I think we're just missing a snapshot update in src/components/date_picker/super_date_picker/quick_select_popover/quick_select_popover.test.tsx.

Comment on lines +120 to +129
const onKeyDown = useCallback(
(e: React.KeyboardEvent<HTMLInputElement>) => {
_onKeyDown?.(e);
// Wait a beat before checking validity - we can't use `e.target` as it's stale
requestAnimationFrame(() => {
setIsNativelyInvalid(!validityRef.current!.validity.valid);
});
},
[_onKeyDown]
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌟

- after playing more with date range picker as well as a broader variety of delimited inputs, this change was too specific to EuiDualRange
@kibanamachine
Copy link

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

@cee-chen
Copy link
Member Author

Whoops, thanks so much for the snapshot reminder! Saw that this morning and then totally forgot. Much appreciated Bree!

@cee-chen cee-chen enabled auto-merge (squash) April 11, 2023 19:50
@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

mistic pushed a commit to elastic/kibana that referenced this pull request May 5, 2023
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants