[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood `EuiFormControlLayoutDelimited` #6705

cee-chen · 2023-04-12T02:18:56Z

Summary

Converting the date range picker components to dogfood EuiFormControlLayoutDelimited accomplishes the following:

Makes form control styling more consistent, primarily isInvalid, isLoading, etc.
DRYs out code
DRYs out styles

This PR also cleans up EuiFormControlLayout's positioning and side logic/CSS.

I strongly recommend following along by commit, mostly because there's a ton of snapshot updates.

closes #2017 - I consider this to be the last major form control component in the list.

Before/After

QA

Regression test https://eui.elastic.co/pr_6705/#/forms/form-controls#form-control-layout-delimited and ensure it looks the same as prod (except for intended changes)
Regression test https://eui.elastic.co/pr_6705/#/forms/date-picker#date-picker-range and ensure it looks the same as prod
Regression test https://eui.elastic.co/pr_6705/#/templates/super-date-picker and ensure it looks the same as prod

General checklist

Checked in both light and dark modes
Checked in mobile - (note: not perfect but already had the same issues in prod)
Checked in Chrome, Safari, Edge, and Firefox
Added or updated jest ~~and cypress tests~~
Updated the Figma library counterpart
A changelog entry exists and is marked appropriately

- [ ] Props have proper autodocs (using @default if default values are missing) and playground toggles
- [ ] Added documentation
~~- [ ] Checked Code Sandbox works for any docs examples~~
~~- [ ] Checked for breaking changes and labeled appropriately~~
~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~

kibanamachine · 2023-04-12T02:52:38Z

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

cee-chen · 2023-04-12T14:01:52Z

Hey @breehall! Sorry to dump another larger PR on you, but hoping you'd be able to take this on this week and help QA/regression test the listed steps/sections to check for any visual regressions. Thanks a million!

…d` state on mount - needs a state rerender on mount - ref is insufficient + update tests to use RTL instead of enzyme and move to inline snapshots

- Positioning: prefer a prop instead, so that each usage (e.g. delimited, ranges) can pass in their own custom control over absolute vs static positioning - Side: clean up EuiFormLayoutControlIcons to render separate for left vs right side, which makes for better and more correct DOM order, + clean up classes + misc cleanup

- height/width doesn't appear to be doing anything - The padding is unnecessary / adding extra spacing and doesn't have a non-compressed version

- instead of using its own custom delimiter + add `controlOnly` support to the `EuiDatePicker` to avoid nested control layouts (matches `EuiFieldText/Number`) + add `append/prepend` props for future `EuiSuperDatePicker` use (& DRY out other props shared w/ `EuiFormControlLayoutDelimited`)

- EuiFormControlLayoutDelimited should be doing almost all the CSS heavy lifting at this point

- Remove duplicated `EuiFormControlLayout` - only render if `EuiDateRangePicker` isn't being used - Add `isLoading` display to picker if update button isn't being shown - Fix icons not displaying correctly if `isQuickSelectOnly` is true

…input children + fix extra padding on delimiter arrow when prepend/append exist + clean up/DRY out invalid styling on icons & delimiter

…ides

kibanamachine · 2023-04-12T17:00:04Z

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

…date` state to match Figma

… controls

kibanamachine · 2023-04-12T20:31:02Z

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

breehall

This is great work and I'm sure it was quite a task! As recommended, I followed commit by commit and scanned snapshots as well. As a whole, everything looks good, but I wanted to run two things past you to see if they were intended.

It looks like the alignment of the popover in EuiSuperDatePicker has shifted very slightly. The position now changes based on which date you select and in prod it is centered with the input.
(https://eui.elastic.co/pr_6705/#/forms/date-picker#date-picker-range)

In the super date picker template, it looks like we're now highlighting and changing the color of the range arrow. This remained white in prod.
(https://eui.elastic.co/pr_6705/#/templates/super-date-picker)

cee-chen · 2023-04-13T16:16:07Z

Thanks for the super thorough QA Bree, it's much appreciated and makes me feel more confident in a (hopeful) lack of regressions!

In the super date picker template, it looks like we're now highlighting and changing the color of the range arrow. This remained white in prod.

Very intentional change - this matches the Figma pattern that the super date picker was apparently supposed to have used this entire time. Plus I think it looks better!

It looks like the alignment of the popover in EuiSuperDatePicker has shifted very slightly. The position now changes based on which date you select and in prod it is centered with the input.

This is not necessarily an intentional change but it is an expected one based on how the underlying EuiFormControlLayoutDelimited component works. I went back and forth a bit on this and actually spiked out trying to get the calendar icon behavior to how it previously was, and then decided against it as unnecessary. I think separating the calendar icon from the first input is how it should behave based on how the underlying EuiFormControlLayoutDelimited works. The Figma designs also very clearly have EuiDatePickerRange using the delimited pattern, so in this case I'm following Figma again.

If we get any actual end-user reports or complaints I'd be open to revisiting in the future, but in the meanwhile I'd argue this is OK to change as it has minimal to no meaningful UX impact.

breehall

✅ Thanks so much for answering my questions! This is good to go

- instead of applying bg colors to individual delimiter and icon elements, apply it to the parent `__childenWrapper` + simplify default delimited icon to not set its own `color` but instead inherit from its `EuiText` wrapper + update downstream EuiSuperDatePicker CSS

cee-chen · 2023-04-13T18:46:26Z

The EuiSuperDatePicker arrow highlight got me thinking of another way to simplify our delimiter CSS a bit more, so I pushed up one last quick cleanup commit. I've tested it thoroughly against the form delimited docs and the super datepicker docs and am confident it's working as before with slightly DRYer CSS, so will do an auto-merge here once CI runs. Thanks again for the review Bree!

kibanamachine · 2023-04-13T19:10:33Z

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

## 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>

cee-chen requested a review from breehall April 12, 2023 14:02

cee-chen added 10 commits April 12, 2023 09:33

[docs setup] Make certain boolean flags/props more easily testable

40782af

[bugfix] Fix EuiFormControlLayoutDelimited not displaying `isInvali…

28e49b2

…d` state on mount - needs a state rerender on mount - ref is insufficient + update tests to use RTL instead of enzyme and move to inline snapshots

Remove unnecessary CSS

240026b

- height/width doesn't appear to be doing anything - The padding is unnecessary / adding extra spacing and doesn't have a non-compressed version

Update downstream snapshots

65592f1

Remove unnecessary CSS

f75da63

- EuiFormControlLayoutDelimited should be doing almost all the CSS heavy lifting at this point

[EuiSuperDatePicker] Fix styling and fullWidth not applying to non-…

b5c077d

…input children + fix extra padding on delimiter arrow when prepend/append exist + clean up/DRY out invalid styling on icons & delimiter

[EuiSuperDatePicker] Fix border-radius & remove unnecessary CSS/overr…

ea6619f

…ides

cee-chen force-pushed the datepicker/invalid branch from 7ae3924 to ea6619f Compare April 12, 2023 16:38

cee-chen force-pushed the datepicker/invalid branch from a324802 to 5e4e85c Compare April 12, 2023 20:01

cee-chen added 2 commits April 12, 2023 13:07

[EuiSuperDatePicker] Update color of delimiter arrow during `needs up…

759fc89

…date` state to match Figma

[EuiSuperDatePicker] Fix invalid & disabled state to match other form…

f6dbd09

… controls

cee-chen force-pushed the datepicker/invalid branch from 5e4e85c to f6dbd09 Compare April 12, 2023 20:08

breehall reviewed Apr 13, 2023

View reviewed changes

breehall approved these changes Apr 13, 2023

View reviewed changes

cee-chen enabled auto-merge (squash) April 13, 2023 18:47

cee-chen merged commit 6a65a47 into elastic:main Apr 13, 2023

cee-chen deleted the datepicker/invalid branch April 13, 2023 19:24

1Copenut mentioned this pull request Apr 18, 2023

Upgrade EUI to v77.2.2 elastic/kibana#155208

Merged

cee-chen mentioned this pull request May 23, 2023

[EuiDatePickerRange] Add inline display behavior; [EuiDatePicker] Improve inline display behavior #6795

Merged

12 tasks

cee-chen mentioned this pull request Aug 8, 2023

[EuiDatePickerRange] Add support for compressed display #7058

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood `EuiFormControlLayoutDelimited` #6705

[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood `EuiFormControlLayoutDelimited` #6705

cee-chen commented Apr 12, 2023 •

edited

Loading

kibanamachine commented Apr 12, 2023

cee-chen commented Apr 12, 2023

kibanamachine commented Apr 12, 2023

kibanamachine commented Apr 12, 2023

breehall left a comment

cee-chen commented Apr 13, 2023

breehall left a comment

cee-chen commented Apr 13, 2023

kibanamachine commented Apr 13, 2023

[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood EuiFormControlLayoutDelimited #6705

[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood EuiFormControlLayoutDelimited #6705

Conversation

cee-chen commented Apr 12, 2023 • edited Loading

Summary

Before/After

QA

General checklist

kibanamachine commented Apr 12, 2023

cee-chen commented Apr 12, 2023

kibanamachine commented Apr 12, 2023

kibanamachine commented Apr 12, 2023

breehall left a comment

Choose a reason for hiding this comment

cee-chen commented Apr 13, 2023

breehall left a comment

Choose a reason for hiding this comment

cee-chen commented Apr 13, 2023

kibanamachine commented Apr 13, 2023

[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood `EuiFormControlLayoutDelimited` #6705

[EuiDatePickerRange][EuiSuperDatePicker] Convert to dogfood `EuiFormControlLayoutDelimited` #6705

cee-chen commented Apr 12, 2023 •

edited

Loading