Follow-up review fixes for MudBlazor PR #25393#25483
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR follows up on prior MudBlazor-related review feedback by improving initial loading UX in the Setting Management page, adding accessibility labels to search fields, and adjusting the MudBlazor page progress service’s show/hide behavior.
Changes:
- Add an explicit
_loadingstate toSettingManagementto show a spinner during async initialization and avoid “stuck” loading based onSelectedGroup. - Add
aria-labelattributes to MudBlazor search inputs to improve screen-reader accessibility. - Refactor
MudBlazorUiPageProgressServiceto track visibility and default options (but currently introduces non-thread-safe state updates).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/setting-management/src/Volo.Abp.SettingManagement.Blazor.MudBlazor/Pages/SettingManagement/SettingManagement.razor.cs | Introduces _loading state toggled during async initialization. |
| modules/setting-management/src/Volo.Abp.SettingManagement.Blazor.MudBlazor/Pages/SettingManagement/SettingManagement.razor | Uses _loading to render a spinner and conditions tab rendering on available groups. |
| modules/permission-management/src/Volo.Abp.PermissionManagement.Blazor.MudBlazor/Components/PermissionManagementModal.razor | Adds aria-label to the permission group search field. |
| modules/identity/src/Volo.Abp.Identity.Blazor.MudBlazor/Pages/Identity/UserManagement.razor | Adds aria-label to the user list search field. |
| framework/src/Volo.Abp.MudBlazorUI/MudBlazorUiPageProgressService.cs | Refactors progress notification logic; current implementation has concurrency issues around shared mutable state. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rel-10.4 #25483 +/- ##
=========================================
Coverage 49.42% 49.42%
=========================================
Files 3670 3670
Lines 123599 123599
Branches 9453 9453
=========================================
+ Hits 61083 61090 +7
+ Misses 60682 60671 -11
- Partials 1834 1838 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ismcagdas
approved these changes
May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues to address Copilot review comments left on #25393 and on the dev merge PR #25479.