From c40c8eb5052e3dd0f602d4e8980e0e33889b7c69 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 4 Apr 2023 15:29:03 -0700 Subject: [PATCH 1/9] [EuiFormControlLayoutDelimited] nit: fix `delimiter` spelling --- .../form_control_layout_delimited.test.tsx.snap | 6 +++--- .../_form_control_layout_delimited.scss | 8 ++++---- .../form_control_layout/form_control_layout_delimited.tsx | 2 +- .../form/range/__snapshots__/dual_range.test.tsx.snap | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap b/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap index bac34682d28..9c8b1cfb5a2 100644 --- a/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap +++ b/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap @@ -15,7 +15,7 @@ exports[`EuiFormControlLayoutDelimited is rendered 1`] = ` start
@@ -41,7 +41,7 @@ exports[`EuiFormControlLayoutDelimited props delimiter is rendered as a node 1`] start
+
diff --git a/src/components/form/form_control_layout/_form_control_layout_delimited.scss b/src/components/form/form_control_layout/_form_control_layout_delimited.scss index 75c5a942d52..56d4dcfada7 100644 --- a/src/components/form/form_control_layout/_form_control_layout_delimited.scss +++ b/src/components/form/form_control_layout/_form_control_layout_delimited.scss @@ -5,7 +5,7 @@ align-items: stretch; padding: 1px; /* 1 */ - .euiFormControlLayoutDelimited__delimeter { + .euiFormControlLayoutDelimited__delimiter { background-color: $euiFormBackgroundColor; } @@ -44,7 +44,7 @@ &[class*='-isDisabled'] { @include euiFormControlDisabledStyle; - .euiFormControlLayoutDelimited__delimeter { + .euiFormControlLayoutDelimited__delimiter { background-color: $euiFormBackgroundDisabledColor; } } @@ -54,7 +54,7 @@ @include euiFormControlReadOnlyStyle; input, - .euiFormControlLayoutDelimited__delimeter { + .euiFormControlLayoutDelimited__delimiter { background-color: $euiFormBackgroundReadOnlyColor; } } @@ -82,7 +82,7 @@ min-width: 0; // Fixes FF } -.euiFormControlLayoutDelimited__delimeter { +.euiFormControlLayoutDelimited__delimiter { // stylelint-disable-next-line declaration-no-important line-height: 1 !important; // Override EuiText line-height flex: 0 0 auto; diff --git a/src/components/form/form_control_layout/form_control_layout_delimited.tsx b/src/components/form/form_control_layout/form_control_layout_delimited.tsx index 683df9f655f..a27b5e9a4b2 100644 --- a/src/components/form/form_control_layout/form_control_layout_delimited.tsx +++ b/src/components/form/form_control_layout/form_control_layout_delimited.tsx @@ -52,7 +52,7 @@ export const EuiFormControlLayoutDelimited: FunctionComponent {addClassesToControl(startControl)} diff --git a/src/components/form/range/__snapshots__/dual_range.test.tsx.snap b/src/components/form/range/__snapshots__/dual_range.test.tsx.snap index 1a703bac6fb..c0e10cb5c5e 100644 --- a/src/components/form/range/__snapshots__/dual_range.test.tsx.snap +++ b/src/components/form/range/__snapshots__/dual_range.test.tsx.snap @@ -537,7 +537,7 @@ exports[`EuiDualRange props loading should display when showInput="inputWithPopo value="1" />
@@ -620,7 +620,7 @@ exports[`EuiDualRange props slider should display in popover 1`] = ` value="1" />
From 99188e6039c3b9b198abc90617c77564bf611a8e Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 4 Apr 2023 15:45:26 -0700 Subject: [PATCH 2/9] [EuiFormControlLayoutDelimited] Change default delimiter to an icon instead of text - to more correctly matches the current Figma designs + add a screen reader label --- ...orm_control_layout_delimited.test.tsx.snap | 7 +++- .../_form_control_layout_delimited.scss | 11 +++--- .../form_control_layout_delimited.tsx | 35 +++++++++++++------ .../__snapshots__/dual_range.test.tsx.snap | 14 ++++++-- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap b/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap index 9c8b1cfb5a2..3f396138333 100644 --- a/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap +++ b/src/components/form/form_control_layout/__snapshots__/form_control_layout_delimited.test.tsx.snap @@ -17,7 +17,12 @@ exports[`EuiFormControlLayoutDelimited is rendered 1`] = `
- → + + to +
= ({ startControl, endControl, - delimiter = '→', + delimiter, className, ...rest }) => { @@ -51,23 +54,35 @@ export const EuiFormControlLayoutDelimited: FunctionComponent {addClassesToControl(startControl)} - - {delimiter} - + {addClassesToControl(endControl)} ); }; -function addClassesToControl(control: ReactElement) { +const addClassesToControl = (control: ReactElement) => { return cloneElement(control, { className: classNames( control.props.className, 'euiFormControlLayoutDelimited__input' ), }); -} +}; + +const EuiFormControlDelimiter = ({ delimiter }: { delimiter?: ReactNode }) => { + const classes = classNames('euiFormControlLayoutDelimited__delimiter'); + const color = 'subdued'; + + const defaultAriaLabel = useEuiI18n( + 'euiFormControlLayoutDelimited.delimiterLabel', + 'to' + ); + + return ( + + {delimiter ?? ( + + )} + + ); +}; diff --git a/src/components/form/range/__snapshots__/dual_range.test.tsx.snap b/src/components/form/range/__snapshots__/dual_range.test.tsx.snap index c0e10cb5c5e..c6b133f5574 100644 --- a/src/components/form/range/__snapshots__/dual_range.test.tsx.snap +++ b/src/components/form/range/__snapshots__/dual_range.test.tsx.snap @@ -539,7 +539,12 @@ exports[`EuiDualRange props loading should display when showInput="inputWithPopo
- → + + to +
- → + + to +
Date: Tue, 4 Apr 2023 15:46:50 -0700 Subject: [PATCH 3/9] [EuiFormControlLayoutDelimited] Add better `isInvalid` styling - color arrow + extend line under arrow and icons + fix background colors of icons --- .../_form_control_layout_delimited.scss | 22 +++++++++++----- .../form_control_layout_delimited.tsx | 26 +++++++++++++++---- src/components/form/range/dual_range.tsx | 1 + 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/components/form/form_control_layout/_form_control_layout_delimited.scss b/src/components/form/form_control_layout/_form_control_layout_delimited.scss index 006b7554429..8d69778b008 100644 --- a/src/components/form/form_control_layout/_form_control_layout_delimited.scss +++ b/src/components/form/form_control_layout/_form_control_layout_delimited.scss @@ -5,7 +5,8 @@ align-items: stretch; padding: 1px; /* 1 */ - .euiFormControlLayoutDelimited__delimiter { + .euiFormControlLayoutDelimited__delimiter, + .euiFormControlLayoutIcons { background-color: $euiFormBackgroundColor; } @@ -28,7 +29,6 @@ } .euiFormControlLayoutIcons { - padding-left: $euiFormControlCompressedPadding; padding-right: $euiFormControlCompressedPadding; } } @@ -44,7 +44,8 @@ &[class*='-isDisabled'] { @include euiFormControlDisabledStyle; - .euiFormControlLayoutDelimited__delimiter { + .euiFormControlLayoutDelimited__delimiter, + .euiFormControlLayoutIcons { background-color: $euiFormBackgroundDisabledColor; } } @@ -54,7 +55,8 @@ @include euiFormControlReadOnlyStyle; input, - .euiFormControlLayoutDelimited__delimiter { + .euiFormControlLayoutDelimited__delimiter, + .euiFormControlLayoutIcons { background-color: $euiFormBackgroundReadOnlyColor; } } @@ -63,13 +65,18 @@ // Absolutely positioning the icons doesn't work because they // overlay only one of controls making the layout unbalanced position: static; // Overrider absolute - padding-left: $euiFormControlPadding; padding-right: $euiFormControlPadding; + align-self: stretch; + flex-grow: 0; &:not(.euiFormControlLayoutIcons--right) { order: -1; } } + + &--isInvalid .euiFormControlLayoutIcons { + @include euiFormControlInvalidStyle; + } } .euiFormControlLayoutDelimited__input { @@ -84,9 +91,12 @@ .euiFormControlLayoutDelimited__delimiter { align-self: stretch; - height: auto; flex-grow: 0; display: flex; align-items: center; line-height: 1; // Override EuiText line-height + + &--isInvalid { + @include euiFormControlInvalidStyle; + } } diff --git a/src/components/form/form_control_layout/form_control_layout_delimited.tsx b/src/components/form/form_control_layout/form_control_layout_delimited.tsx index f03d08780da..43ec8a79a20 100644 --- a/src/components/form/form_control_layout/form_control_layout_delimited.tsx +++ b/src/components/form/form_control_layout/form_control_layout_delimited.tsx @@ -49,12 +49,20 @@ export const EuiFormControlLayoutDelimited: FunctionComponent { - const classes = classNames('euiFormControlLayoutDelimited', className); + const { isInvalid, isDisabled, readOnly } = rest; + const showInvalidState = isInvalid && !isDisabled && !readOnly; + + const classes = classNames('euiFormControlLayoutDelimited', className, { + 'euiFormControlLayoutDelimited--isInvalid': showInvalidState, + }); return ( {addClassesToControl(startControl)} - + {addClassesToControl(endControl)} ); @@ -69,9 +77,17 @@ const addClassesToControl = (control: ReactElement) => { }); }; -const EuiFormControlDelimiter = ({ delimiter }: { delimiter?: ReactNode }) => { - const classes = classNames('euiFormControlLayoutDelimited__delimiter'); - const color = 'subdued'; +const EuiFormControlDelimiter = ({ + delimiter, + isInvalid, +}: { + delimiter?: ReactNode; + isInvalid?: boolean; +}) => { + const classes = classNames('euiFormControlLayoutDelimited__delimiter', { + 'euiFormControlLayoutDelimited__delimiter--isInvalid': isInvalid, + }); + const color = isInvalid ? 'danger' : 'subdued'; const defaultAriaLabel = useEuiI18n( 'euiFormControlLayoutDelimited.delimiterLabel', diff --git a/src/components/form/range/dual_range.tsx b/src/components/form/range/dual_range.tsx index 0d1592827d8..1316539409c 100644 --- a/src/components/form/range/dual_range.tsx +++ b/src/components/form/range/dual_range.tsx @@ -755,6 +755,7 @@ export class EuiDualRangeClass extends Component< append={append} prepend={prepend} isLoading={isLoading} + isInvalid={isInvalid} /> } fullWidth={fullWidth} From efa4089fde754b2ff4e5b3d6caa903b7b59c66e4 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 10 Apr 2023 12:57:45 -0700 Subject: [PATCH 4/9] [EuiDualRange] Fix buggy styling when `isInvalid` and `showInput="inputWithPopover"` - the input was rendering the padding offset for the invalid icon, without actually rendering said icons (due to `controlOnly` prop) --- src/components/form/field_number/field_number.tsx | 10 ++++++---- src/components/form/field_text/field_text.tsx | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/components/form/field_number/field_number.tsx b/src/components/form/field_number/field_number.tsx index 777e287f379..fcfd9c1307d 100644 --- a/src/components/form/field_number/field_number.tsx +++ b/src/components/form/field_number/field_number.tsx @@ -99,10 +99,12 @@ export const EuiFieldNumber: FunctionComponent = ( ...rest } = props; - const numIconsClass = getFormControlClassNameForIconCount({ - isInvalid, - isLoading, - }); + const numIconsClass = controlOnly + ? false + : getFormControlClassNameForIconCount({ + isInvalid, + isLoading, + }); const classes = classNames('euiFieldNumber', className, numIconsClass, { 'euiFieldNumber--withIcon': icon, diff --git a/src/components/form/field_text/field_text.tsx b/src/components/form/field_text/field_text.tsx index 5c75564de71..3a40234d5ed 100644 --- a/src/components/form/field_text/field_text.tsx +++ b/src/components/form/field_text/field_text.tsx @@ -78,10 +78,12 @@ export const EuiFieldText: FunctionComponent = (props) => { ...rest } = props; - const numIconsClass = getFormControlClassNameForIconCount({ - isInvalid, - isLoading, - }); + const numIconsClass = controlOnly + ? false + : getFormControlClassNameForIconCount({ + isInvalid, + isLoading, + }); const classes = classNames('euiFieldText', className, numIconsClass, { 'euiFieldText--withIcon': icon, From 37c35ea5ebfea0a069396207767dba65403e809b Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 10 Apr 2023 18:09:40 -0700 Subject: [PATCH 5/9] [EuiFieldNumber] Fix browser invalid state not showing an icon or setting `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 --- .../__snapshots__/field_number.test.tsx.snap | 21 ++++++- .../form/field_number/field_number.spec.tsx | 55 +++++++++++++++++++ .../form/field_number/field_number.test.tsx | 53 ++++++++++-------- .../form/field_number/field_number.tsx | 49 ++++++++++++++--- .../validatable_control.test.tsx | 14 +++++ .../validatable_control.tsx | 2 +- 6 files changed, 161 insertions(+), 33 deletions(-) create mode 100644 src/components/form/field_number/field_number.spec.tsx diff --git a/src/components/form/field_number/__snapshots__/field_number.test.tsx.snap b/src/components/form/field_number/__snapshots__/field_number.test.tsx.snap index cb7de7ebd8f..ae5a51c949d 100644 --- a/src/components/form/field_number/__snapshots__/field_number.test.tsx.snap +++ b/src/components/form/field_number/__snapshots__/field_number.test.tsx.snap @@ -6,10 +6,12 @@ exports[`EuiFieldNumber is rendered 1`] = ` fullwidth="false" icon="warning" inputid="1" + isinvalid="false" isloading="false" > `; @@ -38,18 +42,21 @@ exports[`EuiFieldNumber props fullWidth is rendered 1`] = ` `; -exports[`EuiFieldNumber props isInvalid is rendered 1`] = ` +exports[`EuiFieldNumber props isInvalid is rendered from a prop 1`] = ` @@ -71,12 +80,15 @@ exports[`EuiFieldNumber props isLoading is rendered 1`] = ` @@ -86,14 +98,17 @@ exports[`EuiFieldNumber props readOnly is rendered 1`] = ` @@ -103,10 +118,12 @@ exports[`EuiFieldNumber props value no initial value 1`] = ` +/// +/// + +import React from 'react'; +import { EuiFieldNumber } from './field_number'; + +describe('EuiFieldNumber', () => { + describe('isNativelyInvalid', () => { + const checkIsValid = () => { + cy.get('[aria-invalid="true"]').should('not.exist'); + cy.get('.euiFormControlLayoutIcons').should('not.exist'); + }; + const checkIsInvalid = () => { + cy.get('[aria-invalid="true"]').should('exist'); + cy.get('.euiFormControlLayoutIcons').should('exist'); + }; + + it('when the value is not a valid number', () => { + cy.mount(); + checkIsValid(); + cy.get('input').click().realType('-.'); + checkIsInvalid(); + }); + + it('sets invalid state when the value is less than the passed min', () => { + cy.mount(); + checkIsValid(); + cy.get('input').click().type('-10'); + checkIsInvalid(); + }); + + it('sets invalid state when the value is greater than the passed max', () => { + cy.mount(); + checkIsValid(); + cy.get('input').click().type('101'); + checkIsInvalid(); + }); + + it('sets invalid state when the value is not a valid step', () => { + cy.mount(); + checkIsValid(); + cy.get('input').click().type('2'); + checkIsInvalid(); + }); + }); +}); diff --git a/src/components/form/field_number/field_number.test.tsx b/src/components/form/field_number/field_number.test.tsx index a734c4be1cd..5ce42c0715e 100644 --- a/src/components/form/field_number/field_number.test.tsx +++ b/src/components/form/field_number/field_number.test.tsx @@ -7,7 +7,7 @@ */ import React from 'react'; -import { render } from 'enzyme'; +import { render } from '../../../test/rtl'; import { requiredProps } from '../../../test/required_props'; import { EuiForm } from '../form'; @@ -26,7 +26,7 @@ jest.mock('../validatable_control', () => ({ describe('EuiFieldNumber', () => { test('is rendered', () => { - const component = render( + const { container } = render( { /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); describe('props', () => { - test('isInvalid is rendered', () => { - const component = render(); + test('isInvalid is rendered from a prop', () => { + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); test('fullWidth is rendered', () => { - const component = render(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); test('isLoading is rendered', () => { - const component = render(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); test('readOnly is rendered', () => { - const component = render(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); test('controlOnly is rendered', () => { - const component = render(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); + }); + + test('inputRef', () => { + const inputRef = jest.fn(); + const { container } = render(); + + expect(inputRef).toHaveBeenCalledTimes(1); + expect(container.querySelector('input[type="number"]')).toBe( + inputRef.mock.calls[0][0] + ); }); describe('value', () => { test('value is number', () => { - const component = render( + const { container } = render( {}} /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); test('no initial value', () => { - const component = render( + const { container } = render( {}} /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); }); describe('inherits', () => { test('fullWidth from ', () => { - const component = render( + const { container } = render( ); + const control = container.querySelector('.euiFieldNumber')!; - if ( - !component.find('.euiFieldNumber').hasClass('euiFieldNumber--fullWidth') - ) { + if (!control.classList.contains('euiFieldNumber--fullWidth')) { throw new Error( 'expected EuiFieldNumber to inherit fullWidth from EuiForm' ); diff --git a/src/components/form/field_number/field_number.tsx b/src/components/form/field_number/field_number.tsx index fcfd9c1307d..24f9c21a28f 100644 --- a/src/components/form/field_number/field_number.tsx +++ b/src/components/form/field_number/field_number.tsx @@ -6,20 +6,27 @@ * Side Public License, v 1. */ -import React, { InputHTMLAttributes, Ref, FunctionComponent } from 'react'; +import React, { + InputHTMLAttributes, + Ref, + FunctionComponent, + useState, + useRef, + useCallback, +} from 'react'; import { CommonProps } from '../../common'; import classNames from 'classnames'; +import { useCombinedRefs } from '../../../services'; +import { IconType } from '../../icon'; + +import { EuiValidatableControl } from '../validatable_control'; import { EuiFormControlLayout, EuiFormControlLayoutProps, } from '../form_control_layout'; - -import { EuiValidatableControl } from '../validatable_control'; - -import { IconType } from '../../icon'; -import { useFormContext } from '../eui_form_context'; import { getFormControlClassNameForIconCount } from '../form_control_layout/_num_icons'; +import { useFormContext } from '../eui_form_context'; export type EuiFieldNumberProps = Omit< InputHTMLAttributes, @@ -96,13 +103,35 @@ export const EuiFieldNumber: FunctionComponent = ( inputRef, readOnly, controlOnly, + onKeyDown: _onKeyDown, ...rest } = props; + // Attempt to determine additional invalid state. The native number input + // will set :invalid state automatically, but we need to also set + // `aria-invalid` as well as display an icon. We also want to *not* set this on + // EuiValidatableControl, in order to not override custom validity messages + const [isNativelyInvalid, setIsNativelyInvalid] = useState(false); + const validityRef = useRef(null); + const setRefs = useCombinedRefs([validityRef, inputRef]); + + // Note that we can't use hook into `onChange` because browsers don't emit change events + // for invalid values - see https://github.com/facebook/react/issues/16554 + const onKeyDown = useCallback( + (e: React.KeyboardEvent) => { + _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] + ); + const numIconsClass = controlOnly ? false : getFormControlClassNameForIconCount({ - isInvalid, + isInvalid: isInvalid || isNativelyInvalid, isLoading, }); @@ -126,7 +155,9 @@ export const EuiFieldNumber: FunctionComponent = ( placeholder={placeholder} readOnly={readOnly} className={classes} - ref={inputRef} + ref={setRefs} + onKeyDown={onKeyDown} + aria-invalid={isInvalid || isNativelyInvalid} {...rest} /> @@ -141,7 +172,7 @@ export const EuiFieldNumber: FunctionComponent = ( icon={icon} fullWidth={fullWidth} isLoading={isLoading} - isInvalid={isInvalid} + isInvalid={isInvalid || isNativelyInvalid} compressed={compressed} readOnly={readOnly} prepend={prepend} diff --git a/src/components/form/validatable_control/validatable_control.test.tsx b/src/components/form/validatable_control/validatable_control.test.tsx index 494adb72575..0c1972d461c 100644 --- a/src/components/form/validatable_control/validatable_control.test.tsx +++ b/src/components/form/validatable_control/validatable_control.test.tsx @@ -26,6 +26,20 @@ describe('EuiValidatableControl', () => { expect(component).toMatchSnapshot(); }); + test('aria-invalid allows falling back to prop set on the child input', () => { + const component = render( + + + + ); + + expect(component).toMatchInlineSnapshot(` + + `); + }); + describe('ref management', () => { it('calls a ref function', () => { const ref = jest.fn(); diff --git a/src/components/form/validatable_control/validatable_control.tsx b/src/components/form/validatable_control/validatable_control.tsx index d8439747a9b..1ae8932911f 100644 --- a/src/components/form/validatable_control/validatable_control.tsx +++ b/src/components/form/validatable_control/validatable_control.tsx @@ -68,7 +68,7 @@ export const EuiValidatableControl: FunctionComponent< return cloneElement(child, { ref: replacedRef, - 'aria-invalid': isInvalid, + 'aria-invalid': isInvalid || child.props['aria-invalid'], }); }; From a14047d7719cc66458c993227667b83d9c9c1c60 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 10 Apr 2023 15:22:34 -0700 Subject: [PATCH 6/9] [EuiRange/EuiDualRange] Improve UX of input widths on EuiRange/EuiDualRange - 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 --- .../__snapshots__/dual_range.test.tsx.snap | 16 ++- .../range/__snapshots__/range.test.tsx.snap | 5 +- .../__snapshots__/range_input.test.tsx.snap | 21 ++++ src/components/form/range/dual_range.tsx | 3 - src/components/form/range/range.tsx | 2 - .../form/range/range_input.test.tsx | 114 ++++++++++++++++++ src/components/form/range/range_input.tsx | 65 ++++++++-- 7 files changed, 203 insertions(+), 23 deletions(-) create mode 100644 src/components/form/range/__snapshots__/range_input.test.tsx.snap create mode 100644 src/components/form/range/range_input.test.tsx diff --git a/src/components/form/range/__snapshots__/dual_range.test.tsx.snap b/src/components/form/range/__snapshots__/dual_range.test.tsx.snap index c6b133f5574..759b2fa8f74 100644 --- a/src/components/form/range/__snapshots__/dual_range.test.tsx.snap +++ b/src/components/form/range/__snapshots__/dual_range.test.tsx.snap @@ -63,13 +63,14 @@ exports[`EuiDualRange input props can be applied to min and max inputs 1`] = ` class="euiFormControlLayout__childrenWrapper" > @@ -112,13 +113,14 @@ exports[`EuiDualRange input props can be applied to min and max inputs 1`] = ` class="euiFormControlLayout__childrenWrapper" > @@ -321,13 +323,14 @@ exports[`EuiDualRange props inputs should render 1`] = ` class="euiFormControlLayout__childrenWrapper" > @@ -372,13 +375,14 @@ exports[`EuiDualRange props inputs should render 1`] = ` class="euiFormControlLayout__childrenWrapper" > @@ -528,6 +532,7 @@ exports[`EuiDualRange props loading should display when showInput="inputWithPopo class="euiFormControlLayout__childrenWrapper" > @@ -354,6 +355,7 @@ exports[`EuiRange props loading should display when showInput="inputWithPopover" class="euiFormControlLayout__childrenWrapper" > +
+ +
+ +`; diff --git a/src/components/form/range/dual_range.tsx b/src/components/form/range/dual_range.tsx index 1316539409c..fdcf835382f 100644 --- a/src/components/form/range/dual_range.tsx +++ b/src/components/form/range/dual_range.tsx @@ -466,7 +466,6 @@ export class EuiDualRangeClass extends Component< const { id } = this.state; - const digitTolerance = Math.max(String(min).length, String(max).length); const showInputOnly = showInput === 'inputWithPopover'; const canShowDropdown = showInputOnly && !readOnly && !disabled; @@ -479,7 +478,6 @@ export class EuiDualRangeClass extends Component< aria-label={this.props['aria-label']} {...minInputProps} // Non-overridable props - digitTolerance={digitTolerance} side="min" min={min} max={Number(this.upperValue)} @@ -510,7 +508,6 @@ export class EuiDualRangeClass extends Component< aria-label={this.props['aria-label']} {...maxInputProps} // Non-overridable props - digitTolerance={digitTolerance} side="max" min={Number(this.lowerValue)} max={max} diff --git a/src/components/form/range/range.tsx b/src/components/form/range/range.tsx index 5f1f4f30d6a..622ed3b8aca 100644 --- a/src/components/form/range/range.tsx +++ b/src/components/form/range/range.tsx @@ -147,7 +147,6 @@ export class EuiRangeClass extends Component< const { id } = this.state; - const digitTolerance = Math.max(String(min).length, String(max).length); const showInputOnly = showInput === 'inputWithPopover'; const canShowDropdown = showInputOnly && !readOnly && !disabled; @@ -156,7 +155,6 @@ export class EuiRangeClass extends Component< id={id} min={min} max={max} - digitTolerance={digitTolerance} step={step} value={value} readOnly={readOnly} diff --git a/src/components/form/range/range_input.test.tsx b/src/components/form/range/range_input.test.tsx new file mode 100644 index 00000000000..17516aa71e2 --- /dev/null +++ b/src/components/form/range/range_input.test.tsx @@ -0,0 +1,114 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { render } from '../../../test/rtl'; +import { shouldRenderCustomStyles } from '../../../test/internal'; + +import { EuiRangeInput } from './range_input'; + +const requiredProps = { + min: 0, + max: 100, + value: 0, + onChange: () => {}, +}; + +describe('EuiRangeInput', () => { + shouldRenderCustomStyles(); + + it('renders', () => { + const { container } = render(); + expect(container.firstChild).toMatchSnapshot(); + }); + + describe('widthStyle', () => { + it('does not set a width style if the `autoSize` is set to false', () => { + const { container } = render( + + ); + const widthStyle = container + .querySelector('.euiRangeInput') + ?.getAttribute('style'); + + expect(widthStyle).toBeFalsy(); + }); + + it('increases input character width dynamically based on value', () => { + const { rerender, container } = render( + + ); + const getWidthStyle = () => + container.querySelector('.euiRangeInput')?.getAttribute('style'); + + expect(getWidthStyle()).toMatchInlineSnapshot( + '"inline-size: calc(12px + 1ch + 2em + 0px);"' + ); + + rerender(); + expect(getWidthStyle()).toMatchInlineSnapshot( + '"inline-size: calc(12px + 2ch + 2em + 0px);"' + ); + + rerender(); + expect(getWidthStyle()).toMatchInlineSnapshot( + '"inline-size: calc(12px + 3ch + 2em + 0px);"' + ); + + rerender(); + expect(getWidthStyle()).toMatchInlineSnapshot( + '"inline-size: calc(12px + 4ch + 2em + 22px);"' + ); + + // Should not go above 4 characters in width + rerender(); + expect(getWidthStyle()).toMatchInlineSnapshot( + '"inline-size: calc(12px + 4ch + 2em + 22px);"' + ); + }); + + test('compressed', () => { + const { container } = render( + + ); + const widthStyle = container + .querySelector('.euiRangeInput') + ?.getAttribute('style'); + + expect(widthStyle).toMatchInlineSnapshot( + '"inline-size: calc(8px + 1ch + 2em + 0px);"' + ); + }); + + test('invalid', () => { + const { container } = render( + + ); + const widthStyle = container + .querySelector('.euiRangeInput') + ?.getAttribute('style'); + + expect(widthStyle).toMatchInlineSnapshot( + '"inline-size: calc(12px + 1ch + 2em + 22px);"' + ); + }); + + test('invalid + compressed', () => { + const { container } = render( + + ); + const widthStyle = container + .querySelector('.euiRangeInput') + ?.getAttribute('style'); + + expect(widthStyle).toMatchInlineSnapshot( + '"inline-size: calc(8px + 1ch + 2em + 18px);"' + ); + }); + }); +}); diff --git a/src/components/form/range/range_input.tsx b/src/components/form/range/range_input.tsx index 621aa5b1370..ad3d41271d2 100644 --- a/src/components/form/range/range_input.tsx +++ b/src/components/form/range/range_input.tsx @@ -6,10 +6,17 @@ * Side Public License, v 1. */ -import React, { FunctionComponent, useMemo } from 'react'; +import React, { + FunctionComponent, + useState, + useEffect, + useMemo, + useRef, +} from 'react'; -import { useEuiTheme } from '../../../services'; +import { useEuiTheme, useCombinedRefs } from '../../../services'; import { logicalStyles } from '../../../global_styling'; +import { euiFormVariables } from '../form.styles'; import { EuiFieldNumber, EuiFieldNumberProps } from '../field_number'; import type { _SingleRangeValue, _SharedRangeInputSide } from './types'; @@ -20,7 +27,6 @@ export interface EuiRangeInputProps Omit<_SingleRangeValue, 'onChange'>, _SharedRangeInputSide { autoSize?: boolean; - digitTolerance: number; } export const EuiRangeInput: FunctionComponent = ({ @@ -28,29 +34,60 @@ export const EuiRangeInput: FunctionComponent = ({ max, step, value, + inputRef, + isInvalid, disabled, compressed, onChange, name, side = 'max', - digitTolerance, fullWidth, autoSize = true, ...rest }) => { - // Chrome will properly size the input based on the max value, but FF does not. - // Calculate the width of the input based on highest number of characters. - // Add 2 to accommodate for input stepper - const widthStyle = useMemo(() => { - return autoSize - ? logicalStyles({ width: `${digitTolerance / 1.25 + 2}em` }) - : {}; - }, [autoSize, digitTolerance]); - const euiTheme = useEuiTheme(); const styles = euiRangeInputStyles(euiTheme); const cssStyles = [styles.euiRangeInput]; + // Determine whether an invalid icon is showing, which can come from + // the underlying EuiFieldNumber's native :invalid state + const [hasInvalidIcon, setHasInvalidIcon] = useState(isInvalid); + const validityRef = useRef(null); + const setRefs = useCombinedRefs([validityRef, inputRef]); + + useEffect(() => { + const isNativelyInvalid = !validityRef.current?.validity.valid; + setHasInvalidIcon(isNativelyInvalid || isInvalid); + }, [value, isInvalid]); + + // Calculate the auto size width of the input + const widthStyle = useMemo(() => { + if (!autoSize) return undefined; + + // Calculate the number of characters to show (dynamic based on user input) + // Uses the min/max char length as a max, then add an extra UX buffer of 1 + const maxChars = Math.max(String(min).length, String(max).length) + 1; + const inputCharWidth = Math.min(String(value).length, maxChars); + + // Calculate the form padding based on `compressed` state + const { controlPadding, controlCompressedPadding } = euiFormVariables( + euiTheme + ); + const inputPadding = compressed ? controlCompressedPadding : controlPadding; + + // Calculate the invalid icon (if being displayed), also based on `compressed` state + const invalidIconWidth = hasInvalidIcon + ? euiTheme.euiTheme.base * (compressed ? 1.125 : 1.375) // TODO: DRY this out once EuiFormControlLayoutIcons is converted to Emotion + : 0; + + // Guesstimate a width for the stepper. Note that it's a little wider in FF than it is in Chrome + const stepperWidth = 2; + + return logicalStyles({ + width: `calc(${inputPadding} + ${inputCharWidth}ch + ${stepperWidth}em + ${invalidIconWidth}px)`, + }); + }, [autoSize, euiTheme, compressed, hasInvalidIcon, min, max, value]); + return ( = ({ max={Number(max)} step={step} value={value === '' ? '' : Number(value)} + inputRef={setRefs} + isInvalid={isInvalid} disabled={disabled} compressed={compressed} onChange={onChange} From e45249ab2b08c85424dc45735e5dd0f7a62316b7 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 10 Apr 2023 18:52:23 -0700 Subject: [PATCH 7/9] changelog --- upcoming_changelogs/6704.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 upcoming_changelogs/6704.md diff --git a/upcoming_changelogs/6704.md b/upcoming_changelogs/6704.md new file mode 100644 index 00000000000..3af1488d1bd --- /dev/null +++ b/upcoming_changelogs/6704.md @@ -0,0 +1,3 @@ +- Updated `EuiFieldNumber` to detect native browser invalid state and show an invalid icon +- Improved the input widths of `EuiRange` and `EuiDualRange` when `showInput={true}` to account for invalid icons +- Improved the `isInvalid` styling of `EuiDualRange` when `showInput="inputWithPopover"` From 8d3d59649bcadb54f5b5a6617a79ef5ef7bc6ed2 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 11 Apr 2023 12:24:00 -0700 Subject: [PATCH 8/9] Revert horizontal padding on delimited icons - after playing more with date range picker as well as a broader variety of delimited inputs, this change was too specific to EuiDualRange --- .../form_control_layout/_form_control_layout_delimited.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/form/form_control_layout/_form_control_layout_delimited.scss b/src/components/form/form_control_layout/_form_control_layout_delimited.scss index 8d69778b008..635e1087baf 100644 --- a/src/components/form/form_control_layout/_form_control_layout_delimited.scss +++ b/src/components/form/form_control_layout/_form_control_layout_delimited.scss @@ -29,7 +29,7 @@ } .euiFormControlLayoutIcons { - padding-right: $euiFormControlCompressedPadding; + padding-inline: $euiFormControlCompressedPadding; } } @@ -65,7 +65,7 @@ // Absolutely positioning the icons doesn't work because they // overlay only one of controls making the layout unbalanced position: static; // Overrider absolute - padding-right: $euiFormControlPadding; + padding-inline: $euiFormControlPadding; align-self: stretch; flex-grow: 0; From 1c25fd7ff7775bb1beb831c55c3383961b728b9d Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 11 Apr 2023 12:49:24 -0700 Subject: [PATCH 9/9] snapshot --- .../__snapshots__/quick_select_popover.test.tsx.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/date_picker/super_date_picker/quick_select_popover/__snapshots__/quick_select_popover.test.tsx.snap b/src/components/date_picker/super_date_picker/quick_select_popover/__snapshots__/quick_select_popover.test.tsx.snap index 9c8a08945f7..09b2b778b60 100644 --- a/src/components/date_picker/super_date_picker/quick_select_popover/__snapshots__/quick_select_popover.test.tsx.snap +++ b/src/components/date_picker/super_date_picker/quick_select_popover/__snapshots__/quick_select_popover.test.tsx.snap @@ -134,6 +134,7 @@ exports[`EuiQuickSelectPanels customQuickSelectPanels should render custom panel >