Add BitInfiniteScrolling extra component (#9643)#9646
Add BitInfiniteScrolling extra component (#9643)#9646msynk merged 7 commits intobitfoundation:developfrom
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 WalkthroughThis pull request introduces a new Changes
Assessment against linked issues
Sequence DiagramsequenceDiagram
participant User
participant InfiniteScrolling
participant ItemsProvider
participant JavaScript
User->>InfiniteScrolling: Scroll near bottom
InfiniteScrolling->>JavaScript: Trigger load more items
JavaScript->>ItemsProvider: Request additional items
ItemsProvider-->>InfiniteScrolling: Return new items
InfiniteScrolling->>User: Render new items
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 7
🔭 Outside diff range comments (2)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts (1)
Line range hint
30-52: Replace non-standard scrollIntoViewIfNeeded with cross-browser alternative.The method uses
scrollIntoViewIfNeededwhich is non-standard and not supported in all browsers.Replace with a cross-browser alternative:
- colOptions.scrollIntoViewIfNeeded(); + // Use standard scrollIntoView with options + colOptions.scrollIntoView({ + block: 'nearest', + inline: 'nearest' + });Also, consider improving type safety:
- public static checkColumnOptionsPosition(tableElement: any) { + public static checkColumnOptionsPosition(tableElement: HTMLTableElement) { - const colOptions = tableElement.tHead && tableElement.tHead.querySelector('.col-options'); + const colOptions = tableElement.tHead?.querySelector<HTMLElement>('.col-options');src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReader.ts (1)
Line range hint
77-108: Add error handling for canvas operationsThe renderPage method should include error handling for canvas operations:
public static async renderPage(id: string, pageNumber: number) { const config = PdfReader._bitPdfReaders.get(id); if (!config || !config.pdfDoc) return; if (config.isRendering[pageNumber]) return; if (pageNumber < 1 || pageNumber > config.pdfDoc.numPages) return; config.isRendering[pageNumber] = true; try { const page = await config.pdfDoc.getPage(pageNumber); const pixelRatio = (config.autoScale && window.devicePixelRatio) || 1; const scale = config.scale * pixelRatio; const viewport = page.getViewport({ scale: scale }); let canvas = document.getElementById(config.id) as HTMLCanvasElement; if (!canvas) { canvas = document.getElementById(`${config.id}-${pageNumber}`) as HTMLCanvasElement; } if (!canvas) return; const context = canvas.getContext('2d'); + if (!context) { + throw new Error('Failed to get 2D context from canvas'); + } - canvas.width = viewport.width; - canvas.height = viewport.height; + try { + canvas.width = viewport.width; + canvas.height = viewport.height; + } catch (error) { + console.error('Failed to set canvas dimensions:', error); + return; + } canvas.style.width = `${viewport.width / pixelRatio}px`; canvas.style.height = `${viewport.height / pixelRatio}px`;
🧹 Nitpick comments (14)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts (3)
2-2: Consider refactoring static-only class to standalone functions.The
DataGridclass contains only static members. Consider refactoring it into standalone functions for better maintainability and reduced complexity.Example refactor:
namespace BitBlazorUI { - export class DataGrid { - public static init(tableElement: any) { + export function initDataGrid(tableElement: any) { // ... implementation - } } - public static checkColumnOptionsPosition(tableElement: any) { + export function checkDataGridColumnOptionsPosition(tableElement: any) { // ... implementation - } } - private static enableColumnResizing(tableElement: any) { + function enableDataGridColumnResizing(tableElement: any) { // ... implementation - } } - } }🧰 Tools
🪛 Biome (1.9.4)
[error] 2-100: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
Line range hint
3-28: Improve type safety in the init method.The method uses
anytype which reduces type safety. Consider using more specific types for better maintainability and type checking.Example improvement:
- public static init(tableElement: any) { + public static init(tableElement: HTMLTableElement) { - const bodyClickHandler = (event: any) => { + const bodyClickHandler = (event: MouseEvent | TouchEvent) => { const columnOptionsElement = tableElement.tHead.querySelector('.col-options'); - if (columnOptionsElement && event.composedPath().indexOf(columnOptionsElement) < 0) { + if (columnOptionsElement && !event.composedPath().includes(columnOptionsElement)) { tableElement.dispatchEvent(new CustomEvent('closecolumnoptions', { bubbles: true })); } }; - const keyDownHandler = (event: any) => { + const keyDownHandler = (event: KeyboardEvent) => {
Line range hint
54-99: Well-implemented column resizing with performance considerations.The implementation handles both mouse and touch events, supports RTL, and uses passive event listeners for better performance. However, type safety could be improved.
Consider adding these type improvements:
- private static enableColumnResizing(tableElement: any) { + private static enableColumnResizing(tableElement: HTMLTableElement) { - tableElement.tHead.querySelectorAll('.col-width-draghandle').forEach((handle: any) => { + tableElement.tHead.querySelectorAll<HTMLElement>('.col-width-draghandle').forEach((handle) => { - function handleMouseDown(evt: any) { + function handleMouseDown(evt: MouseEvent | TouchEvent) { - const th = handle.parentElement; + const th = handle.parentElement as HTMLTableCellElement; - function handleMouseMove(evt: any) { + function handleMouseMove(evt: MouseEvent | TouchEvent) {src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs (2)
18-18: Remove Unnecessary Null-Forgiving OperatorThe null-forgiving operator (
!) afterdefaultis unnecessary here. The[Inject]attribute ensures that_jswill be provided. You can safely remove= default!;.Apply this diff to remove the unnecessary null-forgiving operator:
-[Inject] private IJSRuntime _js { get; set; } = default!; +[Inject] private IJSRuntime _js { get; set; }
53-54: Avoid Leading Underscore in Method NameThe method
_LoadMoreItemsis a public method due to the[JSInvokable]attribute, but its name starts with an underscore, which is unconventional for public methods in C#. Consider renaming it toLoadMoreItemsInvokeror similar.Apply this diff to rename the method:
-[JSInvokable("Load")] -public async Task _LoadMoreItems() +[JSInvokable("Load")] +public async Task LoadMoreItemsInvoker()Also, update any references to
_LoadMoreItemswithin the codebase accordingly.src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingItemsProviderRequest.cs (2)
5-9: ValidateskipParameterConsider adding validation to ensure that the
skipparameter is not negative, as negative values may cause unexpected behavior.Apply this diff to add parameter validation:
public BitInfiniteScrollingItemsProviderRequest(int skip, CancellationToken cancellationToken) { + if (skip < 0) + { + throw new ArgumentOutOfRangeException(nameof(skip), "Skip cannot be negative."); + } Skip = skip; CancellationToken = cancellationToken; }
3-14: Add XML Documentation CommentsAdding XML documentation to the class and its members will improve code maintainability and provide better IntelliSense support.
Apply this diff to add XML comments:
+/// <summary> +/// Represents a request for items in the infinite scrolling component. +/// </summary> public sealed class BitInfiniteScrollingItemsProviderRequest { + /// <summary> + /// Initializes a new instance of the <see cref="BitInfiniteScrollingItemsProviderRequest"/> class. + /// </summary> + /// <param name="skip">The number of items to skip.</param> + /// <param name="cancellationToken">The cancellation token for the request.</param> public BitInfiniteScrollingItemsProviderRequest(int skip, CancellationToken cancellationToken) { Skip = skip; CancellationToken = cancellationToken; } + /// <summary> + /// Gets the number of items to skip. + /// </summary> public int Skip { get; } + /// <summary> + /// Gets the cancellation token for the request. + /// </summary> public CancellationToken CancellationToken { get; } }src/BlazorUI/Bit.BlazorUI.Extras/Scripts/general.ts (1)
10-13: Add debouncing to resize event handler for better performance.The
setBitEnvCssVarsfunction is called on every resize event, which could impact performance during window resizing.Consider adding debouncing:
function debounce(fn: Function, ms = 300) { let timeoutId: ReturnType<typeof setTimeout>; return function (this: any, ...args: any[]) { clearTimeout(timeoutId); timeoutId = setTimeout(() => fn.apply(this, args), ms); }; } const debouncedSetBitEnvCssVars = debounce(setBitEnvCssVars);src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor (1)
24-24: Move inline styles to CSS classes for better maintainability.Instead of using inline styles for the last element's dimensions, consider using CSS classes.
-<div @ref="_lastElementRef" style="@(_isLoading ? "height:0;width:0" : "height:1.5rem;width:1.5rem")"></div> +<div @ref="_lastElementRef" class="@(_isLoading ? "bit-isc-last-hidden" : "bit-isc-last-visible")"></div>Add to your SCSS:
.bit-isc-last { &-hidden { height: 0; width: 0; } &-visible { height: 1.5rem; width: 1.5rem; } }src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cs (1)
3-3: Consider making the class public for better extensibility.The
internalvisibility modifier might be too restrictive if other assemblies need to extend or customize the infinite scrolling functionality.-internal static class BitInfiniteScrollingJsRuntimeExtensions +public static class BitInfiniteScrollingJsRuntimeExtensionssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor (1)
15-22: Consider enhancing accessibility for loading stateAdd ARIA attributes to improve accessibility when items are loading:
<BitInfiniteScrolling ItemsProvider="LoadItems" Class="container"> <ItemTemplate Context="item"> - <div>Item @item</div> + <div role="listitem">Item @item</div> </ItemTemplate> <LoadingTemplate> - <div>Loading...</div> + <div role="status" aria-live="polite">Loading...</div> </LoadingTemplate> </BitInfiniteScrolling>src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.ts (1)
19-21: Review intersection threshold valueThe threshold value of 0.69 seems arbitrary. Consider using a more standard value (e.g., 0.5 for half visibility) or make it configurable.
}, { root: rootElement, - threshold: 0.69, + threshold: 0.5, // Half of the element is visible });src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor.cs (2)
42-46: Make delay and page size configurableConsider making the delay and page size configurable for better demo customization:
+private const int PageSize = 20; +private const int SimulatedDelay = 1000; private async ValueTask<IEnumerable<int>> LoadItems(BitInfiniteScrollingItemsProviderRequest request) { - await Task.Delay(1000); - return Enumerable.Range(request.Skip, 20); + await Task.Delay(SimulatedDelay); + return Enumerable.Range(request.Skip, PageSize); }
49-63: Add comments to example codeThe example code would benefit from comments explaining key concepts:
private readonly string example1RazorCode = @" +<!-- ItemsProvider is called when more items need to be loaded --> +<!-- Class=""container"" ensures proper layout --> <BitInfiniteScrolling ItemsProvider=""GetItems"" Class=""container""> <ItemTemplate Context=""item""> <div>Item @item</div> </ItemTemplate> <LoadingTemplate> <div>Loading...</div> </LoadingTemplate> </BitInfiniteScrolling>"; private readonly string example1CsharpCode = @" +// Simulates an async data fetch with artificial delay +// Returns a range of integers starting from request.Skip private async Task<IEnumerable<int>> GetItems(BitInfiniteScrollingItemsProviderRequest request) { await Task.Delay(1000); return Enumerable.Range(request.Skip, 20); }";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.scss(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.ts(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingItemsProvider.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingItemsProviderRequest.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReader.ts(3 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Scripts/general.ts(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Styles/components.scss(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor.scss(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Home/ComponentsSection.razor(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.scss
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor.scss
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
🧰 Additional context used
🪛 Biome (1.9.4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReader.ts
[error] 16-119: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts
[error] 2-100: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.ts
[error] 2-42: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (8)
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs (1)
8-8:⚠️ Potential issueInitialize
_currentItemsCorrectlyThe initialization of
_currentItemsusing[]is not valid in C#. You should initialize it with a new instance ofList<TItem>.Apply this diff to fix the initialization:
-private List<TItem> _currentItems = []; +private List<TItem> _currentItems = new List<TItem>();Likely invalid or redundant comment.
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingItemsProvider.cs (1)
3-3: Delegate Definition Looks GoodThe delegate
BitInfiniteScrollingItemsProvider<T>is defined correctly and follows C# conventions.src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReaderJsRuntimeExtensions.cs (1)
7-7: Verify JavaScript namespace changes across the codebase.The JavaScript namespace has been changed from
BitBlazorUI.BitPdfReadertoBitBlazorUI.PdfReader. Ensure this change is consistent across all related files.Also applies to: 12-12, 17-17, 22-22
✅ Verification successful
Namespace changes are correctly implemented
The JavaScript namespace in BitPdfReader.ts matches the C# invocation, using
BitBlazorUI.PdfReaderconsistently throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old namespace rg "BitBlazorUI\.BitPdfReader" --type ts --type js # Search for the new namespace to verify consistency rg "BitBlazorUI\.PdfReader" --type ts --type jsLength of output: 102
Script:
#!/bin/bash # Find all JavaScript and TypeScript files echo "=== JavaScript/TypeScript files ===" fd -e js -e ts # Search for PDF-related code in these files echo -e "\n=== PDF-related code ===" rg -l "pdf" --type ts --type js -i # Search for init function calls related to PDF echo -e "\n=== PDF init functions ===" rg "init.*pdf" --type ts --type js -i -A 2 -B 2Length of output: 80269
Script:
#!/bin/bash # Check the content of BitPdfReader.ts cat "src/BlazorUI/Bit.BlazorUI.Extras/Components/PdfReader/BitPdfReader.ts"Length of output: 4376
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor (1)
15-15: Verify the styling for container classThe
containerclass is used but its styling context is not clear. Ensure the container class has appropriate styles defined.✅ Verification successful
Container class is properly defined and scoped
The
containerclass is well-defined in the component's SCSS file with a max-height of 300px, appropriately scoped using the::deepselector to create a scrollable area for the infinite scrolling component.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for container class definition in SCSS files rg -t scss "\.container\s*{"Length of output: 124
Script:
#!/bin/bash # Search for container class in various styling files with fixed regex echo "=== Searching in CSS/SCSS files ===" rg -t css -t scss "\.container[\s{]" -A 3 echo -e "\n=== Checking for style imports in the component file ===" rg "(@import|<link.*css|\.css)" src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor echo -e "\n=== Looking for any CSS/SCSS files in the component's directory ===" fd -e css -e scss . src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/Length of output: 851
Script:
#!/bin/bash # Check the content of the component's SCSS file echo "=== Content of BitInfiniteScrollingDemo.razor.scss ===" cat src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/InfiniteScrolling/BitInfiniteScrollingDemo.razor.scss # Also check for any global styles that might define container class echo -e "\n=== Checking for global style files ===" fd -e css -e scss . src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/wwwroot/Length of output: 499
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs (1)
156-156: LGTM!The new navigation item for InfiniteScrolling is correctly placed under the Extras section and follows the existing URL pattern convention.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Home/ComponentsSection.razor (1)
256-258: LGTM!The new BitLink component for InfiniteScrolling follows the existing pattern and maintains consistency in styling and typography.
src/BlazorUI/Bit.BlazorUI.Extras/Styles/components.scss (1)
5-5: LGTM!The import statement for BitInfiniteScrolling styles is correctly placed and follows the existing pattern.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json (1)
74-79: LGTM!The configuration for BitInfiniteScrollingDemo styles follows the existing pattern and maintains consistent settings.
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs
Outdated
Show resolved
Hide resolved
.../Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrollingJsRuntimeExtensions.cs
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.ts
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.ts
Show resolved
Hide resolved
2 participants
closes #9643
Summary by CodeRabbit
Release Notes
New Features
Refactoring
BitDataGridandBitPdfReaderclasses toDataGridandPdfReaderDocumentation
Styling