From 894da9a0a65065913f2ef8027cdab8f615e92915 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 1 May 2023 16:32:08 -0700 Subject: [PATCH 1/7] DOM cleanup - Remove unnecessary `EuiForm` - we're not taking advantage of form submit behavior, so we might as well remove this - Remove unnecessary `EuiFormRow`s around save/cancel buttons - they're not inputs and don't need form rows - Remove incorrect `className={classes}` placement --- .../inline_edit_form.test.tsx.snap | 1264 ++++++++--------- .../inline_edit/inline_edit_form.tsx | 175 ++- 2 files changed, 639 insertions(+), 800 deletions(-) diff --git a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap index 3f2a5a14210..c4aa7534b34 100644 --- a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap +++ b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap @@ -5,143 +5,121 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.cancelButtonProps 1`] = ` class="euiInlineEdit testClass1 testClass2" >
-
- -
+
-
+ +
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
+
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
@@ -153,143 +131,121 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.formRowProps 1`] = ` class="euiInlineEdit testClass1 testClass2" >
-
- -
+
-
+ +
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
+
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
@@ -301,148 +257,126 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.inputProps 1`] = ` class="euiInlineEdit testClass1 testClass2" >
+
- -
- -
+
-
+ +
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
+
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
@@ -454,142 +388,120 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.saveButtonProps 1`] = ` class="euiInlineEdit testClass1 testClass2" >
-
- -
+
-
+ +
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
+
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
@@ -601,151 +513,129 @@ exports[`EuiInlineEditForm Edit Mode isInvalid 1`] = ` class="euiInlineEdit testClass1 testClass2" >
+
- -
- -
-
+ +
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
+
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
@@ -757,105 +647,81 @@ exports[`EuiInlineEditForm Edit Mode isLoading 1`] = ` class="euiInlineEdit testClass1 testClass2" >
+
- -
- -
-
+ +
+
-
-
-
-
-
-
+ aria-label="Loading " + class="euiSkeletonRectangle emotion-euiSkeletonGradientAnimation-euiSkeletonRectangle-m" + role="progressbar" + style="inline-size: 40px; block-size: 40px;" + />
+
+
-
-
-
-
-
-
+ aria-label="Loading " + class="euiSkeletonRectangle emotion-euiSkeletonGradientAnimation-euiSkeletonRectangle-m" + role="progressbar" + style="inline-size: 40px; block-size: 40px;" + />
@@ -867,142 +733,120 @@ exports[`EuiInlineEditForm Edit Mode renders 1`] = ` class="euiInlineEdit testClass1 testClass2" >
-
- -
+
-
+ +
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
+
+
-
-
-
- Loaded -
- - -
+ Loaded
+ +
diff --git a/src/components/inline_edit/inline_edit_form.tsx b/src/components/inline_edit/inline_edit_form.tsx index 7ac900295c6..c6e0c8662db 100644 --- a/src/components/inline_edit/inline_edit_form.tsx +++ b/src/components/inline_edit/inline_edit_form.tsx @@ -21,7 +21,6 @@ import { EuiFormRow, EuiFormRowProps, EuiFieldText, - EuiForm, EuiFieldTextProps, } from '../form'; import { euiFormVariables } from '../form/form.styles'; @@ -178,98 +177,94 @@ export const EuiInlineEditForm: FunctionComponent = ({ }; const editModeForm = ( - - - - + + + { + setEditModeValue(e.target.value); + }} + aria-label={inputAriaLabel} + autoFocus + compressed={sizes.compressed} isInvalid={isInvalid} - error={isInvalid && editModeProps?.formRowProps?.error} - {...editModeProps?.formRowProps} - > - { - setEditModeValue(e.target.value); - }} - aria-label={inputAriaLabel} - autoFocus - compressed={sizes.compressed} - isInvalid={isInvalid} - isLoading={isLoading} - data-test-subj="euiInlineEditModeInput" - {...editModeProps?.inputProps} - aria-describedby={classNames( - editModeDescribedById, - editModeProps?.inputProps?.['aria-describedby'] - )} - onKeyDown={(e: KeyboardEvent) => { - editModeInputOnKeyDown(e); - editModeProps?.inputProps?.onKeyDown?.(e); - }} - /> - - - + isLoading={isLoading} + data-test-subj="euiInlineEditModeInput" + {...editModeProps?.inputProps} + aria-describedby={classNames( + editModeDescribedById, + editModeProps?.inputProps?.['aria-describedby'] + )} + onKeyDown={(e: KeyboardEvent) => { + editModeInputOnKeyDown(e); + editModeProps?.inputProps?.onKeyDown?.(e); + }} + /> + + + - - - - ) => { - saveInlineEditValue(); - editModeProps?.saveButtonProps?.onClick?.(e); - }} - /> - - - + + + ) => { + saveInlineEditValue(); + editModeProps?.saveButtonProps?.onClick?.(e); + }} + /> + + - - - - ) => { - cancelInlineEdit(); - editModeProps?.cancelButtonProps?.onClick?.(e); - }} - /> - - - - - + + + ) => { + cancelInlineEdit(); + editModeProps?.cancelButtonProps?.onClick?.(e); + }} + /> + + + ); const readModeElement = ( From d1b75f98f345af03389e787ff39368b89883517d Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 1 May 2023 16:34:17 -0700 Subject: [PATCH 2/7] Use `EuiSkeletonLoading` instead of 2 `EuiSkeletonRectangle`s - this prevents double loading screen reader announcements --- .../inline_edit_form.test.tsx.snap | 489 +++++++----------- .../inline_edit/inline_edit_form.tsx | 94 ++-- 2 files changed, 231 insertions(+), 352 deletions(-) diff --git a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap index c4aa7534b34..4c6b22332a4 100644 --- a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap +++ b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap @@ -67,59 +67,37 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.cancelButtonProps 1`] = ` role="status" />
- -
-
-
-
-
- Loaded -
- -
@@ -194,58 +172,36 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.formRowProps 1`] = ` role="status" />
- -
-
-
-
-
- Loaded -
- -
@@ -325,58 +281,36 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.inputProps 1`] = ` role="status" />
- -
-
-
-
-
- Loaded -
- -
@@ -450,58 +384,36 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.saveButtonProps 1`] = ` role="status" />
- -
-
-
-
-
- Loaded -
- -
@@ -584,58 +496,36 @@ exports[`EuiInlineEditForm Edit Mode isInvalid 1`] = ` role="status" />
- -
-
-
-
-
- Loaded -
- -
@@ -703,25 +593,32 @@ exports[`EuiInlineEditForm Edit Mode isLoading 1`] = ` >
-
-
-
-
-
+ > +
+
+
+
+
+
+
@@ -795,58 +692,36 @@ exports[`EuiInlineEditForm Edit Mode renders 1`] = ` role="status" />
- -
-
-
-
-
- Loaded -
- -
diff --git a/src/components/inline_edit/inline_edit_form.tsx b/src/components/inline_edit/inline_edit_form.tsx index c6e0c8662db..9f96a0c98f0 100644 --- a/src/components/inline_edit/inline_edit_form.tsx +++ b/src/components/inline_edit/inline_edit_form.tsx @@ -28,7 +28,7 @@ import { EuiButtonIcon, EuiButtonEmpty } from '../button'; import { EuiButtonIconPropsForButton } from '../button/button_icon'; import { EuiButtonEmptyPropsForButton } from '../button/button_empty/button_empty'; import { EuiFlexGroup, EuiFlexItem } from '../flex'; -import { EuiSkeletonRectangle } from '../skeleton'; +import { EuiSkeletonLoading, EuiSkeletonRectangle } from '../skeleton'; import { useEuiTheme, keys } from '../../services'; import { EuiI18n, useEuiI18n } from '../i18n'; import { useGeneratedHtmlId } from '../../services/accessibility'; @@ -218,51 +218,55 @@ export const EuiInlineEditForm: FunctionComponent = ({ - - ) => { - saveInlineEditValue(); - editModeProps?.saveButtonProps?.onClick?.(e); - }} - /> - - - - - - ) => { - cancelInlineEdit(); - editModeProps?.cancelButtonProps?.onClick?.(e); - }} - /> - + loadingContent={ + + + + + } + loadedContent={ + + ) => { + saveInlineEditValue(); + editModeProps?.saveButtonProps?.onClick?.(e); + }} + /> + ) => { + cancelInlineEdit(); + editModeProps?.cancelButtonProps?.onClick?.(e); + }} + /> + + } + /> ); From 76e04a49d4e416439bf37d2ff3839c7a484cb81c Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 1 May 2023 16:51:52 -0700 Subject: [PATCH 3/7] Update EuiInlineEdit to only announce when state flips to loading - so that a "loaded" announcement doesn't occur whenever edit mode is opened, which doesn't make a whole lot of sense --- .../inline_edit_form.test.tsx.snap | 119 +++--------------- .../inline_edit/inline_edit_form.tsx | 2 + 2 files changed, 19 insertions(+), 102 deletions(-) diff --git a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap index 4c6b22332a4..541ceaf239f 100644 --- a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap +++ b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap @@ -50,23 +50,6 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.cancelButtonProps 1`] = ` aria-busy="false" data-test-subj="euiSkeletonLoadingAriaWrapper" > -
-
- Loaded -
-
@@ -155,23 +138,6 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.formRowProps 1`] = ` aria-busy="false" data-test-subj="euiSkeletonLoadingAriaWrapper" > -
-
- Loaded -
-
@@ -264,23 +230,6 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.inputProps 1`] = ` aria-busy="false" data-test-subj="euiSkeletonLoadingAriaWrapper" > -
-
- Loaded -
-
@@ -367,23 +316,6 @@ exports[`EuiInlineEditForm Edit Mode editModeProps.saveButtonProps 1`] = ` aria-busy="false" data-test-subj="euiSkeletonLoadingAriaWrapper" > -
-
- Loaded -
-
@@ -479,23 +411,6 @@ exports[`EuiInlineEditForm Edit Mode isInvalid 1`] = ` aria-busy="false" data-test-subj="euiSkeletonLoadingAriaWrapper" > -
-
- Loaded -
-
@@ -591,6 +506,23 @@ exports[`EuiInlineEditForm Edit Mode isLoading 1`] = ` aria-busy="true" data-test-subj="euiSkeletonLoadingAriaWrapper" > +
+
+ Loading +
+
-
-
- Loaded -
-
diff --git a/src/components/inline_edit/inline_edit_form.tsx b/src/components/inline_edit/inline_edit_form.tsx index 9f96a0c98f0..7db83b6832e 100644 --- a/src/components/inline_edit/inline_edit_form.tsx +++ b/src/components/inline_edit/inline_edit_form.tsx @@ -220,6 +220,8 @@ export const EuiInlineEditForm: FunctionComponent = ({ Date: Mon, 1 May 2023 17:15:11 -0700 Subject: [PATCH 4/7] Improve SR UX of read mode - add an `aria-describedby` that explains to screen reader users what happens when the read mode button is clicked, since the text itself is not indicative + cleanup: remove unused input ID --- .../inline_edit_form.test.tsx.snap | 21 ++++++++ .../inline_edit_text.test.tsx.snap | 28 +++++++++++ .../inline_edit_title.test.tsx.snap | 49 ++++++++++++++++++ .../inline_edit/inline_edit_form.tsx | 50 +++++++++++-------- 4 files changed, 128 insertions(+), 20 deletions(-) diff --git a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap index 541ceaf239f..44d0882bc92 100644 --- a/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap +++ b/src/components/inline_edit/__snapshots__/inline_edit_form.test.tsx.snap @@ -648,6 +648,7 @@ exports[`EuiInlineEditForm Read Mode readModeProps 1`] = ` class="euiInlineEdit testClass1 testClass2" > +
`; @@ -675,6 +682,7 @@ exports[`EuiInlineEditForm Read Mode renders 1`] = ` class="euiInlineEdit testClass1 testClass2" > +
`; @@ -702,6 +716,7 @@ exports[`EuiInlineEditForm Read Mode sizes 1`] = ` class="euiInlineEdit testClass1 testClass2" > +
`; diff --git a/src/components/inline_edit/__snapshots__/inline_edit_text.test.tsx.snap b/src/components/inline_edit/__snapshots__/inline_edit_text.test.tsx.snap index 42d20b7f64e..c84ffa0c68a 100644 --- a/src/components/inline_edit/__snapshots__/inline_edit_text.test.tsx.snap +++ b/src/components/inline_edit/__snapshots__/inline_edit_text.test.tsx.snap @@ -5,6 +5,7 @@ exports[`EuiInlineEditText renders 1`] = ` class="euiInlineEdit euiInlineEditText testClass1 testClass2 emotion-euiInlineEditText-m-m" > +
`; @@ -36,6 +43,7 @@ exports[`EuiInlineEditText text sizes renders m 1`] = ` class="euiInlineEdit euiInlineEditText testClass1 testClass2 emotion-euiInlineEditText-m-m" > +
`; @@ -67,6 +81,7 @@ exports[`EuiInlineEditText text sizes renders s 1`] = ` class="euiInlineEdit euiInlineEditText testClass1 testClass2 emotion-euiInlineEditText-s-s" > +
`; @@ -98,6 +119,7 @@ exports[`EuiInlineEditText text sizes renders xs 1`] = ` class="euiInlineEdit euiInlineEditText testClass1 testClass2 emotion-euiInlineEditText-xs-xs" > +
`; diff --git a/src/components/inline_edit/__snapshots__/inline_edit_title.test.tsx.snap b/src/components/inline_edit/__snapshots__/inline_edit_title.test.tsx.snap index 19a670a600d..58527fff8f8 100644 --- a/src/components/inline_edit/__snapshots__/inline_edit_title.test.tsx.snap +++ b/src/components/inline_edit/__snapshots__/inline_edit_title.test.tsx.snap @@ -5,6 +5,7 @@ exports[`EuiInlineEditTitle renders 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-m-m" > +
`; @@ -36,6 +43,7 @@ exports[`EuiInlineEditTitle title sizes renders size l 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-l-l" > +
`; @@ -67,6 +81,7 @@ exports[`EuiInlineEditTitle title sizes renders size m 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-m-m" > +
`; @@ -98,6 +119,7 @@ exports[`EuiInlineEditTitle title sizes renders size s 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-s-s" > +
`; @@ -129,6 +157,7 @@ exports[`EuiInlineEditTitle title sizes renders size xs 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-xs-xs" > +
`; @@ -160,6 +195,7 @@ exports[`EuiInlineEditTitle title sizes renders size xxs 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-xxs-xxs" > +
`; @@ -191,6 +233,7 @@ exports[`EuiInlineEditTitle title sizes renders size xxxs 1`] = ` class="euiInlineEdit euiInlineEditTitle testClass1 testClass2 emotion-euiInlineEditTitle-xxxs-xxxs" > +
`; diff --git a/src/components/inline_edit/inline_edit_form.tsx b/src/components/inline_edit/inline_edit_form.tsx index 7db83b6832e..4c32881e5c4 100644 --- a/src/components/inline_edit/inline_edit_form.tsx +++ b/src/components/inline_edit/inline_edit_form.tsx @@ -139,11 +139,10 @@ export const EuiInlineEditForm: FunctionComponent = ({ 'Cancel edit' ); + const readModeDescribedById = useGeneratedHtmlId({ prefix: 'inlineEdit' }); const editModeDescribedById = useGeneratedHtmlId({ prefix: 'inlineEdit' }); const [isEditing, setIsEditing] = useState(false || startWithEditOpen); - const inlineEditInputId = useGeneratedHtmlId({ prefix: '__inlineEditInput' }); - const [editModeValue, setEditModeValue] = useState(defaultValue); const [readModeValue, setReadModeValue] = useState(defaultValue); @@ -187,7 +186,6 @@ export const EuiInlineEditForm: FunctionComponent = ({ > { setEditModeValue(e.target.value); @@ -274,23 +272,35 @@ export const EuiInlineEditForm: FunctionComponent = ({ ); const readModeElement = ( - { - setIsEditing(true); - readModeProps?.onClick?.(e); - }} - > - {children(readModeValue)} - + <> + { + setIsEditing(true); + readModeProps?.onClick?.(e); + }} + > + {children(readModeValue)} + + + ); return ( From a5001a7a371a67591fb0cb975c47140ddf1004ca Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 1 May 2023 17:20:18 -0700 Subject: [PATCH 5/7] Fix `editModeProps.inputProps.onChange` overriding/breaking behavior --- .../inline_edit/inline_edit_form.test.tsx | 14 ++++++++++++-- src/components/inline_edit/inline_edit_form.tsx | 15 ++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/components/inline_edit/inline_edit_form.test.tsx b/src/components/inline_edit/inline_edit_form.test.tsx index 98a46cb935a..cd990756c60 100644 --- a/src/components/inline_edit/inline_edit_form.test.tsx +++ b/src/components/inline_edit/inline_edit_form.test.tsx @@ -76,6 +76,8 @@ describe('EuiInlineEditForm', () => { }); test('editModeProps.inputProps', () => { + const onChange = jest.fn(); + const { container, getByTestSubject } = render( { inputProps: { prepend: 'Prepend Example', 'data-test-subj': 'customInput', + onChange, }, }} /> ); - expect(container.firstChild).toMatchSnapshot(); - expect(getByTestSubject('customInput')).toBeTruthy(); + + const mockChangeEvent = { target: { value: 'changed' } }; + fireEvent.change(getByTestSubject('customInput'), mockChangeEvent); + expect(onChange).toHaveBeenCalled(); + + // Consumer `onChange` callbacks should not override EUI's + expect( + (getByTestSubject('customInput') as HTMLInputElement).value + ).toEqual('changed'); }); test('editModeProps.formRowProps', () => { diff --git a/src/components/inline_edit/inline_edit_form.tsx b/src/components/inline_edit/inline_edit_form.tsx index 4c32881e5c4..5bbab3758c7 100644 --- a/src/components/inline_edit/inline_edit_form.tsx +++ b/src/components/inline_edit/inline_edit_form.tsx @@ -187,9 +187,6 @@ export const EuiInlineEditForm: FunctionComponent = ({ { - setEditModeValue(e.target.value); - }} aria-label={inputAriaLabel} autoFocus compressed={sizes.compressed} @@ -197,14 +194,18 @@ export const EuiInlineEditForm: FunctionComponent = ({ isLoading={isLoading} data-test-subj="euiInlineEditModeInput" {...editModeProps?.inputProps} + onChange={(e) => { + setEditModeValue(e.target.value); + editModeProps?.inputProps?.onChange?.(e); + }} + onKeyDown={(e) => { + editModeInputOnKeyDown(e); + editModeProps?.inputProps?.onKeyDown?.(e); + }} aria-describedby={classNames( editModeDescribedById, editModeProps?.inputProps?.['aria-describedby'] )} - onKeyDown={(e: KeyboardEvent) => { - editModeInputOnKeyDown(e); - editModeProps?.inputProps?.onKeyDown?.(e); - }} />