Skip to content

[material-ui][select] Fix assigning of id to Select #47498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sai6855 wants to merge 4 commits into from
Closed

[material-ui][select] Fix assigning of id to Select #47498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43d03b1
fix
sai6855 Dec 16, 2025
27c7b3e
fix
sai6855 Dec 16, 2025
e9b3239
Update TextField.test.js
sai6855 Dec 17, 2025
edcd215
Merge branch 'master' into select-id
sai6855 Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mui-material/src/TextField/TextField.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,10 @@ const TextField = React.forwardRef(function TextField(inProps, ref) {
{select ? (
<SelectSlot
aria-describedby={helperTextId}
id={id}
Copy link
Member Author

@sai6855 sai6855 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this id as it was sending to div role='combobox'

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a breaking change. We cannot remove the id of the combobox. The id passed on Select is of div combobox.
See the comment #38869 (comment):

I would not remove or change the id on the div itself, though.

See discussions in #38869 and #47489

labelId={inputLabelId}
value={value}
input={InputElement}
inputProps={{ id }}
{...selectProps}
Comment on lines 258 to 264
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the id prop from SelectSlot will break the visible combobox element's id attribute.

In the Select component, the id prop is used to populate SelectDisplayProps.id, which becomes the id of the visible combobox element (via buttonId in SelectInput.js line 480). By only passing id through inputProps, it ends up on the hidden input element but NOT on the visible combobox.

The fix should pass BOTH:

  • id={id} as a direct prop (for the visible combobox via SelectDisplayProps)
  • inputProps={{ id }} (for the hidden input element)

This ensures the visible combobox maintains its proper id for accessibility (label association, etc.) while also fixing the original issue of the hidden input not receiving the id.

Copilot uses AI. Check for mistakes.
>
{children}
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/TextField/TextField.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ describe('<TextField />', () => {
);

const input = container.querySelector('input[aria-hidden]');
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this assertion removal is correct (the hidden input should now have an id attribute after the fix), a new assertion should be added to verify that the visible combobox still maintains its correct id. This would help catch the regression where the combobox loses its id attribute.

Consider adding an assertion like:
expect(screen.getByRole('combobox')).to.have.attribute('id');

Additionally, you should verify that both the visible combobox and the hidden input have the SAME id value when an explicit id prop is passed to TextField, to ensure proper form submission behavior.

Copilot uses AI. Check for mistakes.
expect(input).not.to.have.attribute('id');
expect(input).to.have.attribute('id');
expect(input).not.to.have.attribute('aria-describedby');
});

Expand Down
Loading