From de99f06b472b350cba6f43d960505f430769fb9e Mon Sep 17 00:00:00 2001 From: Or Nuri Date: Mon, 15 Apr 2024 11:37:27 +0300 Subject: [PATCH] feat: Dropdown and TextFeild accessibility improvements (#1898) --- .../combobox-snapshot-tests.jest.js.snap | 48 ++++++++----- .../core/src/components/Dropdown/Dropdown.jsx | 33 ++++++++- .../components/Dropdown/DropdownConstants.js | 2 + .../dropdown-snapshot-tests.jest.js.snap | 63 +++++++++++++++++ .../Dropdown/components/menu/menu.jsx | 15 +++- .../Dropdown/components/option/option.jsx | 12 +++- .../search-snapshot-tests.jest.js.snap | 37 ++++++---- .../src/components/TextField/TextField.tsx | 39 ++++++++--- .../textField-snapshot-tests.jest.js.snap | 70 +++++++++++++------ 9 files changed, 253 insertions(+), 66 deletions(-) diff --git a/packages/core/src/components/Combobox/__tests__/__snapshots__/combobox-snapshot-tests.jest.js.snap b/packages/core/src/components/Combobox/__tests__/__snapshots__/combobox-snapshot-tests.jest.js.snap index c3794a99ab..19ebd9424d 100644 --- a/packages/core/src/components/Combobox/__tests__/__snapshots__/combobox-snapshot-tests.jest.js.snap +++ b/packages/core/src/components/Combobox/__tests__/__snapshots__/combobox-snapshot-tests.jest.js.snap @@ -28,9 +28,10 @@ exports[`Combobox renders correctly when disabled 1`] = ` > ( - + ), - [dropdownMenuWrapperClassName, menuRenderer] + [dropdownMenuWrapperClassName, menuRenderer, menuId, menuAriaLabel] ); const DropdownIndicator = useCallback(props => , [size]); @@ -192,7 +202,10 @@ const Dropdown = forwardRef( [finalOptionRenderer, optionWrapperClassName] ); - const Input = useCallback(props => , []); + const Input = useCallback( + props => , + [menuId] + ); const SingleValue = useCallback( props => ( @@ -334,6 +347,10 @@ const Dropdown = forwardRef( aria-readonly={readOnly} aria-label={overrideAriaLabel} aria-details={tooltipContent} + aria-expanded={!readOnly && menuIsOpen} + aria-haspopup="listbox" + aria-activedescendant + role="combobox" defaultValue={defaultValue} value={value} onMenuOpen={onMenuOpen} @@ -406,6 +423,8 @@ Dropdown.defaultProps = { tabIndex: "0", onOptionRemove: undefined, id: DROPDOWN_ID, + menuId: DROPDOWN_MENU_ID, + menuAriaLabel: DROPDOWN_MENU_ARIA_LABEL, autoFocus: false, closeMenuOnSelect: undefined, closeMenuOnScroll: false, @@ -599,6 +618,14 @@ Dropdown.propTypes = { * ID for the select container */ id: PropTypes.string, + /** + * ID for the menu container + */ + menuId: PropTypes.string, + /** + * aria-label attribute for the menu container + */ + menuAriaLabel: PropTypes.string, /** * focusAuto when component mount */ diff --git a/packages/core/src/components/Dropdown/DropdownConstants.js b/packages/core/src/components/Dropdown/DropdownConstants.js index 39bd886060..d36a81f45f 100644 --- a/packages/core/src/components/Dropdown/DropdownConstants.js +++ b/packages/core/src/components/Dropdown/DropdownConstants.js @@ -3,6 +3,8 @@ export const defaultCustomStyles = baseStyles => baseStyles; export const ADD_AUTO_HEIGHT_COMPONENTS = ["container", "control", "valueContainer"]; export const DROPDOWN_ID = "dropdown-menu-id"; +export const DROPDOWN_MENU_ID = "dropdown-menu-list-id"; +export const DROPDOWN_MENU_ARIA_LABEL = "Dropdown menu"; export const DROPDOWN_CHIP_COLORS = { PRIMARY: "PRIMARY", diff --git a/packages/core/src/components/Dropdown/__tests__/__snapshots__/dropdown-snapshot-tests.jest.js.snap b/packages/core/src/components/Dropdown/__tests__/__snapshots__/dropdown-snapshot-tests.jest.js.snap index 7d3b3360d7..2337f53106 100644 --- a/packages/core/src/components/Dropdown/__tests__/__snapshots__/dropdown-snapshot-tests.jest.js.snap +++ b/packages/core/src/components/Dropdown/__tests__/__snapshots__/dropdown-snapshot-tests.jest.js.snap @@ -40,6 +40,7 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if >
{ - const rendererProps = { children, selectProps, ...props }; +const Menu = ({ children, Renderer, selectProps, dropdownMenuWrapperClassName, id, ariaLabel, ...props }) => { + const rendererProps = { + children, + selectProps, + ...props, + innerProps: { + ...props.innerProps, + id, + role: "listbox", + "aria-label": ariaLabel + } + }; + const withFixedPosition = selectProps?.selectProps?.insideOverflowContainer || selectProps?.selectProps?.insideOverflowWithTransformContainer; diff --git a/packages/core/src/components/Dropdown/components/option/option.jsx b/packages/core/src/components/Dropdown/components/option/option.jsx index 54dd72d7a9..a2df696094 100644 --- a/packages/core/src/components/Dropdown/components/option/option.jsx +++ b/packages/core/src/components/Dropdown/components/option/option.jsx @@ -7,7 +7,17 @@ import styles from "./option.module.scss"; const Option = ({ Renderer, data, children, optionWrapperClassName, ...props }) => { const tooltipProps = data?.tooltipProps || {}; - const rendererProps = { children, data, ...props }; + const rendererProps = { + children, + data, + ...props, + innerProps: { + ...props.innerProps, + role: "option", + "aria-selected": props.isSelected + } + }; + return ( {Renderer ? ( diff --git a/packages/core/src/components/Search/__tests__/__snapshots__/search-snapshot-tests.jest.js.snap b/packages/core/src/components/Search/__tests__/__snapshots__/search-snapshot-tests.jest.js.snap index 60d8af4b87..4a7785b4dc 100644 --- a/packages/core/src/components/Search/__tests__/__snapshots__/search-snapshot-tests.jest.js.snap +++ b/packages/core/src/components/Search/__tests__/__snapshots__/search-snapshot-tests.jest.js.snap @@ -14,9 +14,10 @@ exports[`Search renders correctly when disabled 1`] = ` > & { trim = false, role = "", required = false, + requiredErrorText = "", loading = false, requiredAsterisk = false, dataTestId: backwardCompatibilityDataTestId, @@ -140,6 +142,8 @@ const TextField: VibeComponent & { }, ref ) => { + const [isRequiredAndEmpty, setIsRequiredAndEmpty] = useState(false); + const overrideDataTestId = backwardCompatibilityForProperties( [dataTestId, backwardCompatibilityDataTestId], getTestId(ComponentDefaultTestId.TEXT_FIELD, id) @@ -147,11 +151,24 @@ const TextField: VibeComponent & { const inputRef = useRef(null); const mergedRef = useMergeRef(ref, inputRef, setRef); + const onBlurCallback = useCallback( + (e: React.FocusEvent) => { + if (required && !e.target.value) { + setIsRequiredAndEmpty(true); + } + onBlur(e); + }, + [onBlur, required] + ); + const onChangeCallback = useCallback( (value: string) => { + if (isRequiredAndEmpty && value) { + setIsRequiredAndEmpty(false); + } onChange(value, { target: inputRef.current }); }, - [onChange] + [onChange, isRequiredAndEmpty] ); const { inputValue, onEventChanged, clearValue } = useDebounceEvent({ @@ -185,14 +202,15 @@ const TextField: VibeComponent & { }, [inputRef, disabled, clearOnIconClick, onIconClick, currentStateIconName, clearValue]); const validationClass = useMemo(() => { - if (!validation || !validation.status) { + if ((!validation || !validation.status) && !isRequiredAndEmpty) { return ""; } - return FEEDBACK_CLASSES[validation.status]; - }, [validation]); + const status = isRequiredAndEmpty ? "error" : validation.status; + return FEEDBACK_CLASSES[status]; + }, [validation, isRequiredAndEmpty]); const hasIcon = iconName || secondaryIconName; - const shouldShowExtraText = showCharCount || (validation && validation.text); + const shouldShowExtraText = showCharCount || (validation && validation.text) || isRequiredAndEmpty; const isSecondary = secondaryIconName === currentStateIconName; const isPrimary = iconName === currentStateIconName; const shouldFocusOnSecondaryIcon = secondaryIconName && isSecondary && !!inputValue; @@ -246,16 +264,17 @@ const TextField: VibeComponent & { id={id} data-testid={overrideDataTestId} name={name} - onBlur={onBlur} + onBlur={onBlurCallback} onFocus={onFocus} onKeyDown={onKeyDown} onWheel={onWheel} maxLength={maxLength} role={searchResultsContainerId && "combobox"} // For voice reader aria-label={inputAriaLabel || placeholder} - aria-invalid={validation && validation.status === "error"} + aria-invalid={(validation && validation.status === "error") || isRequiredAndEmpty} aria-owns={searchResultsContainerId} aria-activedescendant={activeDescendant} + aria-required={required} required={required} tabIndex={tabIndex} /> @@ -313,7 +332,9 @@ const TextField: VibeComponent & { {shouldShowExtraText && ( {validation && validation.text && ( - {validation.text} + + {isRequiredAndEmpty ? requiredErrorText : validation.text} + )} {showCharCount && ( diff --git a/packages/core/src/components/TextField/__tests__/__snapshots__/textField-snapshot-tests.jest.js.snap b/packages/core/src/components/TextField/__tests__/__snapshots__/textField-snapshot-tests.jest.js.snap index e9ba5b8ffc..1f064fb67b 100644 --- a/packages/core/src/components/TextField/__tests__/__snapshots__/textField-snapshot-tests.jest.js.snap +++ b/packages/core/src/components/TextField/__tests__/__snapshots__/textField-snapshot-tests.jest.js.snap @@ -14,9 +14,10 @@ exports[`TextField renders correctly when disabled 1`] = ` >