[autocomplete] Wrap the no results message in a status element#48690
[autocomplete] Wrap the no results message in a status element#48690silviuaavram wants to merge 8 commits into
Conversation
Deploy previewhttps://deploy-preview-48690--material-ui.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
There was a problem hiding this comment.
Pull request overview
This PR updates Autocomplete to improve assistive-technology announcements when there are no matching options by introducing a new noOptions slot that provides a live-region status container around the existing “no options” content.
Changes:
- Add a
noOptionsslot (andslotProps.noOptions) and wrap the no-options content in arole="status"live region (aria-live="polite",aria-atomic="true"). - Extend TypeScript typings (slot + overrides augmentation) and PropTypes to include the new slot.
- Add/adjust tests and regenerate API docs reflecting the new slot.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mui-material/src/Autocomplete/Autocomplete.js | Adds noOptions slot via useSlot and wraps the no-options message in a status live region. |
| packages/mui-material/src/Autocomplete/Autocomplete.d.ts | Adds noOptions to AutocompleteSlots and slot-props typing + overrides interface. |
| packages/mui-material/src/Autocomplete/Autocomplete.spec.tsx | Adds TS coverage for slots.noOptions / slotProps.noOptions usage (including ref). |
| packages/mui-material/src/Autocomplete/Autocomplete.test.js | Adds tests around noOptionsText rendering and the status container behavior. |
| docs/translations/api-docs/autocomplete/autocomplete.json | Updates slot descriptions to include noOptions. |
| docs/pages/material-ui/api/autocomplete.json | Updates API docs output for the new noOptions slot and slotProps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
docs/translations/api-docs/autocomplete/autocomplete.json:323
- In the translations JSON,
noOptionshas been moved fromclassDescriptionstoslotDescriptions, implyingnoOptionsis now a slot (and no longer a documented utility class). However, the current implementation still appliesclasses.noOptionsto the inner message wrapper (<AutocompleteNoOptions className={classes.noOptions} ...>), while the newnoOptionsslot container has no class.
This makes the generated docs inconsistent with the actual DOM/class placement. Consider aligning the implementation with the slot documentation (or vice-versa) and regenerating these files.
"slotDescriptions": {
"clearIndicator": "The component used to render the clear indicator element.",
"listbox": "The component used to render the listbox.",
"noOptions": "The component used to render the "no options" container.",
"paper": "The component used to render the body of the popup.",
"popper": "The component used to position the popup.",
"popupIndicator": "The component used to render the popup indicator element.",
"root": "The component that renders the root."
}
| role: 'status', | ||
| 'aria-live': 'polite', | ||
| 'aria-atomic': 'true', |
There was a problem hiding this comment.
I think the loading text would also benefit from being inside this, though it can be done separately
We should come up with a better name for the slot though, maybe status or liveRegion
There was a problem hiding this comment.
@silviuaavram Is it possible to have existing paper slot having these attributes?
There was a problem hiding this comment.
not possible since we don't want to announce all the changes in the listbox.
| ) : null} | ||
| <NoOptionsSlot {...noOptionsProps}> | ||
| {renderedOptions.length === 0 && !freeSolo && !loading ? ( | ||
| <AutocompleteNoOptions |
There was a problem hiding this comment.
@siriwatknp Do you think this (internal) DOM change is ok?
The only risk I can think of is it would break brittle CSS selectors like > .NoOptions which seems ok
There was a problem hiding this comment.
I think > .NoOptions is a very edge case. I don't see why consumers would target it like that.
For me I'd frame it like this:
- a fix? if yes, I think it's okay to risk in minor release
- an enhancement? if yes and a lot of upvotes, it's okay to risk in minor release
- otherwise, I'd wait or find a different solution.
e7139c4 to
29583ca
Compare
Fixes #48666
Add a new
noOptionsContainerslot that wraps the "no options" content with a container element. The container renders withrole="status",aria-live="polite", andaria-atomic="true"so assistive technologies announce the absence of options without requiring the popup to be open.Changes
noOptionsContainerslot viauseSlotbacked by a plain'div'(no styled wrapper needed)noOptionsContainerCSS class to the container element so consumers can style itnoOptionsContainertoautocompleteClassesandAutocompleteClassesAutocompleteNoOptionsContainerSlotPropsOverridesaugmentation interfacenoOptionsContainertoAutocompleteSlotsandAutocompleteSlotsAndSlotPropsslots(noOptionsContainer) andslotProps(noOptionsContainer)