-
-
Notifications
You must be signed in to change notification settings - Fork 251
Add missing loading UI for products and categories in Boilerplate (#10736) #11286
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.
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
Add missing loading UI for products and categories in Boilerplate (#10736) #11286
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds loading UI indicators to Products and Categories pages and triggers immediate UI refresh when loading starts. Updates component providers to call StateHasChanged after setting isLoading. Minor formatting change in a dialog component. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Page as Categories/Products Page
participant Provider as Grid Data Provider
participant Controller as Category/Product Controller
User->>Page: Trigger grid load / filter / add
Page->>Provider: Invoke data load
Provider->>Provider: isLoading = true
Provider->>Page: StateHasChanged()
Page->>User: Show loading indicators
Provider->>Controller: Fetch data (OData query)
Controller-->>Provider: Data/result
Provider->>Provider: isLoading = false
Provider->>Page: (Products: no final StateHasChanged) / (Categories: final StateHasChanged)
Page-->>User: Render data
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs (1)
69-77
: Escape OData string literals to prevent broken filters and possible query manipulation.If ProductNameFilter or CategoryNameFilter contains single quotes, the generated $filter will break (or be exploitable for OData filter injection). Escape by doubling single quotes and simplify the null/empty checks for readability.
Apply this diff:
- if (string.IsNullOrEmpty(ProductNameFilter) is false) + if (!string.IsNullOrEmpty(ProductNameFilter)) { - odataQ.Filter = $"contains(tolower({nameof(ProductDto.Name)}),'{ProductNameFilter.ToLower()}')"; + var nameFilterValue = ProductNameFilter.ToLower().Replace("'", "''"); + odataQ.Filter = $"contains(tolower({nameof(ProductDto.Name)}),'{nameFilterValue}')"; } - if (string.IsNullOrEmpty(CategoryNameFilter) is false) + if (!string.IsNullOrEmpty(CategoryNameFilter)) { - odataQ.AndFilter = $"contains(tolower({nameof(ProductDto.CategoryName)}),'{CategoryNameFilter.ToLower()}')"; + var catFilterValue = CategoryNameFilter.ToLower().Replace("'", "''"); + odataQ.AndFilter = $"contains(tolower({nameof(ProductDto.CategoryName)}),'{catFilterValue}')"; }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Categories/CategoriesPage.razor.cs (1)
53-56
: Escape OData filter value and simplify check.Avoid broken OData filters with single quotes in CategoryNameFilter; also simplify the condition.
- if (string.IsNullOrEmpty(CategoryNameFilter) is false) + if (!string.IsNullOrEmpty(CategoryNameFilter)) { - odataQ.Filter = $"contains(tolower({nameof(CategoryDto.Name)}),'{CategoryNameFilter.ToLower()}')"; + var nameFilterValue = CategoryNameFilter.ToLower().Replace("'", "''"); + odataQ.Filter = $"contains(tolower({nameof(CategoryDto.Name)}),'{nameFilterValue}')"; }
🧹 Nitpick comments (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs (2)
58-58
: Triggering an early re-render is good; schedule it safely via InvokeAsync(StateHasChanged).Directly calling StateHasChanged can race with disposal or resume on a non-UI thread after awaits. Prefer scheduling renders via InvokeAsync to avoid threading/disposal issues and to keep consistency with other Blazor patterns.
Apply this diff:
- StateHasChanged(); + _ = InvokeAsync(StateHasChanged);
86-90
: Treat cancellations as non-errors to reduce UX flicker and noise.When a new query starts, the grid cancels the previous request. Handling OperationCanceledException like a regular error can clear the grid and surface error handling unnecessarily.
Split exception handling:
- catch (Exception exp) - { - ExceptionHandler.Handle(exp); - return BitDataGridItemsProviderResult.From(new List<ProductDto> { }, 0); - } + catch (OperationCanceledException) + { + // Swallow cancellation to avoid error noise/flicker. + return BitDataGridItemsProviderResult.From(Array.Empty<ProductDto>(), 0); + } + catch (Exception exp) + { + ExceptionHandler.Handle(exp); + return BitDataGridItemsProviderResult.From(new List<ProductDto> { }, 0); + }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor (1)
14-27
: Nice UX: in-header loader surfaces state early; consider small a11y enhancement.The nested stacks and BitSlickBarsLoading next to Add Product look good and match Categories. Consider adding an accessible status text or ensure the loader component exposes role/status for screen readers. If not, wrap it with a polite aria-live region.
Example:
@if (isLoading) { - <BitSlickBarsLoading CustomSize="32" /> + <span role="status" aria-live="polite" style="display:flex;align-items:center"> + <BitSlickBarsLoading CustomSize="32" /> + <span class="sr-only">Loading products…</span> + </span> }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Categories/CategoriesPage.razor.cs (2)
42-42
: Early re-render: use InvokeAsync(StateHasChanged) for safety and consistency.Same rationale as Products: schedule the render to avoid threading/disposal hazards.
- StateHasChanged(); + _ = InvokeAsync(StateHasChanged);
62-66
: Handle cancellations separately to avoid error handling on expected scenarios.Consistent with Products, avoid treating OperationCanceledException as an error to reduce flicker and unnecessary notifications.
- catch (Exception exp) - { - ExceptionHandler.Handle(exp); - return BitDataGridItemsProviderResult.From(new List<CategoryDto> { }, 0); - } + catch (OperationCanceledException) + { + return BitDataGridItemsProviderResult.From(Array.Empty<CategoryDto>(), 0); + } + catch (Exception exp) + { + ExceptionHandler.Handle(exp); + return BitDataGridItemsProviderResult.From(new List<CategoryDto> { }, 0); + }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Categories/CategoriesPage.razor (1)
12-22
: Good UX parity: Add button with inline loader matches the Products pattern.Looks good. For consistency with other handlers using WrapHandled, consider wrapping CreateCategory, and optionally add an aria-live wrapper around the loader if BitSlickBarsLoading isn’t already accessible.
- <BitStack Horizontal FitHeight> - <BitButton ReversedIcon - OnClick="CreateCategory" + <BitStack Horizontal FitHeight> + <BitButton ReversedIcon + OnClick="WrapHandled(CreateCategory)" IconName="@BitIconName.Add"> @Localizer[nameof(AppStrings.AddCategory)] </BitButton> @if (isLoading) { - <BitSlickBarsLoading CustomSize="32" /> + <span role="status" aria-live="polite" style="display:flex;align-items:center"> + <BitSlickBarsLoading CustomSize="32" /> + <span class="sr-only">Loading categories…</span> + </span> } </BitStack>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (5)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/SignOutConfirmDialog.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Categories/CategoriesPage.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Categories/CategoriesPage.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor
(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build and test
🔇 Additional comments (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor.cs (1)
91-95
: AI summary inconsistency: final StateHasChanged() still exists here.The AI summary says the trailing StateHasChanged() was removed, but it’s still present after isLoading = false. Confirm the intended behavior.
If the intention is to keep it, consider also scheduling it safely as below:
finally { isLoading = false; - StateHasChanged(); + _ = InvokeAsync(StateHasChanged); }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/SignOutConfirmDialog.razor (1)
19-19
: Formatting-only change looks good.Trailing space before the self-closing tag is harmless; no functional impact.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Products/ProductsPage.razor (1)
95-95
: Minor formatting tweak is fine.Href line spacing change is purely cosmetic.
closes #10736
Summary by CodeRabbit