Pickers: Add InputId parameter#12003
Conversation
mudbot
bot
added
the
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consistently adds an InputId parameter to various picker components, which is a valuable enhancement for accessibility and component interaction. The implementation includes corresponding test components and unit tests, ensuring the new functionality is covered.
I've identified a critical architectural issue regarding the placement of the new InputId property. Its current location in MudPicker<T> will lead to a compilation error in MudDateRangePicker because the internal MudRangeInput component does not inherit from MudPicker<T>. I've provided a detailed comment on how to resolve this by moving the property to a more suitable base class.
Additionally, I've made a suggestion to improve the MudRangeInput by ensuring both of its input fields receive an id, which would make the component more robust and consistent.
Overall, this is a positive contribution. Addressing these points will ensure the new feature is correctly and completely implemented.
versile2
left a comment
versile2
left a comment
There was a problem hiding this comment.
Did we need to create 4 additional UnitTests.Viewer files instead of adding the id to existing test files and test those? Let me know if any questions/comments on my comments.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for setting custom input element IDs in MudBlazor picker components through a new InputId parameter. This enables better accessibility and testability by allowing developers to specify custom IDs for input elements.
- Adds
InputIdparameter toMudPickerbase class - Implements ID suffixing for range pickers (adds
-startand-endsuffixes) - Updates all relevant test components to support the new parameter
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/MudBlazor/Components/Picker/MudPicker.razor.cs | Adds InputId property with documentation explaining usage for both single and range pickers |
| src/MudBlazor/Components/Picker/MudPicker.razor | Passes InputId to the internal MudTextField component |
| src/MudBlazor/Components/Input/MudRangeInput.razor | Implements ID suffixing logic for start and end inputs in range components |
| src/MudBlazor/Components/DatePicker/MudDateRangePicker.razor | Passes InputId parameter to MudRangeInput component |
| src/MudBlazor.UnitTests/Components/TimePickerTests.cs | Adds test verifying InputId functionality for time picker |
| src/MudBlazor.UnitTests/Components/DateRangePickerTests.cs | Adds test verifying InputId with -start and -end suffixes for range picker |
| src/MudBlazor.UnitTests/Components/DatePickerTests.cs | Adds test verifying InputId functionality for date picker |
| src/MudBlazor.UnitTests/Components/ColorPickerTests.cs | Adds test verifying InputId functionality for color picker |
| src/MudBlazor.UnitTests.Viewer/TestComponents/TimePicker/SimpleTimePickerTest.razor | Adds InputId parameter to test component |
| src/MudBlazor.UnitTests.Viewer/TestComponents/DatePicker/SimpleMudMudDateRangePickerTest.razor | Adds InputId parameter to test component |
| src/MudBlazor.UnitTests.Viewer/TestComponents/DatePicker/SimpleMudDatePickerTest.razor | Adds InputId parameter to test component |
| src/MudBlazor.UnitTests.Viewer/TestComponents/ColorPicker/SimpleColorPickerTest.razor | Adds InputId parameter to test component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you! |
Pickers: Add InputId parameter
Add
InputIdparameter to all picker components for consistency with other input components and to allow binding labels or targeting picker inputs.Affects: