Add LoadMore feature to BitBasicList (#10996) #11015
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 WalkthroughThe changes introduce a "LoadMore" feature to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BitBasicList
participant ItemsProvider
participant UI
User->>BitBasicList: Triggers LoadMore (e.g., clicks button)
alt ItemsProvider is set
BitBasicList->>ItemsProvider: Request next batch (skip, size)
ItemsProvider-->>BitBasicList: Returns items
else
BitBasicList->>BitBasicList: Slice Items collection (skip, size)
end
BitBasicList->>UI: Update _viewItems and loading state
UI-->>User: Render new items and updated LoadMore button
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.scss (1)
12-19: Add visual feedback & disabled style for load-more trigger
.bit-bsl-lmtis clickable (cursor: pointer) but has no:hover,:active, or disabled states. A minimal hover style improves affordance; a[disabled]style prevents misleading cursor when loading..bit-bsl-lmt { width: 100%; display: flex; cursor: pointer; padding: spacing(2); align-items: center; justify-content: center; + &:hover:not([disabled]) { + background-color: rgba(0,0,0,.04); + } + &[disabled] { + cursor: default; + opacity: .5; + } }src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/BlazorParameter.cs (1)
15-16: Make async callback name immutable & constructor-supplied
All other fields are initialised through the primary constructor; leavingCallOnSetAsyncMethodNamemutable invites accidental nulls. Consider extending the constructor and making the set accessorinit.-public string? CallOnSetAsyncMethodName { get; set; } +public string? CallOnSetAsyncMethodName { get; init; }and add a sixth constructor parameter.
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs (1)
8-11: Consider thread-safe access to _globalCtsWhile Blazor components typically execute on a single thread, the
_globalCtsfield is accessed from multiple methods (RefreshDataAsync,PerformLoadMore,DisposeAsync). Consider usingInterlocked.CompareExchangefor atomic operations if there's any possibility of concurrent access.For more robust thread safety:
private async Task PerformLoadMore(bool reset) { // ... existing reset logic ... if (LoadMore is false || _globalCts is not null) return; var localCts = new CancellationTokenSource(); - _globalCts = localCts; + var existingCts = Interlocked.CompareExchange(ref _globalCts, localCts, null); + if (existingCts is not null) + { + localCts.Dispose(); + return; // Another operation is already in progress + }Also applies to: 189-192, 253-261
📜 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 (13)
src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/BlazorParameter.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSourceGenerator.cs(5 hunks)src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSyntaxContextReceiver.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Tests/Components/Lists/BasicList/BitBasicListTest.razor(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs(4 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.scss(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/_BitBasicListVirtualize.cs(1 hunks)src/BlazorUI/Bit.BlazorUI/Utils/CallOnSetAsyncAttribute.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor.cs(4 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor.samples.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor.scss(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (13)
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.scss (1)
8-11: Considerdisplay: contentsaccessibility limitations
display: contentsremoves the element from the accessibility tree in several browsers (notably Safari ≤ 17 and some AT combinations). If.bit-bsl-lmbever carries ARIA roles/attributes, they will be lost.-.bit-bsl-lmb { - display: contents; -} +.bit-bsl-lmb { + /* Fallback keeps element in the tree for non-supporting UAs */ + display: block; + @supports (display: contents) { + display: contents; + } +}Please verify that losing the element from the AX tree does not break list semantics and test in Safari.
src/BlazorUI/Bit.BlazorUI.Tests/Components/Lists/BasicList/BitBasicListTest.razor (1)
1-1: Ensure all tests & samples use the newVirtualizeparameter
The rename looks correct here, but several demos/tests referencedEnableVirtualizationpreviously. A repo-wide search avoids silent regressions.rg -n "EnableVirtualization"src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor.scss (1)
1-11: LGTM – SCSS nesting only
The change is purely syntactic; compiled CSS is unchanged.src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/_BitBasicListVirtualize.cs (1)
38-47: Good consistency improvement!Using the open generic type
Virtualize<>innameofexpressions is cleaner and more maintainable than specifying the full generic typeVirtualize<TItem>. This change improves code consistency without affecting functionality.src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSyntaxContextReceiver.cs (1)
47-52: Well-implemented async callback support!The implementation correctly follows the existing pattern for handling attributes. The null-safe extraction of the async callback method name is properly handled.
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor (2)
11-49: Well-structured rendering logic for LoadMore and virtualization!The implementation properly handles all rendering scenarios:
- Virtualization enabled/disabled
- LoadMore mode with internal items management
- ItemsProvider delegation
- Empty content display suppression during loading
The conditional logic is clear and covers all cases appropriately.
51-72: Comprehensive LoadMore button implementation!The LoadMore button rendering correctly:
- Respects the
_loadMoreFinishedflag to hide when all items are loaded- Supports custom templates via
LoadMoreTemplate- Provides default loading indicator with "..."
- Handles click events to trigger
PerformLoadMoresrc/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSourceGenerator.cs (1)
60-60: Excellent async support implementation!The async patterns are correctly implemented:
SetParametersAsyncproperly returnsTaskand usesasync- Async callbacks are conditionally awaited only when values change
- Base method calls are consistently awaited
- Two-way binding
Assignmethods support async callbacksThe implementation follows async/await best practices throughout.
Also applies to: 107-110, 128-139, 169-172
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor.samples.cs (1)
1-456: Comprehensive demo samples covering all features!The demo code effectively showcases:
- Basic and virtualized rendering
- Async ItemsProvider with HTTP data fetching
- LoadMore functionality with various configurations
- Custom templates and styling
- RTL support
The examples provide clear patterns for developers to follow.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor (1)
14-253: Demo examples effectively showcase the new LoadMore functionality!The reorganization and new examples provide comprehensive coverage of the BitBasicList features, including basic usage, virtualization, ItemsProvider, and the new LoadMore functionality with various configurations.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/BasicList/BitBasicListDemo.razor.cs (1)
70-235: Well-structured demo implementation with proper async patterns!The new LoadMore parameters are properly documented, and the async data providers effectively simulate real-world scenarios with appropriate delays and pagination handling.
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs (2)
150-178: Well-implemented parameter change detection!The
OnParametersSetAsyncmethod properly detects changes to bothItemsandItemsProviderparameters and initiates the LoadMore process when appropriate. The priority given toItemsProvideroverItemsis a good design decision.
181-231: Add comprehensive exception handling in PerformLoadMoreThe method only catches
OperationCanceledException. If any other exception occurs (e.g., network errors from ItemsProvider), it won't be caught, potentially leaving_globalCtsin an inconsistent state.Consider wrapping the entire try block logic in an additional try-catch:
try { StateHasChanged(); try { if (ItemsProvider is null) { var items = Items ?? []; _viewItems = [.. _viewItems, .. items.Skip(_loadMoreSkip).Take(LoadMoreSize)]; _loadMoreFinished = _viewItems.Count >= items.Count; } else { var result = await ProvideVirtualizedItems(new(_loadMoreSkip, LoadMoreSize, localCts.Token)); if (localCts.IsCancellationRequested is false) { _viewItems = [.. _viewItems, .. result.Items]; _loadMoreFinished = _viewItems.Count >= result.TotalItemCount; } } _loadMoreSkip += LoadMoreSize; } catch (OperationCanceledException oce) when (oce.CancellationToken == localCts.Token) { } + catch (Exception ex) + { + // Log the exception or handle it appropriately + // You might want to set an error state or notify the user + throw; // Re-throw after cleanup in finally block + } } finally { _globalCts = null; localCts.Dispose(); }Likely an incorrect or invalid review comment.
msynk
force-pushed
the
10996-blazorui-basiclist-loadmore
branch
from
412bb38 to
8bbf60e
Compare
2 participants
closes #10996
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Refactor
Style