Feat(Dropdown): Enhance accessibility by exposing combobox role directly to the span label (Fixes #7584) #7585
Conversation
…ectly to the span label (fixes primefaces#7584)
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
| const inputRef = React.useRef(props.inputRef); | ||
| const labelRef = React.useRef(props.labelRef); |
There was a problem hiding this comment.
Differentiating label and input makes sense for me as they represent different elements. In the current implementation, inputRef assigned to the span element.
| setTimeout(() => { | ||
| const currentValue = inputRef.current ? inputRef.current.value : undefined; |
There was a problem hiding this comment.
Tested the previous issue with onBlur event and couldn't reproduce it. It's working correctly without timeout and calling with the value.
Screen.Recording.2025-01-08.at.13.26.21.mov
| const optionValue = getOptionValue(selectedOption); | ||
| const inputValue = getInputValue(selectedOption); |
There was a problem hiding this comment.
The entire option should be passed as the event value, but the target's value should provide a readable value since exposing an object isn't suitable for forms.
| const focus = React.useCallback( | ||
| (autoFocus) => { | ||
| if (props.editable) { | ||
| DomHandler.focus(inputRef.current, autoFocus); | ||
| } else { | ||
| DomHandler.focus(focusInputRef.current, autoFocus); | ||
| } | ||
| }, | ||
| [props.editable] | ||
| ); |
There was a problem hiding this comment.
Centralizing the focus logic for better maintainability.
| DomHandler.alignOverlay(overlayRef.current, inputRef.current.parentElement, props.appendTo || (context && context.appendTo) || PrimeReact.appendTo); | ||
| DomHandler.alignOverlay(overlayRef.current, elementRef.current, props.appendTo || (context && context.appendTo) || PrimeReact.appendTo); |
There was a problem hiding this comment.
No need to reference the input's parent since we already have a wrapper element.
| React.useEffect(() => { | ||
| const labelElement = ariaLabelledBy ? document.getElementById(ariaLabelledBy) : null; | ||
|
|
||
| const handleClick = () => { | ||
| focus(); | ||
|
|
||
| if (!overlayVisibleState) { | ||
| show(); | ||
| } | ||
| }; | ||
|
|
||
| labelElement?.addEventListener('click', handleClick); | ||
|
|
||
| return () => { | ||
| labelElement?.removeEventListener('click', handleClick); | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps -- show and focus are not required to be in the dependency array | ||
| }, [ariaLabelledBy, overlayVisibleState]); |
There was a problem hiding this comment.
This workaround is necessary because the combobox is a span element now.
| if (props.editable) { | ||
| value = value || props.value || ''; | ||
| return null; | ||
| } |
There was a problem hiding this comment.
No need to manage an additional input when the dropdown is editable.
| ariaLabel: undefined, | ||
| ariaLabelledBy: undefined, |
There was a problem hiding this comment.
Using undefined as the default value keeps a cleaner DOM by preventing unnecessary attributes from being added to the element when they are not provided.
melloware
added
the
Status: Pending Review
Fix #7584. Please take a look for more details.
Here's a summary of the changes included in this PR:
role="combobox"directly to the visiblespanlabel or an editableinputelement.selectelement.disabled,invalid, etc.Demo:
Screen.Recording.2025-01-08.at.13.mp4