MudAutocomplete: ProgressBar AdornmentIcon Hiding#11980
MudAutocomplete: ProgressBar AdornmentIcon Hiding#11980danielchalmers merged 2 commits intoMudBlazor:devfrom
Conversation
mudbot
bot
added
the
bug
![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 correctly adjusts the logic for hiding the adornment icon in MudAutocomplete when a progress indicator is shown. The changes ensure the icon is only hidden for end adornments and only when the default circular progress indicator is used, which aligns with the stated goals. A new visual test component has been added to demonstrate the new behavior across various configurations.
My review includes two main points:
- The new test component is missing some XML documentation for public members, which is required by the style guide.
- I've suggested adding bUnit tests to cover the new conditional styling logic introduced in the SCSS file, which would help prevent future regressions and aligns with the project's testing requirements for logic changes.
src/MudBlazor.UnitTests.Viewer/TestComponents/Autocomplete/AutoCompleteProgressTest.razor
Show resolved
Hide resolved
| &:not(:has(.mud-input-adornment-start)):has(.progress-indicator-circular) .mud-select-input .mud-icon-button { | ||
| display: none !important; |
There was a problem hiding this comment.
This CSS change introduces conditional rendering logic based on the presence of certain classes. While the new visual test component is helpful, it would be beneficial to add bUnit tests to programmatically verify this behavior and prevent future regressions. For example, you could add tests to AutocompleteTests.cs that assert the adornment icon is hidden or visible under different conditions (e.g., Adornment.Start vs Adornment.End, default progress indicator vs. custom template). The style guide requires unit tests for all logic changes.1
Style Guide References
Footnotes
-
The style guide requires that all logic changes must include corresponding unit tests. ↩
There was a problem hiding this comment.
Pretty sure I can't bunit css change?
There was a problem hiding this comment.
Yeah, not without conditionally rendering it. I wonder why display: none was used initially instead, but it probably makes sense to leave it since this is just a minor patch
There was a problem hiding this comment.
This is actually the preferred method if my understanding is correct, you would only do conditional rendering in the event of older browsers which Blazor doesn't support. This one results in slightly less CSS. BUT the downside is this way is usually a bit more complex.
danielchalmers
changed the title
2 participants
Previously when you set ShowProgressIndicator was true it would always hide the icon regardless of situation. This PR changes that since the progress indicator is always at the end (RTL doesn't matter here) then we only hide the adornmenticon if it's at the end. Also added logic to only hide if it contains a circular bar, so if a user does their own template it won't hide (Most use the underline method)
Resolves #11574
Before:
After:
Checklist:
dev).