[Form controls] Added `alert` icon indicators for invalid state #5738

cchaos · 2022-03-21T20:56:20Z

Partially tackles #2017

[EuiFormControlLayout]

Added isInvalid prop which is passed through to EuiFormControlLayoutIcon which renders the alert icon in red (danger)
Added isDropdown to create and control the arrowDown icon (better for quickly creating this internally and handles visibility based on readOnly and disabled states
Added color as a possible option for the IconObject (more configuration options that I needed for color picker)

Passed the `isInvalid` through for only the following form controls

EuiFieldNumber, EuiFieldPassword, EuiFieldText, EuiSelect, EuiSuperSelect, EuiFieldSearch
The rest to be done in a follow up PR, these were just the easy ones

To test, I've set all the Display Toggles to be on by default (I'll revert this before merging).

New Sass method of applying left/right padding based on number of icons

Added a getFormControlClassNameForIconCount localized service for counting icons based on boolean state variables
Changed className output from rendering component-specific to generic to reduce output styles. So instead of .euiFieldText--isInvalid.euiFieldText--hasIcon and all those combinations of state-produced classes, the above service passes back euiFormControlLayout--2icons that's applied to the form control. So we only have 5 (1-5 number of icons possible) classes total for handling padding vs how every many permutations per control.

[EuiColorPicker] Fixed usage of EuiFormControlLayout to not double

In order for the EuiColorPicker to have an icon on the left (color swatch) and the dropdown arrow on the right, it had to create a second EuiFormControlWrapper to add it. Now with the ability to just say isDropdown to add the arrow icon, we no longer need the second wrapper. But instead of being able to just remove the manual wrapper I had to set controlOnly on the EuiFieldText component because we needed to support the clear prop.

[EuiSelect] & [EuiSuperSelect] Updated to new props of EuIFormControlLayout

These components now use the isDropdown prop instead of a custom icon. I was also able to fix a couple other quick issues:

Fixed [EuiSuperSelect] With prepend and append needs it's border-radius fixed in Amsterdam #5442 EuiSuperSelect border radius with append/prepend
Fixed [EuiSuperSelect] Ignores readOnly prop #3510 EuiSuperSelect not respecting readOnly

Checklist

which is passed through to `EuiFormControlLayoutIcon` which renders the `alert` icon in red

- EuiFieldNumber, EuiFieldPassword, EuiFieldText, EuiSelect, EuiSuperSelect - EuiFieldSearch attempts to create a new class for the number of icons

…outing icons

…to reduce output syles

…rrow down icon

kibanamachine · 2022-03-21T21:34:30Z

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

- Fixed EuiSuperSelect border radius with append/prepend elastic#5442 - Fixed EuiSuperSelect not respecting `readOnly` elastic#3510

cchaos · 2022-03-22T03:12:27Z

Ok, this PR is ready for review as I go through the final checklist. Please be sure to read through the PR summary as this PR only tackles some of the form controls.

…id_icon

kibanamachine · 2022-03-22T03:47:52Z

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

cchaos · 2022-03-22T20:23:01Z

src/components/form/field_number/__snapshots__/field_number.test.tsx.snap

  <eui-validatable-control
    isinvalid="true"
  >
    <input
-      class="euiFieldNumber"
+      class="euiFieldNumber euiFormControlLayout--1icons"
      type="number"
    />


I don't know why in the snapshots it's not adding that aria-invalid to the <input>. The one place I did see it get added was in the EuiSuggest snapshot https://github.com/elastic/eui/pull/5738/files#diff-4cd42a1237d4c776e9fd1140e106d4ba0d063ec3a7010a7646ca74236b040f8a

I think because EuiValidatableControl is getting mocked in the test file:

eui/src/components/form/field_number/field_number.test.tsx

Lines 22 to 24 in d723d95

jest.mock('../validatable_control', () => ({

EuiValidatableControl: 'eui-validatable-control',

}));

So the cloneElement additions don't happen. Not sure if that mock is still necessary.

kibanamachine · 2022-03-22T21:05:07Z

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

kibanamachine · 2022-03-23T03:32:33Z

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

elizabetdev

LGTM! 🎉

I looked at the code and tested it in Firefox, Safari, Chrome, and Edge.

I also found two minor issues.

The EuiColorPalettePicker needs to handle better the spacing when is compresseed:

I think the CodeSandbox demos need to have the @emotion/react imported in the demo.tsx file.

src-docs/src/views/form_controls/form_control_layout_range.tsx

cchaos · 2022-03-23T19:10:30Z

No. 1 should be fixed, I was re-calculating the spacing between icons to be smaller in compressed but in fact it never was changing.

kibanamachine · 2022-03-23T19:42:48Z

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

1Copenut

LGTM! The Color Palette Picker is receiving the isInvalid prop correctly, but isn't announcing itself as an invalid text field b/c of the button + listbox structure. Not a problem, just an item we should review and strategize ways to announce as invalid in another issue.

thompsongl

Just the one question. Everything is looking good!

src/components/form/form_control_layout/_num_icons.test.ts

src/components/form/super_select/super_select_control.tsx

Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>

kibanamachine · 2022-03-30T19:25:11Z

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

This reverts commit fce7b90. # Conflicts: # src-docs/src/views/form_controls/display_toggles.js

kibanamachine · 2022-03-30T20:36:40Z

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

kibanamachine · 2022-03-30T21:08:21Z

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

cchaos added 12 commits March 18, 2022 10:13

[EuiFormControlLayout] Added isInvalid prop

d5205a1

which is passed through to `EuiFormControlLayoutIcon` which renders the `alert` icon in red

Padded the isInvalid through for some form controls

5c72a7a

- EuiFieldNumber, EuiFieldPassword, EuiFieldText, EuiSelect, EuiSuperSelect - EuiFieldSearch attempts to create a new class for the number of icons

Finalized new padding mixin

a5ad055

Added a getFormControlClassNameForIconCount localized service for c…

8c6a12d

…outing icons

Change className output from rendering component-specific to generic …

0b1f4a1

…to reduce output syles

Updating form control usage & styles

2b71e2f

Fix docs

32f3f1e

REVERT ME: Docs toggles all on

fce7b90

Moved service into form_control_layout folder and added tests

8a27ecf

[EuiFormControlLayout] Added isDropdown to create and control the a…

1ac2973

…rrow down icon

[EuiColorPicker] Fix usage of EuiFormControlLayout to not double

811231f

Remove no longer necessary prop comment

8ab25ba

cchaos added 3 commits March 21, 2022 22:41

Fix Sass import location

89faf54

Layout docs

fdca711

[EuiSelect] & [EuiSuperSelect] Update to new EuiFormControlLayout props

dd3bdac

- Fixed EuiSuperSelect border radius with append/prepend elastic#5442 - Fixed EuiSuperSelect not respecting `readOnly` elastic#3510

cchaos marked this pull request as ready for review March 22, 2022 03:12

cchaos requested review from 1Copenut, thompsongl and elizabetdev March 22, 2022 03:12

Merge remote-tracking branch 'upstream/main' into form_controls/inval…

2bfd32e

…id_icon

cchaos added 2 commits March 22, 2022 16:01

[EuiValidatableControl] Add aria-invalid

d723d95

Clean up and CL

2effa61

cchaos commented Mar 22, 2022

View reviewed changes

cl again

d91bb12

Moved invalid icon to be second to last from dropdown arrow

d7f83c8

elizabetdev approved these changes Mar 23, 2022

View reviewed changes

src-docs/src/views/form_controls/form_control_layout_range.tsx Show resolved Hide resolved

Fix math for compress icon spacing/padding

2ffd108

1Copenut approved these changes Mar 23, 2022

View reviewed changes

thompsongl reviewed Mar 30, 2022

View reviewed changes

src/components/form/form_control_layout/_num_icons.test.ts Outdated Show resolved Hide resolved

src/components/form/super_select/super_select_control.tsx Show resolved Hide resolved

Update src/components/form/form_control_layout/_num_icons.test.ts

524cda5

Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>

thompsongl approved these changes Mar 30, 2022

View reviewed changes

cchaos and others added 2 commits March 30, 2022 16:00

Merge branch 'elastic:main' into form_controls/invalid_icon

f3cdc9d

Revert "REVERT ME: Docs toggles all on"

03a88af

This reverts commit fce7b90. # Conflicts: # src-docs/src/views/form_controls/display_toggles.js

cchaos enabled auto-merge (squash) March 30, 2022 20:37

cchaos mentioned this pull request Mar 30, 2022

[Form controls] Better indicators for invalid state #2017

Closed

13 tasks

cchaos merged commit d31f753 into elastic:main Mar 30, 2022

cchaos deleted the form_controls/invalid_icon branch April 1, 2022 14:52

cee-chen mentioned this pull request Apr 6, 2022

Upgrade EUI to v54.0.0 elastic/kibana#129653

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form controls] Added `alert` icon indicators for invalid state #5738

[Form controls] Added `alert` icon indicators for invalid state #5738

cchaos commented Mar 21, 2022 •

edited

Loading

kibanamachine commented Mar 21, 2022

cchaos commented Mar 22, 2022

kibanamachine commented Mar 22, 2022

cchaos Mar 22, 2022

thompsongl Mar 22, 2022

kibanamachine commented Mar 22, 2022

kibanamachine commented Mar 23, 2022

elizabetdev left a comment

cchaos commented Mar 23, 2022

kibanamachine commented Mar 23, 2022

1Copenut left a comment

thompsongl left a comment

kibanamachine commented Mar 30, 2022

kibanamachine commented Mar 30, 2022

kibanamachine commented Mar 30, 2022

	jest.mock('../validatable_control', () => ({
	EuiValidatableControl: 'eui-validatable-control',
	}));

[Form controls] Added alert icon indicators for invalid state #5738

[Form controls] Added alert icon indicators for invalid state #5738

Conversation

cchaos commented Mar 21, 2022 • edited Loading

Partially tackles #2017

[EuiFormControlLayout]

Passed the isInvalid through for only the following form controls

New Sass method of applying left/right padding based on number of icons

[EuiColorPicker] Fixed usage of EuiFormControlLayout to not double

[EuiSelect] & [EuiSuperSelect] Updated to new props of EuIFormControlLayout

Checklist

kibanamachine commented Mar 21, 2022

cchaos commented Mar 22, 2022

kibanamachine commented Mar 22, 2022

cchaos Mar 22, 2022

Choose a reason for hiding this comment

thompsongl Mar 22, 2022

Choose a reason for hiding this comment

kibanamachine commented Mar 22, 2022

kibanamachine commented Mar 23, 2022

elizabetdev left a comment

Choose a reason for hiding this comment

cchaos commented Mar 23, 2022

kibanamachine commented Mar 23, 2022

1Copenut left a comment

Choose a reason for hiding this comment

thompsongl left a comment

Choose a reason for hiding this comment

kibanamachine commented Mar 30, 2022

kibanamachine commented Mar 30, 2022

kibanamachine commented Mar 30, 2022

[Form controls] Added `alert` icon indicators for invalid state #5738

[Form controls] Added `alert` icon indicators for invalid state #5738

cchaos commented Mar 21, 2022 •

edited

Loading

Passed the `isInvalid` through for only the following form controls