Add new BitDataGrid component (#12502)#12504
Open
msynk wants to merge 9 commits into
Open
Conversation
msynk
requested review from
Cyrus-Sushiant,
mhrastegari,
yasmoradi and
zahra-yousefi
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds a new ChangesDataGrid feature set
BitQuickGrid extraction
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (18)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cs-12-13 (1)
12-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDeterministic claim is broken by
DateTime.TodayLine 29 makes generated data drift day-to-day, so the “reproducible” promise in Line 12 is not true across dates.
Suggested fix
public static List<Product> Generate(int count, int seed = 42) { var rng = new Random(seed); + var baseDate = new DateTime(2024, 1, 1); var categories = Enum.GetValues<Category>(); var list = new List<Product>(count); @@ - ReleaseDate = DateTime.Today.AddDays(-rng.Next(0, 2000)), + ReleaseDate = baseDate.AddDays(-rng.Next(0, 2000)),Also applies to: 29-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cs` around lines 12 - 13, The Generate method in the SampleData class claims to be deterministic and reproducible in its summary, but it uses DateTime.Today on line 29 which changes daily and breaks the deterministic guarantee. Replace the DateTime.Today usage with a fixed, seed-based date value that remains consistent regardless of when the method is called. This ensures the data generation is truly reproducible and matches the claim in the method's XML summary comment.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs-223-225 (1)
223-225:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExample 16 sample is missing the
OnCellDoubleClickhandlerLine 224 wires
OnCellDoubleClick, but the paired C# snippet (Lines 230-231) does not define it.Suggested fix
void OnCellClick(BitDataGridCellEventArgs<Product> e) { /* e.Item, e.ColumnTitle, e.Value */ } +void OnCellDoubleClick(BitDataGridCellEventArgs<Product> e) { /* ... */ } void OnCellContextMenu(BitDataGridCellEventArgs<Product> e) { /* e.Mouse.ClientX / e.Mouse.ClientY */ }Also applies to: 230-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs` around lines 223 - 225, The Razor component in Example 16 declares an OnCellDoubleClick event handler in the markup, but the corresponding C# method is missing from the code-behind. Add the OnCellDoubleClick handler method in the C# section of BitDataGridDemo.razor.samples.cs following the same pattern and signature as the existing OnCellClick and OnCellContextMenu handler methods. Ensure the method is defined in the appropriate location within the Example 16 sample code block so it matches the event binding in the markup.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs-148-163 (1)
148-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHonor all sort descriptors in the read handlers.
Both handlers only apply
request.Sorts.FirstOrDefault(), so additional sort descriptors are ignored and multi-sort requests produce incorrect ordering.Suggested fix
- var sort = request.Sorts.FirstOrDefault(); - if (sort is not null) + IOrderedEnumerable<Product>? ordered = null; + foreach (var sort in request.Sorts) { Func<Product, object> key = sort.ColumnId switch { nameof(Product.Name) => p => p.Name, nameof(Product.Category) => p => p.Category, nameof(Product.Supplier) => p => p.Supplier, nameof(Product.Price) => p => p.Price, nameof(Product.Stock) => p => p.Stock, + nameof(Product.Rating) => p => p.Rating, _ => p => p.Id }; - query = sort.Direction == BitDataGridSortDirection.Descending - ? query.OrderByDescending(key) - : query.OrderBy(key); + ordered = ordered is null + ? (sort.Direction == BitDataGridSortDirection.Descending + ? query.OrderByDescending(key) + : query.OrderBy(key)) + : (sort.Direction == BitDataGridSortDirection.Descending + ? ordered.ThenByDescending(key) + : ordered.ThenBy(key)); } + if (ordered is not null) query = ordered;Also applies to: 182-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs` around lines 148 - 163, The current implementation only processes the first sort descriptor using request.Sorts.FirstOrDefault(), which causes multi-sort requests to be ignored. Replace the if statement that checks for a single sort with a loop that iterates through all items in request.Sorts collection. For each sort descriptor, apply the appropriate sort operation (OrderBy or OrderByDescending based on sort.Direction) to the query in sequence. The switch statement logic for mapping sort.ColumnId to the appropriate property key should remain the same, but needs to be executed for every sort descriptor in the collection rather than just the first one.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs-203-205 (1)
203-205:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle empty final batches in infinite-scroll status text.
When no rows are returned, the current log renders an inverted range (
start > end), which is misleading.Suggested fix
- var end = request.Skip + batch.Count; - infiniteLog = $"Batch #{infiniteRequests} → loaded rows {request.Skip + 1}–{end} ({batch.Count} rows)"; + if (batch.Count == 0) + { + infiniteLog = $"Batch #{infiniteRequests} → no more rows to load."; + } + else + { + var end = request.Skip + batch.Count; + infiniteLog = $"Batch #{infiniteRequests} → loaded rows {request.Skip + 1}–{end} ({batch.Count} rows)"; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs` around lines 203 - 205, The infiniteLog message construction does not handle the case when batch.Count is zero (empty final batch), resulting in a misleading inverted range display (e.g., "loaded rows 101–100"). Add a conditional check before assigning to infiniteLog to detect when batch.Count equals zero and either skip the log update entirely or provide an alternative status message that clearly indicates no additional rows were loaded, while preserving the current log message logic for non-empty batches.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cs-126-130 (1)
126-130:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear stale accessor when
Fieldis removed.Line 128 sets
Accessoronly whenHasFieldis true; ifFieldchanges from populated to null/empty, the previous accessor remains active.Proposed fix
protected override void OnParametersSet() { if (HasField) Accessor = BitDataGridPropertyAccessor<TItem>.For(Field!); + else + Accessor = null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cs` around lines 126 - 130, In the OnParametersSet method of the BitDataGridColumn class, the Accessor is only set when HasField is true, but when Field is cleared or removed (HasField becomes false), the previous accessor remains active and is never cleared. Add an else clause after the if (HasField) condition that sets Accessor to null when HasField is false, ensuring stale accessors are properly cleaned up when the Field parameter changes from a value to null or empty.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scss-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix Stylelint
value-keyword-caseviolation in font stack.Line 8 currently triggers lint errors; quote named font families so lint passes without changing fallback behavior.
Proposed fix
- --bit-dtg-font: 14px/1.4 system-ui, -apple-system, Segoe UI, Roboto, sans-serif; + --bit-dtg-font: 14px/1.4 system-ui, -apple-system, "Segoe UI", "Roboto", sans-serif;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scss` at line 8, The font stack in the --bit-dtg-font custom property on line 8 of BitDataGrid.scss triggers a stylelint value-keyword-case violation because font family names are not properly quoted. Quote all font family names in the font stack (system-ui, -apple-system, Segoe UI, Roboto, and sans-serif) individually in the declaration to ensure stylelint compliance while maintaining the same fallback behavior.Source: Linters/SAST tools
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cs-20-20 (1)
20-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
CancellationTokenis never populated by the grid.The
CancellationTokenproperty exists but is never set when constructingBitDataGridReadRequestinstances inLoadNextBatchAsyncorLoadServerDataAsync. Consumers will always receiveCancellationToken.None, making this property ineffective for request cancellation.Consider wiring a
CancellationTokenSourcethat cancels on component disposal or when a new request supersedes an in-flight one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cs` at line 20, The CancellationToken property in BitDataGridReadRequest is declared but never initialized when instances are created in LoadNextBatchAsync and LoadServerDataAsync methods. To fix this, create a CancellationTokenSource field in the grid component and pass its Token property to the CancellationToken property when constructing each BitDataGridReadRequest instance. Additionally, dispose the previous CancellationTokenSource and create a new one for each request to handle superseding in-flight requests, and ensure the CancellationTokenSource is disposed when the component is disposed to properly clean up resources.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs-557-566 (1)
557-566:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDead code:
|| truemakes!MultiSortineffective.Line 557's condition
(!MultiSort || true)always evaluates totrue, making the!MultiSortcheck dead code. Additionally, line 560 redundantly re-checks!additive.This appears to be leftover from refactoring. Simplify to the intended logic:
🧹 Proposed fix
- if (!additive && (!MultiSort || true)) - { - // Single sort: clear others unless additive - if (!additive) - { - var keep = existing; - _sorts.Clear(); - if (keep is not null) _sorts.Add(keep); - } - } + if (!additive) + { + // Single sort: clear others + var keep = existing; + _sorts.Clear(); + if (keep is not null) _sorts.Add(keep); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 557 - 566, The condition in the BitDataGrid.razor.cs sort handling logic contains `|| true` on line 557, which makes the `!MultiSort` check ineffective and represents dead code. Additionally, the inner if statement on line 560 redundantly re-checks the `!additive` condition that was already evaluated in the outer if. To fix this, remove the `|| true` portion from the outer condition so it properly checks both `!additive && !MultiSort`, and remove the redundant inner `if (!additive)` block, moving its contents (the sort clearing logic) to directly execute within the outer if block since the condition already guarantees additive is false.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs-1087-1101 (1)
1087-1101:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCSV export may throw if
GetFormattedValuereturns null.
GetFormattedValue(item)at line 1094 could potentially returnnull, and passingnulltoEscape()would cause aNullReferenceExceptionon the.Contains()call.🐛 Proposed fix
public string ToCsv() { var cols = VisibleColumns.Where(c => c.HasField).ToList(); var sb = new System.Text.StringBuilder(); sb.AppendLine(string.Join(",", cols.Select(c => Escape(c.DisplayTitle)))); var rows = IsServerMode ? _pageItems : _view; foreach (var item in rows) - sb.AppendLine(string.Join(",", cols.Select(c => Escape(c.GetFormattedValue(item))))); + sb.AppendLine(string.Join(",", cols.Select(c => Escape(c.GetFormattedValue(item) ?? "")))); return sb.ToString(); static string Escape(string v) - => v.Contains(',') || v.Contains('"') || v.Contains('\n') + => v is null ? "" : v.Contains(',') || v.Contains('"') || v.Contains('\n') ? "\"" + v.Replace("\"", "\"\"") + "\"" : v; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 1087 - 1101, The ToCsv() method passes the return value of GetFormattedValue(item) directly to the Escape() helper function, but GetFormattedValue can return null, causing a NullReferenceException when Escape tries to call .Contains() on the null value. Fix this by adding null handling to the Escape() method—either at the start of the method by converting null to an empty string using the null coalescing operator, or by using the null coalescing operator when calling GetFormattedValue(item) in the Select expression to ensure Escape always receives a non-null string value.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs-74-84 (1)
74-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject empty/whitespace paths up front
For("")/For(" ")currently builds an accessor for the entire item object, which silently masks misconfigured columns and leads to incorrect sort/filter behavior.Suggested patch
public static BitDataGridPropertyAccessor<TItem> For(string path) - => Cache.GetOrAdd(path, Build); +{ + if (string.IsNullOrWhiteSpace(path)) + throw new ArgumentException("Property path cannot be null, empty, or whitespace.", nameof(path)); + + return Cache.GetOrAdd(path, Build); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs` around lines 74 - 84, The For method does not validate the input path parameter before caching and building the accessor. Add validation at the start of the For method to check if the path is null, empty, or contains only whitespace characters. If the path is invalid, throw an appropriate exception to reject the invalid input and prevent the silent masking of misconfigured columns that currently occurs when an empty or whitespace-only path is passed to For.src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor-107-107 (1)
107-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix virtualized placeholder
aria-rowindexoffset.Line 107 should account for the header row. Non-virtual rows start at index 2 (Line 54), but placeholders currently start at 1, which misreports row positions to assistive tech.
Suggested fix
- <tr aria-rowindex="@(placeholderContext.Index + 1)"> + <tr aria-rowindex="@(placeholderContext.Index + 2)">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor` at line 107, The aria-rowindex attribute in the virtualized placeholder row template is not accounting for the header row offset. The placeholder row on line 107 uses placeholderContext.Index + 1, but it should use placeholderContext.Index + 2 to match the offset used in non-virtual rows (which start at index 2 to account for the header). Update the aria-rowindex expression to add 2 instead of 1 to ensure correct row position reporting to assistive technologies.src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss-4-5 (1)
4-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove
.scssextensions in new imports to satisfy Stylelint.Lines 4-5 currently violate
scss/load-partial-extension.Suggested fix
-@import "../Components/QuickGrid/BitQuickGrid.scss"; -@import "../Components/QuickGrid/Pagination/BitQuickGridPaginator.scss"; +@import "../Components/QuickGrid/BitQuickGrid"; +@import "../Components/QuickGrid/Pagination/BitQuickGridPaginator";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss` around lines 4 - 5, Remove the `.scss` file extensions from the `@import` statements on lines 4-5. Change the imports for BitQuickGrid.scss and BitQuickGridPaginator.scss to remove the `.scss` extension so they reference just the partial names without the extension, which satisfies the scss/load-partial-extension Stylelint rule that disallows explicit file extensions in SCSS imports.Source: Linters/SAST tools
src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs-129-134 (1)
129-134:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTypo in XML documentation.
Line 131 has a typo: "A callback that supplies data for the rid" should be "A callback that supplies data for the grid".
📝 Proposed fix
/// <summary> - /// A callback that supplies data for the rid. + /// A callback that supplies data for the grid. /// /// You should supply either <see cref="Items"/> or <see cref="ItemsProvider"/>, but not both. /// </summary>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs` around lines 129 - 134, Fix the typo in the XML documentation comment for the ItemsProvider parameter where it currently states "A callback that supplies data for the rid" should be corrected to "A callback that supplies data for the grid". Locate the summary element in the XML comment block preceding the ItemsProvider property declaration and change the word "rid" to "grid" to accurately describe what the callback supplies data for.src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs-346-353 (1)
346-353:⚠️ Potential issue | 🟡 MinorAwait
SetTotalItemCountAsynccalls to ensure exceptions are not swallowed.
Pagination?.SetTotalItemCountAsync(result.TotalItemCount)is not awaited. The method can perform asynchronous work throughTotalItemCountChangedSubscribable.InvokeCallbacksAsync()orSetCurrentPageIndexAsync(), either of which may trigger component updates and can throw exceptions. Without awaiting, any exceptions from these async operations will be silently swallowed. The same pattern appears at line 392.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs` around lines 346 - 353, The SetTotalItemCountAsync method call on the Pagination object is not being awaited, which means any exceptions thrown by async operations within it (such as TotalItemCountChangedSubscribable.InvokeCallbacksAsync or SetCurrentPageIndexAsync) will be silently swallowed. Add the await keyword before the Pagination?.SetTotalItemCountAsync(result.TotalItemCount) call to properly handle exceptions. Apply the same fix to both occurrences of this pattern in the BitQuickGrid.razor.cs file.src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scss-15-16 (1)
15-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix transition declaration so the delay is applied.
At Line 15,
transition-delayis overridden by the shorthand on Line 16, so the delay never takes effect.💡 Suggested fix
- transition-delay: 25ms; - transition: opacity linear 100ms; + transition: opacity linear 100ms 25ms;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scss` around lines 15 - 16, The shorthand transition property on line 16 overrides the transition-delay specified on line 15, causing the delay to be lost. Combine both properties into a single shorthand transition declaration that includes the delay value as the fourth parameter: change the two separate lines to a single transition property that specifies opacity, linear, the 100ms duration, and the 25ms delay in the correct order.Source: Linters/SAST tools
src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cs-31-35 (1)
31-35:⚠️ Potential issue | 🟡 MinorUpdate XML documentation generic type references.
The class
BitQuickGridSort<TGridItem>has mismatched generic parameters in four methods' XML documentation. All instances use<see cref="BitQuickGridSort{T}"/>instead of<see cref="BitQuickGridSort{TGridItem}"/>, which will produce unresolved-cref warnings.Apply the following changes to all four methods:
Required fixes
- ByAscending method (lines 31, 35): Replace
BitQuickGridSort{T}withBitQuickGridSort{TGridItem}in both the summary and returns documentation.- ByDescending method (lines 41, 45): Replace
BitQuickGridSort{T}withBitQuickGridSort{TGridItem}in both the summary and returns documentation.- ThenAscending method (lines 51, 55): Replace
BitQuickGridSort{T}withBitQuickGridSort{TGridItem}in both the summary and returns documentation.- ThenDescending method (lines 68, 72): Replace
BitQuickGridSort{T}withBitQuickGridSort{TGridItem}in both the summary and returns documentation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cs` around lines 31 - 35, In the BitQuickGridSort class, update the XML documentation for the four sorting methods (ByAscending, ByDescending, ThenAscending, and ThenDescending) to use the correct generic type parameter. Replace all instances of `BitQuickGridSort{T}` with `BitQuickGridSort{TGridItem}` in the see cref tags within both the summary and returns elements of each method's documentation to match the actual class generic parameter name and resolve cref warnings.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorAdd
Relattribute to protect thetarget="_blank"link.BitLink does not automatically apply
rel="noopener noreferrer"for external links. AddRel="BitLinkRels.NoOpener | BitLinkRels.NoReferrer"to prevent opener access:<BitLink Href="https://www.nuget.org/packages/Bit.BlazorUI.Extras" Target="_blank" Rel="BitLinkRels.NoOpener | BitLinkRels.NoReferrer">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor` at line 20, The BitLink component with Target="_blank" is missing the Rel attribute which is needed for security protection against opener access. Add the Rel attribute to the BitLink component (around line 20) with the value "BitLinkRels.NoOpener | BitLinkRels.NoReferrer" to properly secure the external link. This will prevent the opened page from accessing the window.opener object.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor-101-101 (1)
101-101:⚠️ Potential issue | 🟡 MinorUse distinct
@refvariables for the two product grids.At lines 101 and 116, both
BitQuickGridinstances share the same component reference@ref="productsDataGrid", causing the second grid to overwrite the first. TheODataSampleNameFilterproperty setter (in code-behind) refreshes only the last rendered grid instance, breaking the intended refresh behavior for the virtualized grid.Suggested fix
-<BitQuickGrid `@ref`="productsDataGrid" ItemsProvider="`@productsItemsProvider`" ItemKey="(p => p.Id)" TGridItem="ProductDto" Virtualize> +<BitQuickGrid `@ref`="odataProductsDataGrid" ItemsProvider="`@productsItemsProvider`" ItemKey="(p => p.Id)" TGridItem="ProductDto" Virtualize> -<BitQuickGrid `@ref`="productsDataGrid" ItemsProvider="`@productsItemsProvider`" ItemKey="(p => p.Id)" TGridItem="ProductDto" Pagination="pagination3"> +<BitQuickGrid `@ref`="loadingTemplateProductsDataGrid" ItemsProvider="`@productsItemsProvider`" ItemKey="(p => p.Id)" TGridItem="ProductDto" Pagination="pagination3">Also declare these fields in
BitQuickGridDemo.razor.csand update theODataSampleNameFiltersetter to refresh the correct grid reference.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor` at line 101, The two BitQuickGrid instances in the template are sharing the same `@ref` variable "productsDataGrid", causing the second grid to overwrite the first reference. Create distinct `@ref` variables for each BitQuickGrid component (one for the virtualized grid and one for the other grid), then declare corresponding fields in the BitQuickGridDemo.razor.cs code-behind file for each of these new references. Finally, update the ODataSampleNameFilter property setter to refresh the correct grid instance based on which grid should be updated when the filter changes.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs (1)
447-462: 💤 Low valueConsider avoiding repeated
List<string>allocations.
GetRowClassandGetRowStyle(lines 447-479) allocate a newList<string>on every row render. For grids with many rows, this could add GC pressure. Consider using aStringBuilderor direct string concatenation since you're only combining at most two values.♻️ Possible simplification
private string? GetRowClass(TGridItem item) { - var classes = new List<string>(); - - if (RowClass is not null) - { - classes.Add(RowClass); - } - - if (RowClassSelector is not null) - { - classes.Add(RowClassSelector(item)); - } - - return classes.Any() ? string.Join(' ', classes) : null; + var baseClass = RowClass; + var selectorClass = RowClassSelector?.Invoke(item); + + return (baseClass, selectorClass) switch + { + (not null, not null) => $"{baseClass} {selectorClass}", + (not null, null) => baseClass, + (null, not null) => selectorClass, + _ => null + }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs` around lines 447 - 462, The GetRowClass and GetRowStyle methods allocate a new List<string> on every row render, which creates unnecessary garbage collection pressure when rendering large grids. Replace the List<string> allocation and join pattern with direct string concatenation in both methods. Since you're combining at most two values in each method (RowClass and RowClassSelector for GetRowClass, and similarly for GetRowStyle), you can build the result string directly by checking the conditions and concatenating the values with a space separator only when both values exist, avoiding the List allocation entirely.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 448-475: In the LoadNextBatchAsync method, the OnLoadMore callback
invocation can throw an exception that causes the _infiniteLoading flag to
remain true, permanently blocking further loads. Wrap the entire logic starting
from the OnLoadMore call through the state updates in a try-finally block,
ensuring that _infiniteLoading is set to false and StateHasChanged is called in
the finally clause. This guarantees that _infiniteLoading will be reset even if
OnLoadMore throws an exception, allowing users to recover and retry loading more
data.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cs`:
- Around line 144-153: The Sum and Average aggregation calculations in the
BitDataGridDataProcessor class use double type for accumulating values, which
loses precision for decimal/currency columns. Replace the double sum variable
and the TryToDouble method calls with decimal-based logic to preserve numeric
precision. Ensure the accumulation and calculation logic in the Sum case (around
lines 147-152) uses decimal arithmetic throughout, and apply the same fix to the
related aggregation logic mentioned in lines 174-180 to maintain consistency
across all numeric aggregate operations.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs`:
- Around line 95-97: The compiled getter in BitDataGridPropertyAccessor for
nested property paths like Address.City is not null-safe. When an intermediate
property (like Address) is null, the expression throws an exception instead of
returning null. Modify the expression tree construction before creating the
Expression.Lambda to include null checks at each level of the property chain.
This should use Expression.Condition to test if each intermediate property
access is null and return null early if any level is null, allowing sorting and
filtering to handle partial object graphs gracefully without runtime failures.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cs`:
- Around line 7-10: The BitDataGridReadResult constructor accepts `items` and
`totalCount` parameters without validation, which allows null items and negative
totalCount values to pass through and corrupt the grid state. Add argument guard
clauses at the start of the BitDataGridReadResult constructor to validate that
items is not null and totalCount is greater than or equal to zero, throwing
appropriate ArgumentException or ArgumentNullException if validation fails,
ensuring invalid OnRead outputs fail immediately rather than causing downstream
paging issues.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.ts`:
- Line 47: The call to colOptions.scrollIntoViewIfNeeded() is using a
non-standard browser API that is not supported in Firefox and will cause runtime
errors. Replace this single method call with a check that first tests if the
method exists before calling it, and falls back to the standard scrollIntoView()
API if scrollIntoViewIfNeeded is not available. This ensures the column-options
interaction works across all browsers including Firefox.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor`:
- Around line 108-110: The icon-only buttons with class "bit-qkg-cob" that
trigger Grid.ShowColumnOptions(this) at lines 110 and 129 lack accessible names,
making them unannounced to assistive technologies. Add an aria-label attribute
to both button elements that provides a descriptive name for the column options
button (such as "Column options" or similar descriptive text). This ensures
screen readers will properly announce the button's purpose to users with
disabilities.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cs`:
- Around line 657-660: In the code where the OData filter string is constructed
using the query.Add method, the user input from _odataSampleNameFilter is being
directly interpolated without escaping, which creates an OData injection
vulnerability. To fix this, before interpolating _odataSampleNameFilter into the
filter string literal, escape any single quotes in the user input by replacing
each single quote character with two consecutive single quotes, as required by
the OData specification. This ensures that user input containing single quotes
like test' or '1'='1 will be treated as literal string data rather than being
able to break out of or modify the filter expression.
- Line 669: In the productsItemsProvider method at the GetFromJsonAsync call on
line 669, add the cancellation token parameter req.CancellationToken as the
third argument to match the pattern already established in the
foodRecallItemsProvider method. This will ensure the HTTP request can properly
respond to cancellation requests from the grid.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scss`:
- Line 12: The stylelint rule `selector-pseudo-element-no-unknown` flags
`::deep` pseudo-elements at lines 12, 62, 89, and 122 in the
BitQuickGridDemo.razor.scss file because `::deep` is a Blazor-specific
pseudo-element not part of standard CSS. Fix this by adding
`"selector-pseudo-element-no-unknown": null` to the rules section in
`.stylelintrc.json` to globally disable this rule (recommended approach), or
alternatively wrap each `::deep` block with stylelint disable and re-enable
comments (/* stylelint-disable-next-line selector-pseudo-element-no-unknown */
before and /* stylelint-enable */ after) if a file-local approach is preferred.
---
Minor comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 557-566: The condition in the BitDataGrid.razor.cs sort handling
logic contains `|| true` on line 557, which makes the `!MultiSort` check
ineffective and represents dead code. Additionally, the inner if statement on
line 560 redundantly re-checks the `!additive` condition that was already
evaluated in the outer if. To fix this, remove the `|| true` portion from the
outer condition so it properly checks both `!additive && !MultiSort`, and remove
the redundant inner `if (!additive)` block, moving its contents (the sort
clearing logic) to directly execute within the outer if block since the
condition already guarantees additive is false.
- Around line 1087-1101: The ToCsv() method passes the return value of
GetFormattedValue(item) directly to the Escape() helper function, but
GetFormattedValue can return null, causing a NullReferenceException when Escape
tries to call .Contains() on the null value. Fix this by adding null handling to
the Escape() method—either at the start of the method by converting null to an
empty string using the null coalescing operator, or by using the null coalescing
operator when calling GetFormattedValue(item) in the Select expression to ensure
Escape always receives a non-null string value.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scss`:
- Line 8: The font stack in the --bit-dtg-font custom property on line 8 of
BitDataGrid.scss triggers a stylelint value-keyword-case violation because font
family names are not properly quoted. Quote all font family names in the font
stack (system-ui, -apple-system, Segoe UI, Roboto, and sans-serif) individually
in the declaration to ensure stylelint compliance while maintaining the same
fallback behavior.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cs`:
- Around line 126-130: In the OnParametersSet method of the BitDataGridColumn
class, the Accessor is only set when HasField is true, but when Field is cleared
or removed (HasField becomes false), the previous accessor remains active and is
never cleared. Add an else clause after the if (HasField) condition that sets
Accessor to null when HasField is false, ensuring stale accessors are properly
cleaned up when the Field parameter changes from a value to null or empty.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs`:
- Around line 74-84: The For method does not validate the input path parameter
before caching and building the accessor. Add validation at the start of the For
method to check if the path is null, empty, or contains only whitespace
characters. If the path is invalid, throw an appropriate exception to reject the
invalid input and prevent the silent masking of misconfigured columns that
currently occurs when an empty or whitespace-only path is passed to For.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cs`:
- Line 20: The CancellationToken property in BitDataGridReadRequest is declared
but never initialized when instances are created in LoadNextBatchAsync and
LoadServerDataAsync methods. To fix this, create a CancellationTokenSource field
in the grid component and pass its Token property to the CancellationToken
property when constructing each BitDataGridReadRequest instance. Additionally,
dispose the previous CancellationTokenSource and create a new one for each
request to handle superseding in-flight requests, and ensure the
CancellationTokenSource is disposed when the component is disposed to properly
clean up resources.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor`:
- Line 107: The aria-rowindex attribute in the virtualized placeholder row
template is not accounting for the header row offset. The placeholder row on
line 107 uses placeholderContext.Index + 1, but it should use
placeholderContext.Index + 2 to match the offset used in non-virtual rows (which
start at index 2 to account for the header). Update the aria-rowindex expression
to add 2 instead of 1 to ensure correct row position reporting to assistive
technologies.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 129-134: Fix the typo in the XML documentation comment for the
ItemsProvider parameter where it currently states "A callback that supplies data
for the rid" should be corrected to "A callback that supplies data for the
grid". Locate the summary element in the XML comment block preceding the
ItemsProvider property declaration and change the word "rid" to "grid" to
accurately describe what the callback supplies data for.
- Around line 346-353: The SetTotalItemCountAsync method call on the Pagination
object is not being awaited, which means any exceptions thrown by async
operations within it (such as
TotalItemCountChangedSubscribable.InvokeCallbacksAsync or
SetCurrentPageIndexAsync) will be silently swallowed. Add the await keyword
before the Pagination?.SetTotalItemCountAsync(result.TotalItemCount) call to
properly handle exceptions. Apply the same fix to both occurrences of this
pattern in the BitQuickGrid.razor.cs file.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scss`:
- Around line 15-16: The shorthand transition property on line 16 overrides the
transition-delay specified on line 15, causing the delay to be lost. Combine
both properties into a single shorthand transition declaration that includes the
delay value as the fourth parameter: change the two separate lines to a single
transition property that specifies opacity, linear, the 100ms duration, and the
25ms delay in the correct order.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cs`:
- Around line 31-35: In the BitQuickGridSort class, update the XML documentation
for the four sorting methods (ByAscending, ByDescending, ThenAscending, and
ThenDescending) to use the correct generic type parameter. Replace all instances
of `BitQuickGridSort{T}` with `BitQuickGridSort{TGridItem}` in the see cref tags
within both the summary and returns elements of each method's documentation to
match the actual class generic parameter name and resolve cref warnings.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss`:
- Around line 4-5: Remove the `.scss` file extensions from the `@import`
statements on lines 4-5. Change the imports for BitQuickGrid.scss and
BitQuickGridPaginator.scss to remove the `.scss` extension so they reference
just the partial names without the extension, which satisfies the
scss/load-partial-extension Stylelint rule that disallows explicit file
extensions in SCSS imports.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs`:
- Around line 148-163: The current implementation only processes the first sort
descriptor using request.Sorts.FirstOrDefault(), which causes multi-sort
requests to be ignored. Replace the if statement that checks for a single sort
with a loop that iterates through all items in request.Sorts collection. For
each sort descriptor, apply the appropriate sort operation (OrderBy or
OrderByDescending based on sort.Direction) to the query in sequence. The switch
statement logic for mapping sort.ColumnId to the appropriate property key should
remain the same, but needs to be executed for every sort descriptor in the
collection rather than just the first one.
- Around line 203-205: The infiniteLog message construction does not handle the
case when batch.Count is zero (empty final batch), resulting in a misleading
inverted range display (e.g., "loaded rows 101–100"). Add a conditional check
before assigning to infiniteLog to detect when batch.Count equals zero and
either skip the log update entirely or provide an alternative status message
that clearly indicates no additional rows were loaded, while preserving the
current log message logic for non-empty batches.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`:
- Around line 223-225: The Razor component in Example 16 declares an
OnCellDoubleClick event handler in the markup, but the corresponding C# method
is missing from the code-behind. Add the OnCellDoubleClick handler method in the
C# section of BitDataGridDemo.razor.samples.cs following the same pattern and
signature as the existing OnCellClick and OnCellContextMenu handler methods.
Ensure the method is defined in the appropriate location within the Example 16
sample code block so it matches the event binding in the markup.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cs`:
- Around line 12-13: The Generate method in the SampleData class claims to be
deterministic and reproducible in its summary, but it uses DateTime.Today on
line 29 which changes daily and breaks the deterministic guarantee. Replace the
DateTime.Today usage with a fixed, seed-based date value that remains consistent
regardless of when the method is called. This ensures the data generation is
truly reproducible and matches the claim in the method's XML summary comment.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor`:
- Line 20: The BitLink component with Target="_blank" is missing the Rel
attribute which is needed for security protection against opener access. Add the
Rel attribute to the BitLink component (around line 20) with the value
"BitLinkRels.NoOpener | BitLinkRels.NoReferrer" to properly secure the external
link. This will prevent the opened page from accessing the window.opener object.
- Line 101: The two BitQuickGrid instances in the template are sharing the same
`@ref` variable "productsDataGrid", causing the second grid to overwrite the first
reference. Create distinct `@ref` variables for each BitQuickGrid component (one
for the virtualized grid and one for the other grid), then declare corresponding
fields in the BitQuickGridDemo.razor.cs code-behind file for each of these new
references. Finally, update the ODataSampleNameFilter property setter to refresh
the correct grid instance based on which grid should be updated when the filter
changes.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 447-462: The GetRowClass and GetRowStyle methods allocate a new
List<string> on every row render, which creates unnecessary garbage collection
pressure when rendering large grids. Replace the List<string> allocation and
join pattern with direct string concatenation in both methods. Since you're
combining at most two values in each method (RowClass and RowClassSelector for
GetRowClass, and similarly for GetRowStyle), you can build the result string
directly by checking the conditions and concatenating the values with a space
separator only when both values exist, avoiding the List allocation entirely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7038154f-12e2-424f-a8d9-7750b95219c1
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (5)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scss (1)
218-227: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winRespect reduced-motion user preference for spinner/shimmer animations.
The grid always animates loading indicators. Add a
prefers-reduced-motionfallback so motion-sensitive users are not forced into continuous animation.Suggested fix
`@keyframes` bit-dtg-spin { to { transform: rotate(360deg); } } @@ `@keyframes` bit-dtg-shimmer { 0% { background-position: 100% 0; } 100% { background-position: 0 0; } } + +@media (prefers-reduced-motion: reduce) { + .bit-dtg-spinner, + .bit-dtg-skeleton { + animation: none; + } +}Also applies to: 231-243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scss` around lines 218 - 227, The BitDataGrid spinner and shimmer animations do not respect user accessibility preferences for reduced motion. Wrap the animation properties in the `.bit-dtg-spinner` rule and the related animation rules (lines 231-243) in a `@media (prefers-reduced-motion: no-preference)` query so animations only play for users who have not requested reduced motion. For users with `prefers-reduced-motion: reduce`, provide fallback styles that disable the animation property, ensuring the spinner/shimmer elements remain visible but static.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor (2)
265-270: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd
@keyto virtualized row for identity consistency.The infinite (line 246) and paged (line 275) render paths use
@key="item", but the virtualized path does not. Adding@keyensures consistent item identity tracking across all rendering modes, which helps Blazor optimize DOM updates when items are reordered or modified.♻️ Suggested fix
else if (UseVirtualization) { <Virtualize Items="VirtualRows" ItemSize="RowHeight" Context="item" TItem="TItem"> - <BitDataGridRow TItem="TItem" Grid="this" Item="item" /> + <BitDataGridRow TItem="TItem" Grid="this" Item="item" `@key`="item" /> </Virtualize> }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor` around lines 265 - 270, The virtualized rendering path in the UseVirtualization condition is missing the `@key` directive on the BitDataGridRow component, while the infinite and paged rendering paths both include `@key`="item". Add `@key`="item" to the BitDataGridRow component inside the Virtualize block to ensure consistent item identity tracking across all rendering modes and allow Blazor to properly optimize DOM updates when items are reordered or modified.
336-338: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winCache
VirtualRowsto avoid repeated list allocation.
VirtualRowscallsToList()on every access when_viewis not anICollection<TItem>. SinceVirtualizemay accessItemsmultiple times during rendering, this creates unnecessary allocations. Consider caching the list when the view changes instead of computing it on each access.♻️ Suggested approach
Add a cached field that is populated when
_viewchanges (e.g., inProcessClientDataorRefreshAsync):// In code-behind private ICollection<TItem>? _cachedVirtualRows; // Update when _view changes _cachedVirtualRows = _view as ICollection<TItem> ?? _view.ToList();Then in the Razor:
-private ICollection<TItem> VirtualRows => _view as ICollection<TItem> ?? _view.ToList(); +private ICollection<TItem> VirtualRows => _cachedVirtualRows ?? [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor` around lines 336 - 338, The VirtualRows property currently calls ToList() on every access when _view is not an ICollection<TItem>, causing repeated allocations during rendering cycles. Add a private cached field to store the computed virtual rows collection, then populate this cache whenever _view changes (such as in ProcessClientData or RefreshAsync methods). Update the VirtualRows property to return the cached field instead of computing the collection on each access, ensuring the cache remains synchronized whenever the underlying _view is modified.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs (1)
220-222: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider caching
VisibleColumnsto avoid repeated allocations.
VisibleColumnscreates a new list on every property access via.ToList(). This property is accessed frequently during rendering (inBitDataGridRow, keyboard navigation, column spanning, etc.). Consider caching the result and invalidating when columns change.♻️ Suggested caching pattern
+ private IReadOnlyList<BitDataGridColumn<TItem>>? _visibleColumnsCache; + private int _columnsVersion; - internal IReadOnlyList<BitDataGridColumn<TItem>> VisibleColumns => _columns.Where(c => c.Visible).ToList(); + internal IReadOnlyList<BitDataGridColumn<TItem>> VisibleColumns => _visibleColumnsCache ??= _columns.Where(c => c.Visible).ToList(); internal void AddColumn(BitDataGridColumn<TItem> column) { if (_columns.Contains(column)) return; _columns.Add(column); _columnsById[column.Id] = column; + _visibleColumnsCache = null; InvokeAsync(StateHasChanged); } internal void RemoveColumn(BitDataGridColumn<TItem> column) { if (_columns.Remove(column)) { _columnsById.Remove(column.Id); + _visibleColumnsCache = null; InvokeAsync(StateHasChanged); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 220 - 222, The VisibleColumns property in the BitDataGrid class creates a new list on every access via .ToList(), causing unnecessary allocations since this property is accessed frequently during rendering. Implement caching by creating a private backing field to store the visible columns list, update this cached list whenever the underlying _columns collection changes (add invalidation logic in methods that modify _columns such as column add/remove operations), and modify the VisibleColumns property to return the cached list instead of recomputing it with .Where().ToList() on each access. Ensure the cache is refreshed whenever columns are added, removed, or their visibility changes.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cs (1)
1-26: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueEvent args exposes mutable
Columnreference; document stability assumption.The
Columnproperty holds a mutable reference toBitDataGridColumn<TItem>. While event args should ideally be immutable snapshots, this design is acceptable in the Blazor component context—columns are stable objects throughout the grid's lifetime. However, for future maintainers, consider adding an inline comment documenting this assumption so any refactoring (e.g., if columns become dynamically mutated) surfaces the design risk.Alternatively, if column mutability becomes a concern, capture immutable column metadata (Id, DisplayTitle) directly into the event args instead of the Column reference itself.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cs` around lines 1 - 26, The `Column` property in the `BitDataGridCellEventArgs<TItem>` class holds a mutable reference to a `BitDataGridColumn<TItem>` object, which violates the principle that event args should be immutable snapshots. Add an inline XML comment above the `Column` property that documents the design assumption that column objects remain stable throughout the grid's lifetime and warns future maintainers about the refactoring risk if columns become dynamically mutated. Alternatively, if mutable column objects become a concern, refactor the event args to capture only the immutable column metadata directly (such as `Id` and `DisplayTitle`) instead of holding a reference to the entire mutable Column object.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts`:
- Around line 10-33: The dispose method only removes the scroll event listener,
but the setTimeout on line 28 and requestAnimationFrame on line 22 can schedule
the check function to execute after disposal, causing the
dotNetRef.invokeMethodAsync call on line 15 to run on a disposed component and
trigger unhandled rejections. Add a disposed flag that is set to true in the
dispose function, then guard the check function to return early if disposed
before calling dotNetRef.invokeMethodAsync, ensuring no interop calls occur
after the component has been cleaned up.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cs`:
- Around line 87-90: The Path property in BitDataGridGroup is being constructed
using keyText, which is a formatted display value from
column.FormatValue(g.Key). This can cause path collisions when different keys
produce identical display values, leading to incorrect collapse/expand state
reuse. Change the path construction on line 89 to use the actual key value g.Key
instead of the formatted keyText when building the path identifier, while
keeping keyText available for display purposes elsewhere.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs`:
- Around line 115-124: The setter construction in the nested property handling
(around the Expression.Assign call in the setter creation block) does not
include null checks for intermediate properties in the chain, unlike the getter
which safely handles nulls. Add null-guard checks in the expression tree to
verify that intermediate properties are not null before attempting to assign to
nested properties. This should prevent SetValue from throwing exceptions when
intermediate nodes in the property path are null.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cs`:
- Around line 8-9: The Priority property in BitDataGridSortDescriptor class has
no default value, causing it to default to 0 in C#. Since the documentation
states "1 = primary" and the sorting logic uses ascending Priority values, a
default of 0 makes unset descriptors become higher priority than those
explicitly set to 1, contradicting the documented behavior. Assign a default
value to the Priority property (consider using int.MaxValue or another
appropriate high value) so that explicitly set priorities take precedence over
uninitialized descriptors and align with the documented sort precedence where 1
is primary.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 373-377: The count calculation on line 376 within the Pagination
null check in the BitQuickGrid class can produce negative values when
request.StartIndex exceeds the remaining items in the page window, which then
causes invalid negative arguments to be passed to Take() method calls downstream
around lines 416-419. Modify the count assignment to clamp the result to a
minimum of zero by wrapping the subtraction expression with Math.Max(0, ...) to
ensure count is never negative, preventing the negative Take argument issue.
- Around line 327-337: The `_pendingDataLoadCancellationTokenSource`
CancellationTokenSource instances are not being properly disposed, causing
resource leaks on frequent refreshes. When canceling the previous token source
at the start of the method and when setting the field to null (in both the
Virtualize branch and the other branch referenced at line 346-352), you must
explicitly dispose each replaced instance before assigning a new one or clearing
the field. Ensure that any old CancellationTokenSource is disposed immediately
after Cancel is called, and dispose the current instance before setting it to
null.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scss`:
- Around line 68-69: The border-inline-start property on line 69 uses a
hardcoded black color instead of the theme token variable used on line 68 for
border-color. Replace the hardcoded black value in the border-inline-start
property with the same theme variable (--bit-clr-brd-pri) to ensure the
resize-handle border color is consistent with the theme and follows the same
design token pattern.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.ts`:
- Around line 41-45: The transform style on colOptions is only set when overhang
exists in the if block, but when the popup is opened again without overhang, the
previous transform value persists and causes misalignment. Add an else clause
after the overhang check to explicitly clear the transform by setting
colOptions.style.transform to an empty string or "none" when neither
leftOverhang nor rightOverhang is true.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cs`:
- Line 36: The InvalidOperationException thrown in the
AsyncQueryExecutorSupplier class references a non-existent package
Microsoft.AspNetCore.Components.BitQuickGrid.EntityFrameworkAdapter and method
AddBitQuickGridEntityFrameworkAdapter that do not exist in the codebase. Update
the exception message to instead reference the actual correct approach using the
IAsyncQueryExecutor abstraction that is mentioned in the existing comments,
providing clear guidance on how developers should implement a custom query
executor to handle Entity Framework queries properly. Ensure the updated message
includes actionable steps or references to documentation so developers can
resolve the issue.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss`:
- Around line 4-5: Remove the .scss file extensions from the two import
statements in the extra-components.scss file. The imports for BitQuickGrid and
BitQuickGridPaginator should reference the files without the .scss extension
(e.g., change `@import` "../Components/QuickGrid/BitQuickGrid.scss" to `@import`
"../Components/QuickGrid/BitQuickGrid"). This will comply with the
scss/load-partial-extension stylelint rule.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`:
- Around line 331-344: The LoadData sample in the filtering and sorting sections
always applies operations to the Name property regardless of which column was
actually requested. Modify the filtering loop to check the field/column name
from the filter request (likely available as a property on the filter object)
and conditionally apply the Contains filter to the appropriate property (Name,
Id, or Price). Similarly, update the sorting section to check which column is
being sorted from the sort request object and apply OrderBy or OrderByDescending
to the correct property instead of always using Name.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor`:
- Around line 164-166: The BitButton component used for the row expand/collapse
toggle lacks an accessible label for assistive technologies. Add an
accessibility attribute (such as AriaLabel or Title) to the BitButton element
that contains the ChevronDown/ChevronRight icon. The label should clearly
describe the button's purpose, such as indicating whether it will expand or
collapse the row details, and should update dynamically based on the same
condition used to toggle the IconName property
(expandedRowTemplateCodes.Contains(context.Code)).
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cs`:
- Around line 68-69: The Description strings in the BitQuickGridDemo class
contain user-facing typos at multiple locations. On line 68, the word "rid"
should be corrected to "grid" in the description text. Additionally, fix any
other grammar or wording errors in the Description properties at lines 160 and
254. Review each Description string for clarity and ensure they are properly
formatted for documentation display purposes.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cs`:
- Around line 726-729: The OData filter query being constructed in the query.Add
call with the contains function is vulnerable to syntax errors when
_odataSampleNameFilter contains single quotes. Escape the user input value by
applying Replace("'", "''") to _odataSampleNameFilter before interpolating it
into the OData filter string. This will properly escape single quotes according
to OData standards and prevent filter syntax breakage.
- Line 738: The GetFromJsonAsync method calls in the sample code are missing the
cancellation token parameter. Add req.CancellationToken as a parameter to both
GetFromJsonAsync calls at lines 738 and 906 to ensure proper cancellation
propagation, making the sample code consistent with the runtime demo provider
implementation.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor`:
- Around line 265-270: The virtualized rendering path in the UseVirtualization
condition is missing the `@key` directive on the BitDataGridRow component, while
the infinite and paged rendering paths both include `@key`="item". Add `@key`="item"
to the BitDataGridRow component inside the Virtualize block to ensure consistent
item identity tracking across all rendering modes and allow Blazor to properly
optimize DOM updates when items are reordered or modified.
- Around line 336-338: The VirtualRows property currently calls ToList() on
every access when _view is not an ICollection<TItem>, causing repeated
allocations during rendering cycles. Add a private cached field to store the
computed virtual rows collection, then populate this cache whenever _view
changes (such as in ProcessClientData or RefreshAsync methods). Update the
VirtualRows property to return the cached field instead of computing the
collection on each access, ensuring the cache remains synchronized whenever the
underlying _view is modified.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 220-222: The VisibleColumns property in the BitDataGrid class
creates a new list on every access via .ToList(), causing unnecessary
allocations since this property is accessed frequently during rendering.
Implement caching by creating a private backing field to store the visible
columns list, update this cached list whenever the underlying _columns
collection changes (add invalidation logic in methods that modify _columns such
as column add/remove operations), and modify the VisibleColumns property to
return the cached list instead of recomputing it with .Where().ToList() on each
access. Ensure the cache is refreshed whenever columns are added, removed, or
their visibility changes.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scss`:
- Around line 218-227: The BitDataGrid spinner and shimmer animations do not
respect user accessibility preferences for reduced motion. Wrap the animation
properties in the `.bit-dtg-spinner` rule and the related animation rules (lines
231-243) in a `@media (prefers-reduced-motion: no-preference)` query so
animations only play for users who have not requested reduced motion. For users
with `prefers-reduced-motion: reduce`, provide fallback styles that disable the
animation property, ensuring the spinner/shimmer elements remain visible but
static.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cs`:
- Around line 1-26: The `Column` property in the
`BitDataGridCellEventArgs<TItem>` class holds a mutable reference to a
`BitDataGridColumn<TItem>` object, which violates the principle that event args
should be immutable snapshots. Add an inline XML comment above the `Column`
property that documents the design assumption that column objects remain stable
throughout the grid's lifetime and warns future maintainers about the
refactoring risk if columns become dynamically mutated. Alternatively, if
mutable column objects become a concern, refactor the event args to capture only
the immutable column metadata directly (such as `Id` and `DisplayTitle`) instead
of holding a reference to the entire mutable Column object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 908cae7a-5590-47b0-bff5-7de18c8d32e9
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cs (1)
89-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a typed, culture-invariant identity for group path keys.
Line 91 still relies on string interpolation of
g.Key, which can collide when distinct keys produce identical text and causes incorrect collapse/expand state reuse.Suggested fix
var keyText = column.FormatValue(g.Key); var items = g.ToList(); // Use the raw key (not the formatted display text) for the path identifier so that // distinct keys producing identical display values don't collide and share collapse/expand state. - var path = $"{parentPath}/{level}:{g.Key}"; + var keyIdentity = g.Key is null + ? "null" + : g.Key is IFormattable f + ? $"{g.Key.GetType().FullName}:{f.ToString(null, CultureInfo.InvariantCulture)}" + : $"{g.Key.GetType().FullName}:{g.Key}"; + var path = $"{parentPath}/{level}:{descriptor.ColumnId}:{keyIdentity}";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cs` around lines 89 - 92, In the BitDataGridDataProcessor.cs file where the path variable is constructed using string interpolation with g.Key, replace the simple string concatenation of g.Key with a typed, culture-invariant identifier that prevents collisions between distinct keys producing identical text. Instead of relying on the string representation of g.Key which can be culture-dependent and cause identical display values to collide, use a robust uniqueness mechanism such as the key's hash code or another unique type-safe identifier that preserves the actual key identity across different key instances.
🧹 Nitpick comments (2)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cs (1)
9-9: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider making
FormattedValuerequired or nullable for consistency.The
FormattedValueproperty has a default value ofstring.Emptybut is not marked asrequired, whileColumnIdis required. This creates an inconsistent initialization pattern. IfFormattedValueis always expected to have a value, mark itrequired; if it's truly optional, consider making itstring?with anulldefault instead.♻️ Proposed options
Option 1: Make it required
- public string FormattedValue { get; init; } = string.Empty; + public required string FormattedValue { get; init; }Option 2: Make it nullable
- public string FormattedValue { get; init; } = string.Empty; + public string? FormattedValue { get; init; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cs` at line 9, The `FormattedValue` property in the `BitDataGridAggregateResult` class has a default value of `string.Empty` but is not marked as `required`, creating an inconsistency with the `ColumnId` property which is required. To fix this, either mark `FormattedValue` as `required` to match the pattern of `ColumnId`, or change its type to `string?` and set the default value to `null` to properly represent it as an optional nullable property. Choose the approach that best represents the semantic intent of this property in your data model.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cs (1)
25-25: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winInconsistent use of
requiredmodifier.The
ValueandMouseproperties are not marked asrequired, but based on the construction code inBitDataGrid.razor.cs:947, they are always explicitly set:new() { Item = item, Column = column, Value = column.GetValue(item), Mouse = e }This is inconsistent with
ItemandColumnbeing markedrequired. Either markValueandMouseasrequiredfor consistency and compile-time safety, or document why they are intentionally optional.♻️ Proposed fix for consistency
/// <summary>The raw value of the cell.</summary> - public object? Value { get; init; } + public required object? Value { get; init; } /// <summary>The underlying browser mouse event.</summary> - public MouseEventArgs Mouse { get; init; } = new(); + public required MouseEventArgs Mouse { get; init; }Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cs` at line 25, The BitDataGridCellEventArgs class has inconsistent use of the required modifier. The Value property at line 25 and Mouse property at line 28 are not marked as required, while Item and Column properties are. Since the construction code in BitDataGrid.razor.cs always explicitly sets all four properties when creating new instances, either add the required modifier to the Value and Mouse properties to match Item and Column for consistency and compile-time safety, or add documentation comments explaining why these two properties are intentionally optional despite always being set during construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 161-162: The _selected HashSet stores TItem references, but when
items are refreshed from server or through infinite scrolling, new object
instances are created with the same key value. Since reference equality fails
for these new instances, previously selected rows appear unselected and
select-all state becomes incorrect. Change the selection mechanism to track
items by their KeyField value instead of object reference. This means either
storing the key values in the _selected collection instead of TItem instances,
or implementing a custom equality comparer for TItem that compares based on the
KeyField value. Apply the same fix to _expandedDetails which has the same issue
at lines 227-230 and 697-715.
- Around line 458-478: The LoadNextBatchAsync method has a race condition where
older OnLoadMore responses can still mutate state after newer requests are
initiated, causing stale data. Implement a version-guard pattern by tracking the
current request version before calling OnLoadMore (similar to how
ResetLoadCancellation is used), then verify this version matches the current
state before applying the response mutations to _infiniteItems, _view,
_pageItems, and _footerAggregates. Apply the same version-guard pattern to both
the LoadNextBatchAsync method shown in the diff and the other similar location
mentioned in the comment (lines 543-557).
- Around line 1137-1139: The ReorderStickyStyle, DetailStickyStyle, and
SelectStickyStyle properties hardcode "left:" for positioning but should use
"right:" when the page is in RTL (right-to-left) mode. Modify each of these
three property definitions to conditionally output either "left:" or "right:"
based on the current RTL state, similar to how the FrozenStyle property handles
directional positioning. Check the BitDataGrid component for any existing RTL
detection logic or boolean flag you can use to determine when to use "right:"
instead of "left:".
- Around line 581-587: The condition on line 581 in the sort-clearing logic uses
`if (!additive && !MultiSort)` which requires both conditions to be true, but
this causes existing sorts to persist when MultiSort=true and a non-additive
click occurs (without Ctrl/Meta). The fix is to change the condition to only
check `if (!additive)` so that prior sorts are cleared whenever it is a
non-additive action, regardless of the MultiSort setting, making the behavior
consistent with Line 393 where additive mode is tied to Ctrl/Meta detection.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razor`:
- Around line 38-45: The column spanning logic in BitDataGridRow.razor skips
rendering columns when span is greater than 1, but the keyboard focus and
navigation model still operates using the full VisibleColumns index space,
creating a mismatch where focus targets can point to non-existent cells. Update
the focus/navigation model to map column indices to account for spanned columns,
ensuring that when columns are skipped due to spanning in the rendering loop
(where skip is decremented and columns are skipped), the corresponding focus
targets and tabindex assignments properly exclude those skipped column indices
so that keyboard focus always lands on a valid rendered BitDataGridCell with
tabindex=0.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 374-381: The Task.Delay(100) call in the debounce logic does not
respect the cancellation token, causing unnecessary delays even when virtualized
requests have already been canceled. Pass the request.CancellationToken as a
parameter to Task.Delay so the delay can be interrupted early if cancellation is
requested. Change Task.Delay(100) to Task.Delay(100, request.CancellationToken)
in the debounce section to make the delay cancellation-aware.
- Around line 324-366: In the RefreshDataCoreAsync method, wrap the call to
ResolveItemsRequestAsync(request) in a try-catch block to explicitly handle
OperationCanceledException. When this exception is caught, it indicates the load
was superseded by a newer request, so ensure the cleanup logic (checking if this
load is still current and disposing the cancellation token source) still
executes properly before allowing the method to complete gracefully without
propagating the exception. This prevents noisy failures during rapid sort/page
refreshes and keeps the load-state cleanup paths robust.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.ts`:
- Around line 83-87: The column resize logic does not enforce a minimum width
constraint on the nextWidth calculation, allowing columns to become negative or
excessively small during drag operations. Add a minimum width constraint by
clamping nextWidth to a reasonable minimum value (such as 20 or 30 pixels)
before assigning it to updatedColumnWidth and applying it to th.style.width.
This prevents column collapse and maintains layout stability during resizing
operations.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs`:
- Around line 139-196: The LoadServerData method updates serverLoading and
serverLastRequest at the end but does not trigger a final StateHasChanged call
to re-render the parent component. Since this is a callback function, these
state changes may render late or not at all. Wrap the lines that set
serverLoading to false and update serverLastRequest in a finally block, and add
an await InvokeAsync(StateHasChanged) call within that finally block to ensure
the parent component always re-renders after the data loading operation
completes.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cs`:
- Around line 384-386: The Razor snippet in the
BitQuickGridDemo.razor.samples.cs file binds to `virtualSampleNameFilter` in the
`@bind-Value` attribute of the BitSearchBox component, but the corresponding C#
property is defined as `VirtualSampleNameFilter` in PascalCase. Update the
binding to use `VirtualSampleNameFilter` to match the actual property name
defined in the C# code-behind, ensuring the sample compiles correctly when
copied.
- Around line 1090-1093: The BitButton component at lines 1090-1093 that
controls row expansion lacks accessibility attributes. Add a dynamic AriaLabel
or Title attribute to the BitButton element that changes based on the expansion
state. The label should indicate whether clicking the button will expand or
collapse the row, similar to the implementation in the runtime demo. Reference
the expandedRowTemplateCodes collection to conditionally set the label, matching
the same logic used for the IconName attribute selection.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cs`:
- Around line 89-92: In the BitDataGridDataProcessor.cs file where the path
variable is constructed using string interpolation with g.Key, replace the
simple string concatenation of g.Key with a typed, culture-invariant identifier
that prevents collisions between distinct keys producing identical text. Instead
of relying on the string representation of g.Key which can be culture-dependent
and cause identical display values to collide, use a robust uniqueness mechanism
such as the key's hash code or another unique type-safe identifier that
preserves the actual key identity across different key instances.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cs`:
- Line 9: The `FormattedValue` property in the `BitDataGridAggregateResult`
class has a default value of `string.Empty` but is not marked as `required`,
creating an inconsistency with the `ColumnId` property which is required. To fix
this, either mark `FormattedValue` as `required` to match the pattern of
`ColumnId`, or change its type to `string?` and set the default value to `null`
to properly represent it as an optional nullable property. Choose the approach
that best represents the semantic intent of this property in your data model.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cs`:
- Line 25: The BitDataGridCellEventArgs class has inconsistent use of the
required modifier. The Value property at line 25 and Mouse property at line 28
are not marked as required, while Item and Column properties are. Since the
construction code in BitDataGrid.razor.cs always explicitly sets all four
properties when creating new instances, either add the required modifier to the
Value and Mouse properties to match Item and Column for consistency and
compile-time safety, or add documentation comments explaining why these two
properties are intentionally optional despite always being set during
construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e5869d38-2410-48a4-ad7d-364183e27bec
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs (1)
324-346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnsure CTS cleanup also runs for the virtualized branch.
thisLoadCtscleanup is currently only guaranteed in the non-virtualizedfinally. IfRefreshDataAsync()throws in the virtualized path,_pendingDataLoadCancellationTokenSourcecan stay non-null and undisposed.Suggested minimal restructuring
private async Task RefreshDataCoreAsync() { var previousCts = _pendingDataLoadCancellationTokenSource; if (previousCts is not null) { previousCts.Cancel(); previousCts.Dispose(); } var thisLoadCts = _pendingDataLoadCancellationTokenSource = new CancellationTokenSource(); - - if (_virtualizeComponent is not null) - { - await _virtualizeComponent.RefreshDataAsync(); - if (ReferenceEquals(_pendingDataLoadCancellationTokenSource, thisLoadCts)) - { - thisLoadCts.Dispose(); - _pendingDataLoadCancellationTokenSource = null; - } - } - else + try { - _lastRefreshedPaginationStateHash = Pagination?.GetHashCode(); - var startIndex = Pagination is null ? 0 : (Pagination.CurrentPageIndex * Pagination.ItemsPerPage); - var request = new BitQuickGridItemsProviderRequest<TGridItem>( - startIndex, Pagination?.ItemsPerPage, _sortByColumn, _sortByAscending, thisLoadCts.Token); - try + if (_virtualizeComponent is not null) { - var result = await ResolveItemsRequestAsync(request); - if (!thisLoadCts.IsCancellationRequested) - { - _currentNonVirtualizedViewItems = result.Items; - _ariaBodyRowCount = _currentNonVirtualizedViewItems.Count; - await (Pagination?.SetTotalItemCountAsync(result.TotalItemCount) ?? Task.CompletedTask); - } + await _virtualizeComponent.RefreshDataAsync(); } - catch (OperationCanceledException) + else { - // This load was superseded by a newer request; swallow the cancellation and fall through - // to the cleanup below so the load-state remains consistent. + _lastRefreshedPaginationStateHash = Pagination?.GetHashCode(); + var startIndex = Pagination is null ? 0 : (Pagination.CurrentPageIndex * Pagination.ItemsPerPage); + var request = new BitQuickGridItemsProviderRequest<TGridItem>( + startIndex, Pagination?.ItemsPerPage, _sortByColumn, _sortByAscending, thisLoadCts.Token); + var result = await ResolveItemsRequestAsync(request); + if (!thisLoadCts.IsCancellationRequested) + { + _currentNonVirtualizedViewItems = result.Items; + _ariaBodyRowCount = _currentNonVirtualizedViewItems.Count; + await (Pagination?.SetTotalItemCountAsync(result.TotalItemCount) ?? Task.CompletedTask); + } } - finally - { - if (ReferenceEquals(_pendingDataLoadCancellationTokenSource, thisLoadCts)) - { - thisLoadCts.Dispose(); - _pendingDataLoadCancellationTokenSource = null; - } - } + } + catch (OperationCanceledException) when (thisLoadCts.IsCancellationRequested) + { + // Superseded by a newer request. + } + finally + { + if (ReferenceEquals(_pendingDataLoadCancellationTokenSource, thisLoadCts)) + { + _pendingDataLoadCancellationTokenSource = null; + } + thisLoadCts.Dispose(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs` around lines 324 - 346, The RefreshDataCoreAsync method's virtualized branch does not guarantee cleanup of thisLoadCts if an exception is thrown during the RefreshDataAsync() call on the _virtualizeComponent. The cleanup logic that disposes thisLoadCts and clears _pendingDataLoadCancellationTokenSource (the ReferenceEquals check) should be wrapped in a try-finally block around the virtualized code path to ensure it always executes even when RefreshDataAsync() throws, preventing resource leaks and keeping _pendingDataLoadCancellationTokenSource properly managed.
🧹 Nitpick comments (7)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs (1)
323-364: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winReset
loadinginfinallyand honor cancellation in the server-read sample.This sample can leave
loading = trueif an exception/cancellation happens before Line 362. SinceBitDataGridReadRequestexposesCancellationToken, it should be used in the simulated async work as well.Suggested snippet adjustment
private async Task<BitDataGridReadResult<Product>> LoadData(BitDataGridReadRequest request) { - loading = true; - await Task.Delay(250); // simulate a backend round-trip - - IEnumerable<Product> query = all; - - // filtering - foreach (var f in request.Filters) - { - var term = f.Value?.ToString() ?? ""; - query = f.ColumnId switch - { - nameof(Product.Name) => query.Where(p => p.Name.Contains(term, StringComparison.OrdinalIgnoreCase)), - nameof(Product.Price) => query.Where(p => p.Price.ToString().Contains(term)), - nameof(Product.Id) => query.Where(p => p.Id.ToString().Contains(term)), - _ => query - }; - } - - // sorting - var sort = request.Sorts.FirstOrDefault(); - if (sort is not null) - { - Func<Product, object> key = sort.ColumnId switch - { - nameof(Product.Name) => p => p.Name, - nameof(Product.Price) => p => p.Price, - _ => p => p.Id - }; - query = sort.Direction == BitDataGridSortDirection.Descending - ? query.OrderByDescending(key) - : query.OrderBy(key); - } - - // paging - var filtered = query.ToList(); - var items = filtered.Skip(request.Skip).Take(request.Take ?? filtered.Count).ToList(); - - loading = false; - return new BitDataGridReadResult<Product>(items, filtered.Count); + loading = true; + try + { + await Task.Delay(250, request.CancellationToken); // simulate a backend round-trip + + IEnumerable<Product> query = all; + + // filtering + foreach (var f in request.Filters) + { + var term = f.Value?.ToString() ?? ""; + query = f.ColumnId switch + { + nameof(Product.Name) => query.Where(p => p.Name.Contains(term, StringComparison.OrdinalIgnoreCase)), + nameof(Product.Price) => query.Where(p => p.Price.ToString().Contains(term)), + nameof(Product.Id) => query.Where(p => p.Id.ToString().Contains(term)), + _ => query + }; + } + + // sorting + var sort = request.Sorts.FirstOrDefault(); + if (sort is not null) + { + Func<Product, object> key = sort.ColumnId switch + { + nameof(Product.Name) => p => p.Name, + nameof(Product.Price) => p => p.Price, + _ => p => p.Id + }; + query = sort.Direction == BitDataGridSortDirection.Descending + ? query.OrderByDescending(key) + : query.OrderBy(key); + } + + // paging + var filtered = query.ToList(); + var items = filtered.Skip(request.Skip).Take(request.Take ?? filtered.Count).ToList(); + return new BitDataGridReadResult<Product>(items, filtered.Count); + } + finally + { + loading = false; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs` around lines 323 - 364, The LoadData method sets loading to true but only resets it to false at the end of the method, leaving it stuck at true if an exception occurs. Additionally, the simulated async work via Task.Delay does not honor the CancellationToken from the BitDataGridReadRequest. Wrap the method body in a try-finally block and move the loading = false assignment to the finally block to ensure it always executes regardless of exceptions. Also, pass request.CancellationToken to the Task.Delay call to respect cancellation requests.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor (2)
403-403: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
int.Parsecould throw on malformed select value.While the
<select>only contains valid integer options, usingint.Parsewithout validation is fragile. Considerint.TryParsefor defensive coding.♻️ Defensive parsing
-<select class="bit-dtg-page-size" `@onchange`="e => SetPageSizeAsync(int.Parse((string)e.Value!))"> +<select class="bit-dtg-page-size" `@onchange`="e => { if (int.TryParse((string?)e.Value, out var size)) _ = SetPageSizeAsync(size); }">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor` at line 403, The select element's `@onchange` handler in the page size dropdown uses int.Parse without error handling, which can throw exceptions if the value is malformed. Replace int.Parse((string)e.Value!) with int.TryParse to safely attempt the conversion and gracefully handle parsing failures. Check the result of TryParse before calling SetPageSizeAsync, and only proceed if the conversion succeeds.
27-31: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffCSV data URI export may fail for large datasets.
The CSV export uses a
data:text/csvURI which has browser-specific length limits (typically 2MB in modern browsers, but varies). For grids with thousands of rows, this approach may silently truncate or fail.Consider adding a fallback using Blob URLs or JS interop for larger exports if this becomes an issue.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor` around lines 27 - 31, The CSV export functionality in the BitDataGrid component uses a data:text/csv URI approach which has browser limitations (typically 2MB) and may silently fail for large datasets. Modify the CSV export section to check the size of the CSV data generated by the ToCsv() method, and if it exceeds a reasonable threshold (around 1-2MB), implement a fallback approach using Blob URLs or JavaScript interop to trigger the download instead of using the data URI. This ensures large exports work reliably while maintaining backward compatibility for smaller datasets that work fine with the current data URI approach.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs (3)
226-226: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
VisibleColumnsallocates a new list on every access.The property
VisibleColumnscalls.ToList()on every access, creating a new allocation each time. It's accessed frequently (header rendering, cell navigation, spanning, footer, etc.), causing unnecessary GC pressure.♻️ Consider caching the visible columns list
+ private IReadOnlyList<BitDataGridColumn<TItem>>? _visibleColumnsCache; + private int _visibleColumnsVersion; + - internal IReadOnlyList<BitDataGridColumn<TItem>> VisibleColumns => _columns.Where(c => c.Visible).ToList(); + internal IReadOnlyList<BitDataGridColumn<TItem>> VisibleColumns + { + get + { + // Invalidate cache when columns change + if (_visibleColumnsCache is null || _visibleColumnsVersion != _columns.Count) + { + _visibleColumnsCache = _columns.Where(c => c.Visible).ToList(); + _visibleColumnsVersion = _columns.Count; + } + return _visibleColumnsCache; + } + }Also invalidate the cache in
AddColumn,RemoveColumn, andSetColumnVisibilityAsync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` at line 226, The VisibleColumns property creates a new list allocation on every access via .ToList(), causing unnecessary garbage collection. Cache the filtered list result in a private field (e.g., _visibleColumnsCache) and update the VisibleColumns property to return the cached value. Invalidate the cache by resetting it to null in the AddColumn, RemoveColumn, and SetColumnVisibilityAsync methods so the list is only recalculated when columns are actually modified, not on every property access.
734-735: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMultiple enumerations over
_pageItemsfor selection state.
AllPageSelectedandSomePageSelectedboth call_pageItems.Where(CanSelectRow)multiple times, causing repeated enumeration. Consider caching the selectable items.♻️ Suggested optimization
- internal bool AllPageSelected => _pageItems.Where(CanSelectRow).Any() && _pageItems.Where(CanSelectRow).All(_selected.Contains); - internal bool SomePageSelected => _pageItems.Where(CanSelectRow).Any(_selected.Contains) && !AllPageSelected; + internal bool AllPageSelected + { + get + { + var selectable = _pageItems.Where(CanSelectRow).ToList(); + return selectable.Count > 0 && selectable.All(_selected.Contains); + } + } + + internal bool SomePageSelected + { + get + { + var selectable = _pageItems.Where(CanSelectRow).ToList(); + return selectable.Any(_selected.Contains) && !selectable.All(_selected.Contains); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 734 - 735, The AllPageSelected and SomePageSelected properties both enumerate _pageItems.Where(CanSelectRow) multiple times within their respective expressions, causing inefficient repeated filtering. Refactor both properties by caching the filtered enumerable result (the result of _pageItems.Where(CanSelectRow)) into a local variable first, then use that cached variable in the subsequent checks with Any, All, and Contains operations to avoid multiple enumerations.
1068-1072: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUnnecessary list allocation in
ResolveColSpan.
cols.ToList().IndexOf(column)creates a temporary list just to find the index. Use a loop or LINQ instead.♻️ Suggested fix
internal int ResolveColSpan(BitDataGridColumn<TItem> column, TItem item) { if (column.ColSpan is null) return 1; var span = column.ColSpan(item) ?? 1; if (span < 1) span = 1; var cols = VisibleColumns; - var idx = cols.ToList().IndexOf(column); + var idx = -1; + for (int i = 0; i < cols.Count; i++) + { + if (cols[i] == column) { idx = i; break; } + } if (idx < 0) return 1; return Math.Min(span, cols.Count - idx); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 1068 - 1072, The ResolveColSpan method is creating an unnecessary temporary list by calling cols.ToList() solely to use the IndexOf method for finding the column's position. Replace this inefficient approach by either using LINQ enumeration-based indexing without materialization, or iterating through the cols collection with a counter to find the index position directly, avoiding the allocation of the temporary list.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razor (1)
24-31: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueSelect element has redundant value binding.
The
<select>element has bothvalue="@(GetString())"and each<option>hasselected="@(name == GetString())". The Blazorvaluebinding on<select>should suffice; the explicitselectedattribute is redundant and can cause inconsistent behavior in some browsers.♻️ Suggested simplification
- <select class="bit-dtg-editor" value="@(GetString())" `@onchange`="e => Set(e.Value)"> + <select class="bit-dtg-editor" `@bind`="`@_enumValue`" `@bind`:event="onchange" `@bind`:after="() => Set(_enumValue)"> `@foreach` (var name in Enum.GetNames(Column.Accessor!.UnderlyingType)) { - <option value="`@name`" selected="@(name == GetString())">`@name`</option> + <option value="`@name`">`@name`</option> } </select>Or simply remove the
selectedattribute sincevalueon<select>handles selection:- <option value="`@name`" selected="@(name == GetString())">`@name`</option> + <option value="`@name`">`@name`</option>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razor` around lines 24 - 31, The Enum case in the BitDataGridCellEditor.razor component has redundant selection binding on the select element. Remove the explicit selected="@(name == GetString())" attribute from each option element within the foreach loop, as the value binding on the parent select element with value="@(GetString())" already handles selecting the correct option in Blazor. Keep only the value binding on the select element for proper Blazor two-way binding behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.ts`:
- Around line 11-18: The invokeMethodAsync call in the check function lacks
error handling for promise rejections that may occur if the Blazor circuit
disconnects between the disposed flag check and the async method invocation. Add
a .catch() handler to the
dotNetRef.invokeMethodAsync('OnInfiniteScrollNearEndAsync') call to gracefully
handle potential promise rejections and prevent unhandled rejection errors in
the console during circuit disconnects or navigation.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cs`:
- Around line 201-219: The filter evaluation in the method has incorrect null
comparison semantics. The early return on line 201-202 when filter.Value is null
causes all rows to match, which is incorrect for Equals and NotEquals operators.
Additionally, when the row value itself is null, null values are passed to the
comparer which can cause incorrect matches for LessThan and LessThanOrEqual
operators. Remove the early return for null filter values, and instead add
proper null handling logic that checks if the row value is null separately
before calling BitDataGridValueComparer.Instance.Compare(). When the row value
is null, return false for comparison operators (GreaterThan, GreaterThanOrEqual,
LessThan, LessThanOrEqual) and handle Equals/NotEquals based on whether the
filter value is also null.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs`:
- Around line 89-90: The Split call in BitDataGridPropertyAccessor.cs uses
StringSplitOptions.RemoveEmptyEntries which silently normalizes malformed paths
like "." and "Address..City" instead of rejecting them. Remove the
RemoveEmptyEntries option from the Split call and add validation logic to check
if any segments are empty or whitespace, then throw an appropriate exception
(such as ArgumentException) if found, ensuring that only valid property paths
are processed and preventing silent misbinding in sort/filter/edit operations.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 411-414: The ProvideVirtualizedItems method calls
ResolveItemsRequestAsync which can throw OperationCanceledException when the
virtualized request is superseded by a newer request after debounce. Wrap the
ResolveItemsRequestAsync call in a try-catch block to explicitly catch and
handle OperationCanceledException so it does not propagate out of the
ProvideVirtualizedItems method. Handle the exception appropriately by returning
an empty or default result set that the virtualization system expects.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs`:
- Around line 215-231: The LoadMore method currently applies only the first sort
descriptor from request.Sorts by using FirstOrDefault(), which causes
multi-column sorts to be ignored. Replace the single sort application logic with
a loop that iterates through all items in request.Sorts (if any exist) and
applies each sort descriptor sequentially to the query. Reuse the key selector
switch statement that maps sort.ColumnId to the Product properties for each sort
descriptor, applying OrderByDescending or OrderBy based on each sort's Direction
property to maintain proper multi-sort ordering.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 324-346: The RefreshDataCoreAsync method's virtualized branch does
not guarantee cleanup of thisLoadCts if an exception is thrown during the
RefreshDataAsync() call on the _virtualizeComponent. The cleanup logic that
disposes thisLoadCts and clears _pendingDataLoadCancellationTokenSource (the
ReferenceEquals check) should be wrapped in a try-finally block around the
virtualized code path to ensure it always executes even when RefreshDataAsync()
throws, preventing resource leaks and keeping
_pendingDataLoadCancellationTokenSource properly managed.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor`:
- Line 403: The select element's `@onchange` handler in the page size dropdown
uses int.Parse without error handling, which can throw exceptions if the value
is malformed. Replace int.Parse((string)e.Value!) with int.TryParse to safely
attempt the conversion and gracefully handle parsing failures. Check the result
of TryParse before calling SetPageSizeAsync, and only proceed if the conversion
succeeds.
- Around line 27-31: The CSV export functionality in the BitDataGrid component
uses a data:text/csv URI approach which has browser limitations (typically 2MB)
and may silently fail for large datasets. Modify the CSV export section to check
the size of the CSV data generated by the ToCsv() method, and if it exceeds a
reasonable threshold (around 1-2MB), implement a fallback approach using Blob
URLs or JavaScript interop to trigger the download instead of using the data
URI. This ensures large exports work reliably while maintaining backward
compatibility for smaller datasets that work fine with the current data URI
approach.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Line 226: The VisibleColumns property creates a new list allocation on every
access via .ToList(), causing unnecessary garbage collection. Cache the filtered
list result in a private field (e.g., _visibleColumnsCache) and update the
VisibleColumns property to return the cached value. Invalidate the cache by
resetting it to null in the AddColumn, RemoveColumn, and
SetColumnVisibilityAsync methods so the list is only recalculated when columns
are actually modified, not on every property access.
- Around line 734-735: The AllPageSelected and SomePageSelected properties both
enumerate _pageItems.Where(CanSelectRow) multiple times within their respective
expressions, causing inefficient repeated filtering. Refactor both properties by
caching the filtered enumerable result (the result of
_pageItems.Where(CanSelectRow)) into a local variable first, then use that
cached variable in the subsequent checks with Any, All, and Contains operations
to avoid multiple enumerations.
- Around line 1068-1072: The ResolveColSpan method is creating an unnecessary
temporary list by calling cols.ToList() solely to use the IndexOf method for
finding the column's position. Replace this inefficient approach by either using
LINQ enumeration-based indexing without materialization, or iterating through
the cols collection with a counter to find the index position directly, avoiding
the allocation of the temporary list.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razor`:
- Around line 24-31: The Enum case in the BitDataGridCellEditor.razor component
has redundant selection binding on the select element. Remove the explicit
selected="@(name == GetString())" attribute from each option element within the
foreach loop, as the value binding on the parent select element with
value="@(GetString())" already handles selecting the correct option in Blazor.
Keep only the value binding on the select element for proper Blazor two-way
binding behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`:
- Around line 323-364: The LoadData method sets loading to true but only resets
it to false at the end of the method, leaving it stuck at true if an exception
occurs. Additionally, the simulated async work via Task.Delay does not honor the
CancellationToken from the BitDataGridReadRequest. Wrap the method body in a
try-finally block and move the loading = false assignment to the finally block
to ensure it always executes regardless of exceptions. Also, pass
request.CancellationToken to the Task.Delay call to respect cancellation
requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 181acf5c-8519-490b-bb7d-03de1ca65181
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cs (1)
13-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting common generation logic to reduce duplication.
The
GenerateandGeneratePersianmethods duplicate the entire Product generation algorithm. Only the string arrays differ (adjectives, nouns, suppliers). Extracting the common logic into a shared helper would improve maintainability and ensure consistency when updating the generation algorithm.♻️ Refactoring approach
Extract a private helper that accepts the string arrays as parameters:
+ private static List<Product> GenerateCore( + int count, + int seed, + string[] adjectives, + string[] nouns, + string[] suppliers) + { + var rng = new Random(seed); + var categories = Enum.GetValues<Category>(); + var list = new List<Product>(count); + var referenceDate = new DateTime(2024, 1, 1); + for (int i = 1; i <= count; i++) + { + list.Add(new Product + { + Id = i, + Name = $"{adjectives[rng.Next(adjectives.Length)]} {nouns[rng.Next(nouns.Length)]} {rng.Next(100, 999)}", + Category = categories[rng.Next(categories.Length)], + Price = Math.Round((decimal)(rng.NextDouble() * 990 + 5), 2), + Stock = rng.Next(0, 500), + Rating = Math.Round(rng.NextDouble() * 4 + 1, 1), + Discontinued = rng.Next(0, 5) == 0, + ReleaseDate = referenceDate.AddDays(-rng.Next(0, 2000)), + Supplier = suppliers[rng.Next(suppliers.Length)] + }); + } + return list; + } + public static List<Product> Generate(int count, int seed = 42) - { - var rng = new Random(seed); - var categories = Enum.GetValues<Category>(); - var list = new List<Product>(count); - var referenceDate = new DateTime(2024, 1, 1); - for (int i = 1; i <= count; i++) - { - list.Add(new Product - { - Id = i, - Name = $"{Adjectives[rng.Next(Adjectives.Length)]} {Nouns[rng.Next(Nouns.Length)]} {rng.Next(100, 999)}", - Category = categories[rng.Next(categories.Length)], - Price = Math.Round((decimal)(rng.NextDouble() * 990 + 5), 2), - Stock = rng.Next(0, 500), - Rating = Math.Round(rng.NextDouble() * 4 + 1, 1), - Discontinued = rng.Next(0, 5) == 0, - ReleaseDate = referenceDate.AddDays(-rng.Next(0, 2000)), - Supplier = Suppliers[rng.Next(Suppliers.Length)] - }); - } - return list; - } + => GenerateCore(count, seed, Adjectives, Nouns, Suppliers); public static List<Product> GeneratePersian(int count, int seed = 42) - { - var rng = new Random(seed); - var categories = Enum.GetValues<Category>(); - var list = new List<Product>(count); - var referenceDate = new DateTime(2024, 1, 1); - for (int i = 1; i <= count; i++) - { - list.Add(new Product - { - Id = i, - Name = $"{PersianAdjectives[rng.Next(PersianAdjectives.Length)]} {PersianNouns[rng.Next(PersianNouns.Length)]} {rng.Next(100, 999)}", - Category = categories[rng.Next(categories.Length)], - Price = Math.Round((decimal)(rng.NextDouble() * 990 + 5), 2), - Stock = rng.Next(0, 500), - Rating = Math.Round(rng.NextDouble() * 4 + 1, 1), - Discontinued = rng.Next(0, 5) == 0, - ReleaseDate = referenceDate.AddDays(-rng.Next(0, 2000)), - Supplier = PersianSuppliers[rng.Next(PersianSuppliers.Length)] - }); - } - return list; - } + => GenerateCore(count, seed, PersianAdjectives, PersianNouns, PersianSuppliers);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cs` around lines 13 - 68, The Generate and GeneratePersian methods contain duplicate code for the entire Product generation algorithm, differing only in which string arrays they use. Create a private helper method that accepts three string array parameters (for adjectives, nouns, and suppliers) and contains the common generation logic currently duplicated in both methods. Then refactor the Generate method to call this helper with Adjectives, Nouns, and Suppliers arrays, and the GeneratePersian method to call it with PersianAdjectives, PersianNouns, and PersianSuppliers arrays. This will eliminate duplication while maintaining the same external behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 456-498: The LoadNextBatchAsync method awaits the OnLoadMore
callback which receives a cancellation token from ResetLoadCancellation(). If
the provider honors the cancellation token, it will throw an
OperationCanceledException that breaks the component flow. Wrap the await
statement for OnLoadMore in a try-catch block to catch
OperationCanceledException and handle it gracefully by returning early, since
cancellation is expected behavior when requests are superseded by newer loads.
This same fix should also be applied to any other similar load methods mentioned
in the "Also applies to" section that use ResetLoadCancellation() tokens.
- Around line 1199-1212: The Escape static method within the ToCsv() method
currently only handles CSV delimiter escaping but does not prevent formula
injection. When cell values begin with =, +, -, or @, spreadsheet applications
can interpret them as formulas when the CSV file is opened. Modify the Escape
static method to detect if the input value starts with any of these dangerous
characters and prepend a single quote to prevent formula execution, in addition
to the existing CSV delimiter escaping logic.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razor`:
- Line 13: The `@onkeydown`="HandleKeyDown" binding at line 13 does not prevent
the default browser keyboard behavior, which can conflict with the grid
navigation logic implemented in the HandleKeyDown method (referenced around line
70). Modify the onkeydown event binding to include preventDefault:true parameter
to suppress native browser key handling and ensure the custom grid navigation
logic executes exclusively without browser defaults interfering with the
cell-navigation behavior.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razor`:
- Around line 24-27: The enum editor case in BitDataGridCellEditor.razor does
not validate that Column.Accessor exists and that its UnderlyingType is actually
an enum before attempting to call Enum.GetNames on it. Add a guard condition
before the select element that checks whether Column.Accessor is not null and
whether Column.Accessor.UnderlyingType is a valid enum type using Type.IsEnum.
If the guard condition fails, render a fallback editor instead of the enum
select dropdown to prevent null reference exceptions and invalid type operations
at runtime.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cs`:
- Around line 36-40: The ConvertValue method (referenced around lines 68-70) is
silently coercing conversion failures to default values for non-nullable
properties, which causes invalid user input to be silently overwritten with
defaults like 0. Instead of silently returning default(T) on conversion failure,
modify ConvertValue to throw an exception or return a result that indicates
conversion failure, then update the SetValue method to handle this exception or
failure result appropriately by either not setting the value or propagating the
error to the caller so users are aware their invalid input was rejected rather
than silently converted.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 302-306: The call to _js.BitQuickGridCheckColumnOptionsPosition on
line 305 discards the returned ValueTask using the discard pattern instead of
awaiting it. Since the containing method is already async, replace the line that
discards the result with `await
_js.BitQuickGridCheckColumnOptionsPosition(_tableReference);` to properly await
the interop call and ensure positioning errors are not hidden.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor`:
- Around line 156-161: The Math.Round(p.Rating) call on line 160 uses banker's
rounding (midpoint-to-even), which causes inconsistent star rendering when the
rating is exactly at a midpoint value like 4.5. To fix this, replace
Math.Round(p.Rating) with Math.Round(p.Rating, MidpointRounding.AwayFromZero) to
ensure values round away from zero, providing more intuitive and consistent
visual feedback in the star rating display within the span element.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs`:
- Around line 139-144: The LoadServerData method and other async loader methods
do not honor the CancellationToken provided in BitDataGridReadRequest, allowing
superseded requests to complete and update stale UI state. Pass
request.CancellationToken to the Task.Delay() calls, and check for cancellation
(using ThrowIfCancellationRequested or similar checks) before updating UI state
variables like serverLoading, serverLastRequest, and infiniteLog to prevent
stale data from being written when a newer request supersedes the current one.
Apply this fix to all async loader methods mentioned, including LoadServerData
and any other similar loaders at the specified locations.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`:
- Around line 346-358: The sort logic in this sample applies only the first sort
descriptor using FirstOrDefault() and ignores all additional sorts. Instead of
handling only a single sort, iterate through all items in request.Sorts and
chain the sort operations using OrderBy/OrderByDescending for the first sort and
ThenBy/ThenByDescending for subsequent sorts based on their respective column
IDs and directions. Apply this same complete sorting logic to both the initial
query sorting section (around the switch statement with Product.Name,
Product.Price cases) and the LoadMore method (lines 382-386) which currently
ignores request.Sorts entirely, ensuring consistent sort semantics are applied
in both code paths.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cs`:
- Around line 629-630: The controller route definitions at lines 629, 736, 810,
and 904 use the attribute Route with pattern "[controller]/[action]" but the
client sample code expects routes prefixed with "api/". Update the Route
attribute in all four controller definitions (ProductsController and other
similar controllers) to include the "api/" prefix in the route pattern, changing
"[controller]/[action]" to "api/[controller]/[action]" to ensure consistency
between the server-side sample and the client-side usage.
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cs`:
- Around line 13-68: The Generate and GeneratePersian methods contain duplicate
code for the entire Product generation algorithm, differing only in which string
arrays they use. Create a private helper method that accepts three string array
parameters (for adjectives, nouns, and suppliers) and contains the common
generation logic currently duplicated in both methods. Then refactor the
Generate method to call this helper with Adjectives, Nouns, and Suppliers
arrays, and the GeneratePersian method to call it with PersianAdjectives,
PersianNouns, and PersianSuppliers arrays. This will eliminate duplication while
maintaining the same external behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ca5f25fb-53db-4b03-bab4-dc6a2e8df316
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs (1)
141-147: 🩺 Stability & Availability | 🟠 MajorMove the cancellable delay inside
tryso cleanup always remains in scope.At Line 143,
await Task.Delay(250, request.CancellationToken)can throw before thetryblock at Line 146, causing thefinallycleanup (Lines 198–209) to be bypassed and leavingserverLoadingin an inconsistent state.Proposed fix
private async Task<BitDataGridReadResult<Product>> LoadServerData(BitDataGridReadRequest request) { serverLoading = true; await InvokeAsync(StateHasChanged); - await Task.Delay(250, request.CancellationToken); int total = 0; try { + await Task.Delay(250, request.CancellationToken); + IEnumerable<Product> query = serverAll;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs` around lines 141 - 147, The cancellable Task.Delay on line 143 can throw a cancellation exception before entering the try block that starts on line 146, which causes the finally cleanup block to be bypassed and leaves serverLoading in an inconsistent state. Move the await Task.Delay(250, request.CancellationToken) statement inside the try block so that the finally cleanup block (lines 198-209) will always execute regardless of whether the delay is cancelled, ensuring serverLoading is properly reset.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs (1)
1226-1235: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQuote CSV fields containing carriage returns.
Line 1233 quotes
\nbut not\r, so CR-only values can split rows in spreadsheet parsers. Include\rin the quoting condition.Suggested fix
- return v.Contains(',') || v.Contains('"') || v.Contains('\n') + return v.Contains(',') || v.Contains('"') || v.Contains('\n') || v.Contains('\r') ? "\"" + v.Replace("\"", "\"\"") + "\"" : v;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 1226 - 1235, The Escape method checks for newline characters (\n) when deciding whether to quote CSV fields, but it does not include carriage return characters (\r) in the quoting condition. Update the return statement in the Escape method to also check for \r using an additional .Contains('\r') condition alongside the existing checks for comma, double quote, and newline, so that fields containing carriage returns are properly quoted to prevent row splitting in spreadsheet parsers.
🧹 Nitpick comments (3)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cs (1)
4-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd XML documentation to properties.
All properties lack documentation summaries, reducing IntelliSense discoverability.
📝 Proposed XML documentation additions
public sealed class BitDataGridFilterDescriptor { + /// <summary>Identifier of the column being filtered.</summary> public required string ColumnId { get; init; } + /// <summary>The filter operation to apply (e.g., Contains, Equals, GreaterThan).</summary> public BitDataGridFilterOperator Operator { get; set; } = BitDataGridFilterOperator.Contains; + /// <summary>The filter value to compare against, or null for operators like IsNull.</summary> public object? Value { get; set; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cs` around lines 4 - 9, The BitDataGridFilterDescriptor class properties (ColumnId, Operator, and Value) lack XML documentation comments, which prevents IntelliSense from displaying helpful information for developers using these properties. Add XML documentation summary comments above each property using the standard /// <summary> syntax to describe what each property represents and its purpose in the filter descriptor context.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cs (1)
4-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd XML documentation to properties for IntelliSense clarity.
All properties lack individual doc summaries. Even simple DTOs benefit from documentation that appears in IDE tooltips and API docs.
📝 Proposed XML documentation additions
public sealed class BitDataGridAggregateResult { + /// <summary>Identifier of the column this aggregate result applies to.</summary> public required string ColumnId { get; init; } + /// <summary>The type of aggregate function applied (sum, average, count, etc.).</summary> public BitDataGridAggregateType Type { get; init; } + /// <summary>The raw computed aggregate value, or null if computation produced no result.</summary> public object? Value { get; init; } + /// <summary>The formatted string representation of the aggregate value for display.</summary> public required string FormattedValue { get; init; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cs` around lines 4 - 10, The BitDataGridAggregateResult class and its properties are missing XML documentation comments that would provide IntelliSense clarity in IDEs and API documentation. Add XML documentation comments above each property (ColumnId, Type, Value, and FormattedValue) in the BitDataGridAggregateResult class using the summary tags to describe what each property represents, its purpose, and any relevant details about its usage or constraints.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cs (1)
10-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd XML documentation to undocumented properties for better IntelliSense discoverability.
Properties
ColumnId,KeyText,Aggregates, and the computed propertyHasSubGroupslack XML documentation summaries, which reduces discoverability in IntelliSense and impacts documentation generation. Lines 14 and 30 demonstrate the pattern already in use.📝 Proposed XML documentation additions
public sealed class BitDataGridGroup<TItem> { + /// <summary>Identifier of the column used for grouping.</summary> public required string ColumnId { get; init; } + /// <summary>The grouping key value for this group.</summary> public required object? Key { get; init; } + /// <summary>Human-readable text representation of the grouping key.</summary> public string KeyText { get; init; } = string.Empty; /// <summary>Zero-based nesting depth (0 = top level).</summary> public int Level { get; init; } /// <summary>Stable, unique path identifying this group across the whole tree (used for collapse state).</summary> public required string Path { get; init; } /// <summary>All rows that fall under this group (across nested subgroups).</summary> public List<TItem> Items { get; init; } = new(); /// <summary>Child groups when this group is further grouped; empty for leaf groups.</summary> public List<BitDataGridGroup<TItem>> SubGroups { get; init; } = new(); + /// <summary>Computed aggregates (e.g., sum, average) for columns in this group.</summary> public List<BitDataGridAggregateResult> Aggregates { get; init; } = new(); + /// <summary>Indicates whether this group contains child groups (subgroups).</summary> public bool HasSubGroups => SubGroups.Count > 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cs` around lines 10 - 26, The properties ColumnId, KeyText, and Aggregates in the BitDataGridGroup class are missing XML documentation summaries, which reduces IntelliSense discoverability. Add XML documentation comments above each of these properties following the same pattern already used for Level, Path, Items, and SubGroups properties. Each summary should provide a brief description of what the property represents in the context of data grid grouping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor`:
- Around line 124-141: The span element with the "bit-dtg-htext" class that
handles OnHeaderClick is not keyboard accessible because it lacks keyboard event
handling. Add a keyboard event handler (such as `@onkeydown`) to this span element
that triggers the sort action when Enter or Space keys are pressed. Create a new
event handler method (such as OnHeaderKeyDown) that checks if the pressed key is
Enter or Space and calls the appropriate sorting method to enable keyboard users
to activate column sorting.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 488-493: The catch block for OperationCanceledException is too
broad and swallows all operation cancellations, including unrelated ones or
provider-side timeouts. Instead of catching all OperationCanceledException
instances, you need to filter the catch block to only handle cancellations from
the grid's own request token. Add a condition inside the catch block or use a
filtered catch pattern to check if the cancellation exception's
CancellationToken property matches the expected grid request token (identify the
token field used for this load operation). If the cancellation is not from the
grid's own token, rethrow the exception so legitimate errors surface. Apply this
same filtering pattern to both occurrences mentioned (lines 488-493 and
583-588).
- Around line 433-448: In the ResetInfiniteAsync method, bump the load version
before awaiting the scrollToTop call to prevent race conditions where older
batches complete during the await and append stale rows to the freshly reset
list. The load version should be incremented immediately after clearing
_infiniteItems and before the try block that awaits
_infiniteHandle.InvokeVoidAsync("scrollToTop"), so that when
ResetLoadCancellation is called later in LoadNextBatchAsync, the version check
will correctly reject any stale batches that completed during the await window.
- Around line 280-289: The inputsChanged check in the BitDataGrid parameter
update logic only compares the Items reference using ReferenceEquals, which
fails to detect when a parent mutates the same collection instance in place
(adding/removing items without changing the reference). This leaves the cached
_view data stale in client-mode scenarios. Modify the inputsChanged condition to
include an additional check that forces a refresh in client-mode data scenarios
even when the Items reference remains the same, while keeping the existing
reference guard for server and infinite scroll modes. This ensures that
RefreshAsync is called when the actual collection contents may have changed.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cs`:
- Around line 1-9: Add XML documentation summaries for the three properties in
the BitDataGridFilterDescriptor class. For the ColumnId property, document that
it represents the identifier of the column being filtered and note that it is
immutable (required init property). For the Operator property, document its
purpose as specifying the filter operation and mention that it defaults to
BitDataGridFilterOperator.Contains. For the Value property, document that it
represents the filter value and note that it is nullable, explaining that its
meaning depends on the selected operator (e.g., not used for IsEmpty or
IsNotEmpty operators).
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`:
- Around line 388-391: The LoadMore method's Task.Delay call does not respect
the cancellation token provided in the BitDataGridReadRequest parameter. Update
the await Task.Delay(350) statement to pass request.CancellationToken as the
second parameter to Task.Delay so that the sample code correctly honors
cancellation requests, matching the actual implementation behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor`:
- Line 4: The `@using` directive on line 4 of BitQuickGridDemo.razor imports from
Demo.Shared.Dtos.QuickGridDemo, but this namespace does not exist. The actual
namespace is Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo as used in the
code-behind file BitQuickGridDemo.razor.cs. Update the `@using` statement to use
the full namespace Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo instead of the
abbreviated Demo.Shared.Dtos.QuickGridDemo to match the correct fully-qualified
namespace and allow the code to compile.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 1226-1235: The Escape method checks for newline characters (\n)
when deciding whether to quote CSV fields, but it does not include carriage
return characters (\r) in the quoting condition. Update the return statement in
the Escape method to also check for \r using an additional .Contains('\r')
condition alongside the existing checks for comma, double quote, and newline, so
that fields containing carriage returns are properly quoted to prevent row
splitting in spreadsheet parsers.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cs`:
- Around line 141-147: The cancellable Task.Delay on line 143 can throw a
cancellation exception before entering the try block that starts on line 146,
which causes the finally cleanup block to be bypassed and leaves serverLoading
in an inconsistent state. Move the await Task.Delay(250,
request.CancellationToken) statement inside the try block so that the finally
cleanup block (lines 198-209) will always execute regardless of whether the
delay is cancelled, ensuring serverLoading is properly reset.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cs`:
- Around line 10-26: The properties ColumnId, KeyText, and Aggregates in the
BitDataGridGroup class are missing XML documentation summaries, which reduces
IntelliSense discoverability. Add XML documentation comments above each of these
properties following the same pattern already used for Level, Path, Items, and
SubGroups properties. Each summary should provide a brief description of what
the property represents in the context of data grid grouping.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cs`:
- Around line 4-10: The BitDataGridAggregateResult class and its properties are
missing XML documentation comments that would provide IntelliSense clarity in
IDEs and API documentation. Add XML documentation comments above each property
(ColumnId, Type, Value, and FormattedValue) in the BitDataGridAggregateResult
class using the summary tags to describe what each property represents, its
purpose, and any relevant details about its usage or constraints.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cs`:
- Around line 4-9: The BitDataGridFilterDescriptor class properties (ColumnId,
Operator, and Value) lack XML documentation comments, which prevents
IntelliSense from displaying helpful information for developers using these
properties. Add XML documentation summary comments above each property using the
standard /// <summary> syntax to describe what each property represents and its
purpose in the filter descriptor context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5c2fdcdd-b19e-430a-8c6b-b05b04b60e5f
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cs (1)
35-57: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecompute cell formatting when
Formatchanges.At Line 35, formatter regeneration is gated only by
Propertyreference changes. IfFormatchanges whilePropertystays the same,_cellTextFuncis not rebuilt and the UI keeps old formatting.💡 Proposed fix
public class BitQuickGridPropertyColumn<TGridItem, TProp> : BitQuickGridColumnBase<TGridItem>, IBitQuickGridSortBuilderColumn<TGridItem> { private Expression<Func<TGridItem, TProp>>? _lastAssignedProperty; + private string? _lastAssignedFormat; private Func<TGridItem, string?>? _cellTextFunc; private BitQuickGridSort<TGridItem>? _sortBuilder; @@ - if (_lastAssignedProperty != Property) + var propertyChanged = _lastAssignedProperty != Property; + var formatChanged = _lastAssignedFormat != Format; + if (propertyChanged || formatChanged) { _lastAssignedProperty = Property; + _lastAssignedFormat = Format; var compiledPropertyExpression = Property.Compile(); @@ - _sortBuilder = BitQuickGridSort<TGridItem>.ByAscending(Property); + if (propertyChanged) + { + _sortBuilder = BitQuickGridSort<TGridItem>.ByAscending(Property); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cs` around lines 35 - 57, The cell formatting logic in BitQuickGridPropertyColumn only recomputes _cellTextFunc when the Property reference changes, but it does not account for changes to the Format parameter. To fix this, add a field to track the last assigned Format value (similar to _lastAssignedProperty), and modify the condition that gates the formatter regeneration to check both whether Property has changed AND whether Format has changed. When either Property or Format changes, the _cellTextFunc should be rebuilt to ensure the UI reflects the current formatting rules.
♻️ Duplicate comments (4)
src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss (1)
4-5: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the
.scsssuffixes here too.This is the same
scss/load-partial-extensionfailure already called out on this file, so the stylelint error is still unresolved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss` around lines 4 - 5, Remove the `.scss` file extensions from the two import statements for BitQuickGrid.scss and BitQuickGridPaginator.scss in the extra-components.scss file. The scss/load-partial-extension stylelint rule requires partial imports to omit the file extension, so change the imports to reference the paths without the `.scss` suffix while keeping the directory paths intact.Source: Linters/SAST tools
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scss (1)
12-12: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStylelint is currently failing on Blazor
::deepselectors in this file.Lines 12, 62, 89, and 122 trigger
selector-pseudo-element-no-unknown. Add rule suppression for Blazor-scoped CSS (locally or globally) so this stylesheet passes lint.🔧 Minimal file-local fix pattern
+/* stylelint-disable-next-line selector-pseudo-element-no-unknown */ ::deep {Also applies to: 62-62, 89-89, 122-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scss` at line 12, The stylelint rule `selector-pseudo-element-no-unknown` is failing because it does not recognize the Blazor-specific `::deep` pseudo-element selector used throughout the file at lines 12, 62, 89, and 122. Add stylelint-disable rule suppressions locally before each `::deep` selector block to allow this Blazor-scoped CSS syntax to pass linting, or add a file-level disable comment at the top of the stylesheet to suppress this rule globally for the entire file. Choose the approach that best fits your project's linting standards (local suppressions for targeted fixes or file-level suppression if `::deep` is used frequently in Blazor components).Source: Linters/SAST tools
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor (1)
4-4: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winUse the fully qualified QuickGrid DTO namespace import.
Line 4 imports
Demo.Shared.Dtos.QuickGridDemo, while the DTO namespace used in the demo code-behind isBit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo. This mismatch can break compilation.🔧 Proposed fix
-@using Demo.Shared.Dtos.QuickGridDemo +@using Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor` at line 4, The `@using` directive on line 4 imports an incomplete namespace path Demo.Shared.Dtos.QuickGridDemo, but the actual namespace in the demo code-behind is Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo. Update the `@using` statement to use the fully qualified namespace by changing Demo.Shared.Dtos.QuickGridDemo to Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo to ensure proper compilation and type resolution.src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs (1)
372-376: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFilter cancellation catches to superseded grid requests only.
Line 372 and Line 426 currently swallow all
OperationCanceledException. This can mask provider-side cancellations/timeouts unrelated to request supersession and silently return stale/empty results.Suggested fix
- catch (OperationCanceledException) + catch (OperationCanceledException) when (thisLoadCts.IsCancellationRequested) { // This load was superseded by a newer request; swallow the cancellation and fall through // to the cleanup below so the load-state remains consistent. } @@ - catch (OperationCanceledException) + catch (OperationCanceledException) when (request.CancellationToken.IsCancellationRequested) { // The request was superseded by a newer one after the debounce window; the items provider // observed the cancellation token and bailed out. Return an empty result the virtualization // system can handle rather than letting the cancellation propagate out of here. return default; }Also applies to: 426-432
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs` around lines 372 - 376, The catch blocks for OperationCanceledException in the BitQuickGrid class (at lines 372-376 and 426-432) are catching all cancellation exceptions indiscriminately, which masks provider-side cancellations and timeouts unrelated to request supersession. Filter the exception handling to only swallow OperationCanceledException when it is actually caused by a superseded request, such as by checking a cancellation token or request supersession flag before deciding to swallow the exception. Add a conditional check inside each catch block to verify the cancellation is due to request supersession, and either re-throw or handle the exception differently if the cancellation came from another source like a provider timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor`:
- Line 250: The BitDataGridRow rendering uses the item reference directly as the
Blazor key, but this should instead use the same GetKey method that's used for
selection, detail, and edit functionality. Replace `@key`="item" with
`@key`="GetKey(item)" in all row rendering paths including the main BitDataGridRow
at line 250, virtualized rows at line 272, grouped rows at line 279, and any
other row render paths at line 378 to ensure consistent identity tracking and
prevent unnecessary DOM recreation when refreshes return new instances with the
same key.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 925-938: The DropColumn method does not validate whether both the
dragged column and target column are reorderable before mutating the _columns
list. Add a guard condition after the existing validity checks that uses the
ColumnReorderable method to verify that both _dragColumn and target are
reorderable, and return early if either is not reorderable, preventing
non-reorderable columns from being involved in reordering operations.
- Around line 240-254: The AddColumn method only triggers a re-render via
StateHasChanged but does not recompute the data view needed for Aggregate
columns to calculate their values properly. After adding the column to the
_columns and _columnsById collections in the AddColumn method, call RefreshAsync
or an equivalent method that recomputes the data view to ensure aggregate/footer
calculations are updated, rather than just invoking StateHasChanged which only
performs re-rendering without data recomputation.
- Around line 1237-1246: The Escape method currently only checks if the first
character of a value is a formula-injection character (=, +, -, @), but
spreadsheet applications can bypass this protection by treating values with
leading whitespace followed by these characters as executable formulas. Modify
the formula-injection check to first trim leading whitespace (spaces, tabs,
newlines) from the value, then check if the trimmed result starts with one of
the dangerous characters. If a formula injection is detected after trimming
whitespace, prefix the original unmodified value (preserving its whitespace)
with a single quote to prevent execution.
- Around line 497-503: The catch block for OperationCanceledException is
checking if the exception's token matches the read cancellation token exactly,
but providers might throw OperationCanceledException without preserving the
original token object. Replace the when condition that checks
`ex.CancellationToken == read.CancellationToken` with
`read.CancellationToken.IsCancellationRequested` to verify whether the token was
actually canceled rather than requiring an exact token match. Apply this change
to both locations where this pattern appears in the catch handler for
OperationCanceledException.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razor`:
- Around line 20-22: Add the aria-expanded attribute to the toggle disclosure
buttons to properly communicate their expanded/collapsed state to assistive
technologies. In the button element within BitDataGridRow.razor that calls
Grid.ToggleDetail(Item), add an aria-expanded attribute that dynamically binds
to the result of Grid.IsDetailExpanded(Item), returning true when expanded and
false when collapsed. Apply this same fix to the second disclosure button
mentioned at line 92-96 that also needs the aria-expanded attribute.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cs`:
- Around line 35-57: The cell formatting logic in BitQuickGridPropertyColumn
only recomputes _cellTextFunc when the Property reference changes, but it does
not account for changes to the Format parameter. To fix this, add a field to
track the last assigned Format value (similar to _lastAssignedProperty), and
modify the condition that gates the formatter regeneration to check both whether
Property has changed AND whether Format has changed. When either Property or
Format changes, the _cellTextFunc should be rebuilt to ensure the UI reflects
the current formatting rules.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cs`:
- Around line 372-376: The catch blocks for OperationCanceledException in the
BitQuickGrid class (at lines 372-376 and 426-432) are catching all cancellation
exceptions indiscriminately, which masks provider-side cancellations and
timeouts unrelated to request supersession. Filter the exception handling to
only swallow OperationCanceledException when it is actually caused by a
superseded request, such as by checking a cancellation token or request
supersession flag before deciding to swallow the exception. Add a conditional
check inside each catch block to verify the cancellation is due to request
supersession, and either re-throw or handle the exception differently if the
cancellation came from another source like a provider timeout.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss`:
- Around line 4-5: Remove the `.scss` file extensions from the two import
statements for BitQuickGrid.scss and BitQuickGridPaginator.scss in the
extra-components.scss file. The scss/load-partial-extension stylelint rule
requires partial imports to omit the file extension, so change the imports to
reference the paths without the `.scss` suffix while keeping the directory paths
intact.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor`:
- Line 4: The `@using` directive on line 4 imports an incomplete namespace path
Demo.Shared.Dtos.QuickGridDemo, but the actual namespace in the demo code-behind
is Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo. Update the `@using` statement to
use the fully qualified namespace by changing Demo.Shared.Dtos.QuickGridDemo to
Bit.BlazorUI.Demo.Shared.Dtos.QuickGridDemo to ensure proper compilation and
type resolution.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scss`:
- Line 12: The stylelint rule `selector-pseudo-element-no-unknown` is failing
because it does not recognize the Blazor-specific `::deep` pseudo-element
selector used throughout the file at lines 12, 62, 89, and 122. Add
stylelint-disable rule suppressions locally before each `::deep` selector block
to allow this Blazor-scoped CSS syntax to pass linting, or add a file-level
disable comment at the top of the stylesheet to suppress this rule globally for
the entire file. Choose the approach that best fits your project's linting
standards (local suppressions for targeted fixes or file-level suppression if
`::deep` is used frequently in Blazor components).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 877ce7d9-1dcb-45b1-a0b2-897b2f95907a
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor`:
- Around line 27-30: The ToCsv() method is being called during every render in
the href attribute binding, which is inefficient for large grids and creates
oversized DOM attributes. Replace the current href data URI approach with an
onclick event handler that calls a method to generate and download the CSV data
on demand instead. Implement the download using JavaScript interop or the Blob
API to trigger the download when the user clicks the link, avoiding the need to
place the full CSV content in the DOM attribute and eliminating unnecessary
regeneration during unrelated state changes.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 580-609: The issue is that when the server returns a TotalCount
smaller than the current page range, the code sets _pageItems to the empty
result from the out-of-range request, then calls ClampPage() to adjust
_currentPage to a valid page without reloading data for that newly clamped page.
Fix this by checking if ClampPage() actually modified _currentPage, and if it
did, recursively call the load method or re-request the data with the updated
page number to ensure the UI displays data matching the clamped page.
- Around line 268-280: The issue is that when SelectionMode changes in the
BitDataGrid component, the code clears the internal _selected collection and
notifies the change, but it skips synchronizing the incoming SelectedItems
property that was provided by the parent in the same render cycle. To fix this,
after the mode-change branch that handles clearing _selected and calling
NotifySelectionAsync (when _lastSelectionMode is not null and differs from
SelectionMode), you need to also apply the incoming SelectedItems by adding the
same synchronization logic (clearing _selected and populating it from
SelectedItems) that currently exists only in the else-if branch. This ensures
that when a parent component changes both SelectionMode and provides
SelectedItems simultaneously, the internal selection state reflects the new
SelectedItems values instead of remaining empty.
- Around line 569-575: The ResetLoadCancellation() method is immediately
disposing the previous CancellationTokenSource after canceling it, but pending
OnRead or OnLoadMore operations may still be holding and using that token,
causing ObjectDisposedException instead of OperationCanceledException. Remove
the Dispose call on the old _loadCts so that canceled tokens remain valid for
in-flight operations to complete gracefully. You can either defer disposal of
the old CTS or rely on garbage collection to clean it up once all operations
complete. Only create and assign the new CancellationTokenSource without
disposing the previous one.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razor`:
- Around line 13-14: The `@onkeydown`:preventDefault="PreventKeyDefault" directive
on the BitDataGridCell component is preventing default behavior for all keys,
including Tab, which traps keyboard users and prevents them from navigating away
from the cell. Remove the :preventDefault modifier from the `@onkeydown` event
binding and instead implement key-specific prevention within the HandleKeyDown
method, ensuring that Tab key presses are allowed to propagate naturally while
only preventing default behavior for keys that are actually handled by the grid
navigation logic (such as arrow keys, Enter, or Escape). This fix should be
applied to all occurrences mentioned in the comment (lines 13-14, 32-35, and
55-77).
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razor`:
- Around line 12-15: The numeric editor in the BitDataGridCellEditor.razor
component uses GetString() which applies culture-sensitive formatting, causing
decimal values to render with commas in comma-decimal cultures while HTML number
inputs expect periods as decimal separators. Fix the Number case by replacing
the GetString() call in the value binding with a culture-invariant formatting
approach that explicitly uses invariant culture to ensure the decimal separator
is always a period, making the numeric input function correctly across all
locales.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cs`:
- Around line 112-114: The numeric type checking logic in the data-type
inference is incomplete and missing support for unsigned numeric primitives
(ushort, uint, ulong) and sbyte, causing these types to be incorrectly inferred
as Text. In the condition within the type-checking block, extend the existing
typeof checks for numeric types to also include typeof(sbyte), typeof(ushort),
typeof(uint), and typeof(ulong) alongside the already-checked types to ensure
all numeric primitives return BitDataGridColumnDataType.Number.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`:
- Around line 323-376: The LoadData method updates the loading state variable
but does not explicitly request a component re-render, causing the UI to display
stale values when this callback is invoked. Add explicit calls to
StateHasChanged() after setting loading to true (right after the assignment at
the start of the try block) and after setting loading to false (in the finally
block) to ensure the component re-renders when the loading state changes. Use
await InvokeAsync(StateHasChanged) to properly invoke the render on the correct
synchronization context within this async callback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 32ece994-452d-4965-ada7-05cf23e20214
📒 Files selected for processing (86)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridRow.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridDataProcessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridGroup.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridPropertyAccessor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Infrastructure/BitDataGridValueComparer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridAggregateType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridCellEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridColumnDataType.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridFilterOperator.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridGroupDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridPagerPosition.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridReadResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridRowReorderEventArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSelectionMode.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDescriptor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Models/BitDataGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGrid.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridRowTemplateArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/BitQuickGridSortDirection.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridAlign.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridColumnBase.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridPropertyColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridSort.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/BitQuickGridTemplateColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Columns/IBitQuickGridSortBuilderColumn.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/AsyncQueryExecutorSupplier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridColumnsCollectedNotifier.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/BitQuickGridDefer.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscribable.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventCallbackSubscriber.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/EventHandlers.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/IAsyncQueryExecutor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Infrastructure/InternalGridContext.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderRequest.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/ItemsProvider/BitQuickGridItemsProviderResult.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginationState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/QuickGrid/Pagination/BitQuickGridPaginator.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/AppJsonContext.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecall.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/FoodRecallQueryResult.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Meta.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Openfda.cssrc/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/QuickGridDemo/Results.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.params.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/FileSystemData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/Product.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SampleData.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/SupplierModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/BitQuickGridDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/CountryModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/QuickGrid/MedalsModel.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (6)
- src/BlazorUI/Demo/Bit.BlazorUI.Demo.Shared/Dtos/DataGridDemo/Openfda.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/IBitDataGridSortBuilderColumn.cs
- src/BlazorUI/Bit.BlazorUI/Utils/BitDirection.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridJsRuntimeExtensions.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/ItemsProvider/BitDataGridItemsProvider.cs
- src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/Columns/BitDataGridColumnBase.razor.cs
Comment on lines
+27
to
+30
| @if (ShowCsvExport) | ||
| { | ||
| <a class="bit-dtg-btn" download="export.csv" | ||
| href="data:text/csv;charset=utf-8,@Uri.EscapeDataString(ToCsv())">Export CSV</a> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid generating the CSV data URI during every render.
This href evaluates ToCsv() on each render, walking all export rows/cells and placing the full CSV in the DOM. On large grids, unrelated state changes can become expensive and produce oversized attributes; generate/download the CSV on click instead, preferably via a Blob/JS interop path or an invalidated cache.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor`
around lines 27 - 30, The ToCsv() method is being called during every render in
the href attribute binding, which is inefficient for large grids and creates
oversized DOM attributes. Replace the current href data URI approach with an
onclick event handler that calls a method to generate and download the CSV data
on demand instead. Implement the download using JavaScript interop or the Blob
API to trigger the download when the user clicks the link, avoiding the need to
place the full CSV content in the DOM attribute and eliminating unnecessary
regeneration during unrelated state changes.
Comment on lines
+268
to
+280
| if (_lastSelectionMode is not null && _lastSelectionMode != SelectionMode) | ||
| { | ||
| if (_selected.Count > 0) | ||
| { | ||
| _selected.Clear(); | ||
| await NotifySelectionAsync(); | ||
| } | ||
| } | ||
| else if (SelectedItems is not null) | ||
| { | ||
| _selected.Clear(); | ||
| foreach (var i in SelectedItems) _selected.Add(i); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply incoming SelectedItems after selection-mode changes.
The mode-change branch skips the SelectedItems sync in Lines 276-280, so a parent that changes SelectionMode and provides selected rows in the same render gets an empty/stale internal selection.
🐛 Proposed fix
- if (_lastSelectionMode is not null && _lastSelectionMode != SelectionMode)
+ var selectionModeChanged = _lastSelectionMode is not null && _lastSelectionMode != SelectionMode;
+ var hadSelection = _selected.Count > 0;
+
+ if (selectionModeChanged)
{
- if (_selected.Count > 0)
- {
- _selected.Clear();
- await NotifySelectionAsync();
- }
+ _selected.Clear();
}
- else if (SelectedItems is not null)
+
+ if (SelectedItems is not null)
{
_selected.Clear();
- foreach (var i in SelectedItems) _selected.Add(i);
+ var incoming = SelectionMode switch
+ {
+ BitDataGridSelectionMode.None => Enumerable.Empty<TItem>(),
+ BitDataGridSelectionMode.Single => SelectedItems.Take(1),
+ _ => SelectedItems
+ };
+ foreach (var i in incoming) _selected.Add(i);
+ }
+ else if (selectionModeChanged && hadSelection)
+ {
+ await NotifySelectionAsync();
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`
around lines 268 - 280, The issue is that when SelectionMode changes in the
BitDataGrid component, the code clears the internal _selected collection and
notifies the change, but it skips synchronizing the incoming SelectedItems
property that was provided by the parent in the same render cycle. To fix this,
after the mode-change branch that handles clearing _selected and calling
NotifySelectionAsync (when _lastSelectionMode is not null and differs from
SelectionMode), you need to also apply the incoming SelectedItems by adding the
same synchronization logic (clearing _selected and populating it from
SelectedItems) that currently exists only in the else-if branch. This ensures
that when a parent component changes both SelectionMode and provides
SelectedItems simultaneously, the internal selection state reflects the new
SelectedItems values instead of remaining empty.
Comment on lines
+569
to
+575
| private CancellationToken ResetLoadCancellation() | ||
| { | ||
| _loadCts?.Cancel(); | ||
| _loadCts?.Dispose(); | ||
| _loadCts = new CancellationTokenSource(); | ||
| _loadVersion++; | ||
| return _loadCts.Token; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect cancellation-source lifetime around BitDataGrid data loads.
# Expected: ResetLoadCancellation cancels superseded work but does not dispose a CTS that older providers may still be using.
rg -n -C 4 '\bResetLoadCancellation\b|_loadCts\?\.(Cancel|Dispose)' src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.csRepository: bitfoundation/bitplatform
Length of output: 2002
🏁 Script executed:
cd /tmp && git clone https://github.com/bitfoundation/bitplatform.git --depth 1 2>&1 | head -20 && echo "Clone status: $?"Repository: bitfoundation/bitplatform
Length of output: 1972
🏁 Script executed:
# If clone fails, try to work with alternative approach
# Look for the file in common locations or search for information about the issue
find . -name "BitDataGrid.razor.cs" 2>/dev/null | head -5Repository: bitfoundation/bitplatform
Length of output: 1972
🌐 Web query:
CancellationTokenSource disposal pattern C# async cancellation token reuse providers
💡 Result:
The CancellationTokenSource (CTS) class in.NET implements IDisposable and is designed to manage the cooperative cancellation lifecycle [1][2]. Disposal Pattern You should always call Dispose on a CancellationTokenSource when you are finished using it to free any unmanaged resources it holds [3][1][2]. The standard pattern is to use a using statement or a try/finally block [2]. Key rules for disposal: 1. When to Dispose: Dispose the CTS only when you are certain that no further operations will attempt to use its associated CancellationToken [4][5]. Calling Dispose while a task is still actively using the token or waiting on a callback can result in an ObjectDisposedException or, in specific scenarios, a deadlock if a callback is blocked waiting on a lock held by the thread performing the disposal [1][5][6]. 2. Scope: If you create a CTS within a method, wrap it in a using statement [2][7]. If it is a class-level field, ensure it is disposed of when the owner object is disposed [5]. 3. Linked Tokens: If you use CancellationTokenSource.CreateLinkedTokenSource, remember that the resulting CTS must also be explicitly disposed of, as it maintains registrations on the source tokens [1][7]. Reusability A CancellationTokenSource cannot be effectively reused after it has been cancelled [8]. Once Cancel is called, the IsCancellationRequested property remains true, and any new operations using that token will immediately perceive a cancellation request [8]. TryReset In modern.NET versions, the CancellationTokenSource.TryReset method allows for the reuse of a CTS instance, but it is highly restrictive [9][10]. It is only intended for scenarios where you can guarantee the following [9]: - The operation previously associated with the CTS has fully completed [9]. - No other components hold references that might attempt to cancel it concurrently [9]. - No further registrations exist [9]. Because of these stringent requirements, the common and recommended practice is to dispose of the old CTS and instantiate a new one when a fresh cancellation scope is needed, rather than attempting to reset it [8][10][6].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/standard/threading/cancellation-in-managed-threads
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtokensource?view=net-8.0
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtokensource.dispose?view=net-10.0
- 4: https://stackoverflow.com/questions/61359443/correct-pattern-to-dispose-of-cancellation-token-source
- 5: https://stackoverflow.com/questions/6960520/when-to-dispose-cancellationtokensource
- 6: https://stackoverflow.com/questions/54301013/are-there-any-issues-with-assigning-a-new-cancellationtokensource-to-a-variab
- 7: https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/coalesce-cancellation-tokens-from-timeouts
- 8: https://stackoverflow.com/questions/71358689/being-able-to-reuse-same-cancellationtokensource-cancellationtokensource-creat
- 9: https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtokensource.tryreset?view=net-9.0
- 10: https://stackoverflow.com/questions/71042494/what-is-the-use-case-for-cancellationtokensource-tryreset
Do not dispose the cancellation source immediately after cancellation; providers may still use the token.
ResetLoadCancellation() cancels and immediately disposes the previous CancellationTokenSource. A superseded OnRead or OnLoadMore provider can still be using that token after cancellation, resulting in ObjectDisposedException instead of the expected OperationCanceledException.
The old CTS should be canceled but not disposed, since there may be pending operations still holding or using its token. Only the new CTS created in the next reset needs eventual disposal.
Proposed fix
private CancellationToken ResetLoadCancellation()
{
_loadCts?.Cancel();
- _loadCts?.Dispose();
_loadCts = new CancellationTokenSource();
_loadVersion++;
return _loadCts.Token;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`
around lines 569 - 575, The ResetLoadCancellation() method is immediately
disposing the previous CancellationTokenSource after canceling it, but pending
OnRead or OnLoadMore operations may still be holding and using that token,
causing ObjectDisposedException instead of OperationCanceledException. Remove
the Dispose call on the old _loadCts so that canceled tokens remain valid for
in-flight operations to complete gracefully. You can either defer disposal of
the old CTS or rely on garbage collection to clean it up once all operations
complete. Only create and assign the new CancellationTokenSource without
disposing the previous one.
Comment on lines
+580
to
+609
| var request = new BitDataGridReadRequest | ||
| { | ||
| Skip = Pageable ? (_currentPage - 1) * _effectivePageSize : 0, | ||
| Take = Pageable ? _effectivePageSize : null, | ||
| Sorts = _sorts.Where(s => s.Direction != BitDataGridSortDirection.None).OrderBy(s => s.Priority).ToList(), | ||
| Filters = _filters.ToList(), | ||
| CancellationToken = ResetLoadCancellation() | ||
| }; | ||
| // Capture this request's version right after ResetLoadCancellation; bail out below if a newer | ||
| // request has since superseded it so a stale response can't overwrite fresher state. | ||
| var version = _loadVersion; | ||
| BitDataGridReadResult<TItem> result; | ||
| try | ||
| { | ||
| result = await OnRead!(request); | ||
| } | ||
| catch (OperationCanceledException) when (request.CancellationToken.IsCancellationRequested) | ||
| { | ||
| // Superseded by a newer request whose cancellation token fired; cancellation from our own | ||
| // token is expected, so keep the existing state and let the newer load complete. Any other | ||
| // cancellation (e.g. a provider-side timeout) is a real error and propagates. | ||
| return; | ||
| } | ||
| if (version != _loadVersion) return; | ||
| _pageItems = result.Items; | ||
| _view = result.Items; | ||
| _totalCount = result.TotalCount; | ||
| _footerAggregates = BitDataGridDataProcessor.Aggregate(_pageItems, _columns); | ||
| _viewGroups = null; | ||
| ClampPage(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Re-read after clamping an out-of-range server page.
If server TotalCount shrinks while _currentPage is beyond the new last page, this applies the empty result for the old Skip, then changes the UI to the clamped page without loading that page’s data.
🐛 Proposed fix
private async Task LoadServerDataAsync()
{
+ var requestedPage = _currentPage;
var request = new BitDataGridReadRequest
{
- Skip = Pageable ? (_currentPage - 1) * _effectivePageSize : 0,
+ Skip = Pageable ? (requestedPage - 1) * _effectivePageSize : 0,
Take = Pageable ? _effectivePageSize : null,
Sorts = _sorts.Where(s => s.Direction != BitDataGridSortDirection.None).OrderBy(s => s.Priority).ToList(),
Filters = _filters.ToList(),
@@
}
if (version != _loadVersion) return;
+ _totalCount = result.TotalCount;
+ ClampPage();
+ if (Pageable && _currentPage != requestedPage)
+ {
+ await LoadServerDataAsync();
+ return;
+ }
_pageItems = result.Items;
_view = result.Items;
- _totalCount = result.TotalCount;
_footerAggregates = BitDataGridDataProcessor.Aggregate(_pageItems, _columns);
_viewGroups = null;
- ClampPage();
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`
around lines 580 - 609, The issue is that when the server returns a TotalCount
smaller than the current page range, the code sets _pageItems to the empty
result from the out-of-range request, then calls ClampPage() to adjust
_currentPage to a valid page without reloading data for that newly clamped page.
Fix this by checking if ClampPage() actually modified _currentPage, and if it
did, recursively call the load method or re-request the data with the updated
page number to ensure the UI displays data matching the clamped page.
Comment on lines
+13
to
+14
| @onkeydown="HandleKeyDown" | ||
| @onkeydown:preventDefault="PreventKeyDefault"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not suppress Tab for every cell keydown.
PreventKeyDefault applies to all keys in cell-navigation mode, but Tab is not handled by the grid key path. This can trap keyboard users on the focused cell; remove the broad modifier or replace it with key-specific prevention that preserves native Tab behavior.
🐛 Minimal safe fix
`@oncontextmenu`="HandleContextMenu"
`@oncontextmenu`:preventDefault="Grid.OnCellContextMenu.HasDelegate"
`@onfocusin`="HandleFocusIn"
- `@onkeydown`="HandleKeyDown"
- `@onkeydown`:preventDefault="PreventKeyDefault">
+ `@onkeydown`="HandleKeyDown">
@@
- // Suppress native browser key handling (e.g. scrolling on arrows/Page keys) only while cell
- // navigation is active and the cell is not being inline-edited, so editor inputs keep their
- // default typing/caret behavior.
- private bool PreventKeyDefault => Grid.CellNavigation && !Editing;
-Also applies to: 32-35, 55-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCell.razor`
around lines 13 - 14, The `@onkeydown`:preventDefault="PreventKeyDefault"
directive on the BitDataGridCell component is preventing default behavior for
all keys, including Tab, which traps keyboard users and prevents them from
navigating away from the cell. Remove the :preventDefault modifier from the
`@onkeydown` event binding and instead implement key-specific prevention within
the HandleKeyDown method, ensuring that Tab key presses are allowed to propagate
naturally while only preventing default behavior for keys that are actually
handled by the grid navigation logic (such as arrow keys, Enter, or Escape).
This fix should be applied to all occurrences mentioned in the comment (lines
13-14, 32-35, and 55-77).
Comment on lines
+12
to
+15
| case BitDataGridColumnDataType.Number: | ||
| <input type="number" class="bit-dtg-editor" step="any" | ||
| value="@(GetString())" | ||
| @oninput="e => Set(e.Value)" /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Format numeric editor values with invariant culture.
Value?.ToString() is culture-sensitive, so decimals render like 1,5 in comma-decimal cultures; <input type="number"> expects 1.5 and may show a blank value.
🐛 Proposed fix
case BitDataGridColumnDataType.Number:
<input type="number" class="bit-dtg-editor" step="any"
- value="@(GetString())"
+ value="@(GetNumber())"
`@oninput`="e => Set(e.Value)" />
break;
@@
private object? Value => Column.GetValue(Item);
private string GetString() => Value?.ToString() ?? string.Empty;
+ private string GetNumber() => Value is IFormattable f
+ ? f.ToString(null, System.Globalization.CultureInfo.InvariantCulture)
+ : GetString();
private bool GetBool() => Value is bool b && b;Also applies to: 45-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridCellEditor.razor`
around lines 12 - 15, The numeric editor in the BitDataGridCellEditor.razor
component uses GetString() which applies culture-sensitive formatting, causing
decimal values to render with commas in comma-decimal cultures while HTML number
inputs expect periods as decimal separators. Fix the Number case by replacing
the GetString() call in the value binding with a culture-invariant formatting
approach that explicitly uses invariant culture to ensure the decimal separator
is always a period, making the numeric input function correctly across all
locales.
Comment on lines
+112
to
+114
| if (t == typeof(int) || t == typeof(long) || t == typeof(short) || t == typeof(byte) | ||
| || t == typeof(double) || t == typeof(float) || t == typeof(decimal)) | ||
| return BitDataGridColumnDataType.Number; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include unsigned numeric primitives in auto data-type inference.
EffectiveDataType currently misses sbyte/ushort/uint/ulong, so these columns are inferred as Text, which can break number-specific UI/behavior.
💡 Suggested fix
- if (t == typeof(int) || t == typeof(long) || t == typeof(short) || t == typeof(byte)
- || t == typeof(double) || t == typeof(float) || t == typeof(decimal))
+ if (t == typeof(byte) || t == typeof(sbyte)
+ || t == typeof(short) || t == typeof(ushort)
+ || t == typeof(int) || t == typeof(uint)
+ || t == typeof(long) || t == typeof(ulong)
+ || t == typeof(float) || t == typeof(double) || t == typeof(decimal))
return BitDataGridColumnDataType.Number;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (t == typeof(int) || t == typeof(long) || t == typeof(short) || t == typeof(byte) | |
| || t == typeof(double) || t == typeof(float) || t == typeof(decimal)) | |
| return BitDataGridColumnDataType.Number; | |
| if (t == typeof(byte) || t == typeof(sbyte) | |
| || t == typeof(short) || t == typeof(ushort) | |
| || t == typeof(int) || t == typeof(uint) | |
| || t == typeof(long) || t == typeof(ulong) | |
| || t == typeof(float) || t == typeof(double) || t == typeof(decimal)) | |
| return BitDataGridColumnDataType.Number; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGridColumn.cs`
around lines 112 - 114, The numeric type checking logic in the data-type
inference is incomplete and missing support for unsigned numeric primitives
(ushort, uint, ulong) and sbyte, causing these types to be incorrectly inferred
as Text. In the condition within the type-checking block, extend the existing
typeof checks for numeric types to also include typeof(sbyte), typeof(ushort),
typeof(uint), and typeof(ulong) alongside the already-checked types to ensure
all numeric primitives return BitDataGridColumnDataType.Number.
Comment on lines
+323
to
+376
| private async Task<BitDataGridReadResult<Product>> LoadData(BitDataGridReadRequest request) | ||
| { | ||
| loading = true; | ||
| try | ||
| { | ||
| await Task.Delay(250, request.CancellationToken); // simulate a backend round-trip | ||
|
|
||
| } | ||
| } | ||
|
|
||
|
|
||
| // ========== Client code ========== | ||
|
|
||
| IEnumerable<Product> query = all; | ||
|
|
||
| BitDataGrid<ProductDto>? productsDataGrid; | ||
| BitDataGridItemsProvider<ProductDto> productsItemsProvider; | ||
| BitDataGridPaginationState pagination = new() { ItemsPerPage = 7 }; | ||
|
|
||
| protected override async Task OnInitializedAsync() | ||
| { | ||
| productsItemsProvider = async req => | ||
| { | ||
| try | ||
| // filtering | ||
| foreach (var f in request.Filters) | ||
| { | ||
| // https://docs.microsoft.com/en-us/odata/concepts/queryoptions-overview | ||
|
|
||
| var query = new Dictionary<string, object>() | ||
| var term = f.Value?.ToString() ?? """"; | ||
| query = f.ColumnId switch | ||
| { | ||
| { ""$top"", req.Count ?? 50 }, | ||
| { ""$skip"", req.StartIndex } | ||
| nameof(Product.Name) => query.Where(p => p.Name.Contains(term, StringComparison.OrdinalIgnoreCase)), | ||
| nameof(Product.Price) => query.Where(p => p.Price.ToString().Contains(term)), | ||
| nameof(Product.Id) => query.Where(p => p.Id.ToString().Contains(term)), | ||
| _ => query | ||
| }; | ||
|
|
||
| if (req.GetSortByProperties().Any()) | ||
| { | ||
| query.Add(""$orderby"", string.Join("", "", req.GetSortByProperties().Select(p => $""{p.PropertyName} {(p.Direction == BitDataGridSortDirection.Ascending ? ""asc"" : ""desc"")}""))); | ||
| } | ||
|
|
||
| var url = NavManager.GetUriWithQueryParameters(""api/Products/GetProducts"", query); | ||
|
|
||
| var data = await HttpClient.GetFromJsonAsync(url, AppJsonContext.Default.PagedResultProductDto); | ||
|
|
||
| return BitDataGridItemsProviderResult.From(data!.Items, (int)data!.TotalCount); | ||
| } | ||
| catch | ||
| { | ||
| return BitDataGridItemsProviderResult.From<ProductDto>(new List<ProductDto> { }, 0); | ||
| } | ||
| }; | ||
| }"; | ||
|
|
||
| private readonly string example6RazorCode = @" | ||
| <style> | ||
| .responsive-grid table { | ||
| border-collapse: collapse; | ||
| } | ||
|
|
||
| .responsive-grid tr { | ||
| padding: 1rem; | ||
| margin-bottom: 1rem; | ||
| border: 1px solid var(--bit-clr-brd-sec); | ||
| } | ||
|
|
||
| @media (max-width: 600px) { | ||
| .responsive-grid table { | ||
| width: 100%; | ||
| } | ||
|
|
||
| .responsive-grid table, | ||
| .responsive-grid thead, | ||
| .responsive-grid tbody, | ||
| .responsive-grid th, | ||
| .responsive-grid td, | ||
| .responsive-grid tr, | ||
| .responsive-grid td { | ||
| display: block; | ||
| // sorting (honor every active sort descriptor, not just the first) | ||
| IOrderedEnumerable<Product>? ordered = null; | ||
| foreach (var sort in request.Sorts) | ||
| { | ||
| Func<Product, object> key = sort.ColumnId switch | ||
| { | ||
| nameof(Product.Name) => p => p.Name, | ||
| nameof(Product.Price) => p => p.Price, | ||
| _ => p => p.Id | ||
| }; | ||
| if (ordered is null) | ||
| ordered = sort.Direction == BitDataGridSortDirection.Descending | ||
| ? query.OrderByDescending(key) | ||
| : query.OrderBy(key); | ||
| else | ||
| ordered = sort.Direction == BitDataGridSortDirection.Descending | ||
| ? ordered.ThenByDescending(key) | ||
| : ordered.ThenBy(key); | ||
| } | ||
| if (ordered is not null) query = ordered; | ||
|
|
||
| .responsive-grid thead tr { | ||
| display: none; | ||
| } | ||
| // paging | ||
| var filtered = query.ToList(); | ||
| var items = filtered.Skip(request.Skip).Take(request.Take ?? filtered.Count).ToList(); | ||
|
|
||
| .responsive-grid td::before { | ||
| font-weight: bold; | ||
| content: attr(data-title) "" : ""; | ||
| } | ||
| return new BitDataGridReadResult<Product>(items, filtered.Count); | ||
| } | ||
| </style> | ||
|
|
||
| <div class=""responsive-grid""> | ||
| <BitDataGrid Items=""@allCountries"" Pagination=""@pagination""> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Name)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Gold)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Silver)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Bronze)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Total)"" /> | ||
| </BitDataGrid> | ||
| <BitDataGridPaginator Value=""@pagination"" /> | ||
| </div>"; | ||
| private readonly string example6CsharpCode = @" | ||
| private IQueryable<CountryModel> allCountries; | ||
| private BitDataGridPaginationState pagination = new() { ItemsPerPage = 7 }; | ||
|
|
||
| protected override async Task OnInitializedAsync() | ||
| { | ||
| allCountries = _countries.AsQueryable(); | ||
| } | ||
|
|
||
| private static readonly CountryModel[] _countries = | ||
| [ | ||
| new CountryModel { Code = ""AR"", Name = ""Argentina"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 2 } }, | ||
| new CountryModel { Code = ""AM"", Name = ""Armenia"", Medals = new MedalsModel { Gold = 0, Silver = 2, Bronze = 2 } }, | ||
| new CountryModel { Code = ""AU"", Name = ""Australia"", Medals = new MedalsModel { Gold = 17, Silver = 7, Bronze = 22 } }, | ||
| new CountryModel { Code = ""AT"", Name = ""Austria"", Medals = new MedalsModel { Gold = 1, Silver = 1, Bronze = 5 } }, | ||
| new CountryModel { Code = ""AZ"", Name = ""Azerbaijan"", Medals = new MedalsModel { Gold = 0, Silver = 3, Bronze = 4 } }, | ||
| new CountryModel { Code = ""BS"", Name = ""Bahamas"", Medals = new MedalsModel { Gold = 2, Silver = 0, Bronze = 0 } }, | ||
| new CountryModel { Code = ""BH"", Name = ""Bahrain"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""BY"", Name = ""Belarus"", Medals = new MedalsModel { Gold = 1, Silver = 3, Bronze = 3 } }, | ||
| new CountryModel { Code = ""BE"", Name = ""Belgium"", Medals = new MedalsModel { Gold = 3, Silver = 1, Bronze = 3 } }, | ||
| new CountryModel { Code = ""BM"", Name = ""Bermuda"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 0 } }, | ||
| new CountryModel { Code = ""BW"", Name = ""Botswana"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""BR"", Name = ""Brazil"", Medals = new MedalsModel { Gold = 7, Silver = 6, Bronze = 8 } }, | ||
| new CountryModel { Code = ""BF"", Name = ""Burkina Faso"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""CA"", Name = ""Canada"", Medals = new MedalsModel { Gold = 7, Silver = 6, Bronze = 11 } }, | ||
| new CountryModel { Code = ""TW"", Name = ""Chinese Taipei"", Medals = new MedalsModel { Gold = 2, Silver = 4, Bronze = 6 } }, | ||
| new CountryModel { Code = ""CO"", Name = ""Colombia"", Medals = new MedalsModel { Gold = 0, Silver = 4, Bronze = 1 } }, | ||
| new CountryModel { Code = ""CI"", Name = ""Côte d'Ivoire"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""HR"", Name = ""Croatia"", Medals = new MedalsModel { Gold = 3, Silver = 3, Bronze = 2 } }, | ||
| new CountryModel { Code = ""CU"", Name = ""Cuba"", Medals = new MedalsModel { Gold = 7, Silver = 3, Bronze = 5 } }, | ||
| new CountryModel { Code = ""CZ"", Name = ""Czech Republic"", Medals = new MedalsModel { Gold = 4, Silver = 4, Bronze = 3 } }, | ||
| new CountryModel { Code = ""DK"", Name = ""Denmark"", Medals = new MedalsModel { Gold = 3, Silver = 4, Bronze = 4 } }, | ||
| new CountryModel { Code = ""DO"", Name = ""Dominican Republic"", Medals = new MedalsModel { Gold = 0, Silver = 3, Bronze = 2 } }, | ||
| new CountryModel { Code = ""EC"", Name = ""Ecuador"", Medals = new MedalsModel { Gold = 2, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""EE"", Name = ""Estonia"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""ET"", Name = ""Ethiopia"", Medals = new MedalsModel { Gold = 1, Silver = 1, Bronze = 2 } }, | ||
| new CountryModel { Code = ""FJ"", Name = ""Fiji"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""FI"", Name = ""Finland"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 2 } }, | ||
| new CountryModel { Code = ""FR"", Name = ""France"", Medals = new MedalsModel { Gold = 10, Silver = 12, Bronze = 11 } }, | ||
| new CountryModel { Code = ""GE"", Name = ""Georgia"", Medals = new MedalsModel { Gold = 2, Silver = 5, Bronze = 1 } }, | ||
| new CountryModel { Code = ""DE"", Name = ""Germany"", Medals = new MedalsModel { Gold = 10, Silver = 11, Bronze = 16 } }, | ||
| new CountryModel { Code = ""GH"", Name = ""Ghana"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""GB"", Name = ""Great Britain"", Medals = new MedalsModel { Gold = 22, Silver = 21, Bronze = 22 } }, | ||
| new CountryModel { Code = ""GR"", Name = ""Greece"", Medals = new MedalsModel { Gold = 2, Silver = 1, Bronze = 1 } }, | ||
| new CountryModel { Code = ""GD"", Name = ""Grenada"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""HK"", Name = ""Hong Kong, China"", Medals = new MedalsModel { Gold = 1, Silver = 2, Bronze = 3 } }, | ||
| new CountryModel { Code = ""HU"", Name = ""Hungary"", Medals = new MedalsModel { Gold = 6, Silver = 7, Bronze = 7 } }, | ||
| new CountryModel { Code = ""ID"", Name = ""Indonesia"", Medals = new MedalsModel { Gold = 1, Silver = 1, Bronze = 3 } }, | ||
| new CountryModel { Code = ""IE"", Name = ""Ireland"", Medals = new MedalsModel { Gold = 2, Silver = 0, Bronze = 2 } }, | ||
| new CountryModel { Code = ""IR"", Name = ""Iran"", Medals = new MedalsModel { Gold = 3, Silver = 2, Bronze = 2 } }, | ||
| new CountryModel { Code = ""IL"", Name = ""Israel"", Medals = new MedalsModel { Gold = 2, Silver = 0, Bronze = 2 } }, | ||
| new CountryModel { Code = ""IT"", Name = ""Italy"", Medals = new MedalsModel { Gold = 10, Silver = 10, Bronze = 20 } }, | ||
| new CountryModel { Code = ""JM"", Name = ""Jamaica"", Medals = new MedalsModel { Gold = 4, Silver = 1, Bronze = 4 } }, | ||
| new CountryModel { Code = ""JO"", Name = ""Jordan"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 1 } }, | ||
| new CountryModel { Code = ""KZ"", Name = ""Kazakhstan"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 8 } }, | ||
| new CountryModel { Code = ""KE"", Name = ""Kenya"", Medals = new MedalsModel { Gold = 4, Silver = 4, Bronze = 2 } }, | ||
| new CountryModel { Code = ""XK"", Name = ""Kosovo"", Medals = new MedalsModel { Gold = 2, Silver = 0, Bronze = 0 } }, | ||
| new CountryModel { Code = ""KW"", Name = ""Kuwait"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""LV"", Name = ""Latvia"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""LT"", Name = ""Lithuania"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""MY"", Name = ""Malaysia"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 1 } }, | ||
| new CountryModel { Code = ""MX"", Name = ""Mexico"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 4 } }, | ||
| new CountryModel { Code = ""MA"", Name = ""Morocco"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 0 } }, | ||
| new CountryModel { Code = ""NA"", Name = ""Namibia"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""NL"", Name = ""Netherlands"", Medals = new MedalsModel { Gold = 10, Silver = 12, Bronze = 14 } }, | ||
| new CountryModel { Code = ""NZ"", Name = ""New Zealand"", Medals = new MedalsModel { Gold = 7, Silver = 6, Bronze = 7 } }, | ||
| new CountryModel { Code = ""MK"", Name = ""North Macedonia"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""NO"", Name = ""Norway"", Medals = new MedalsModel { Gold = 4, Silver = 2, Bronze = 2 } }, | ||
| new CountryModel { Code = ""PH"", Name = ""Philippines"", Medals = new MedalsModel { Gold = 1, Silver = 2, Bronze = 1 } }, | ||
| new CountryModel { Code = ""PL"", Name = ""Poland"", Medals = new MedalsModel { Gold = 4, Silver = 5, Bronze = 5 } }, | ||
| new CountryModel { Code = ""PT"", Name = ""Portugal"", Medals = new MedalsModel { Gold = 1, Silver = 1, Bronze = 2 } }, | ||
| new CountryModel { Code = ""PR"", Name = ""Puerto Rico"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 0 } }, | ||
| new CountryModel { Code = ""QA"", Name = ""Qatar"", Medals = new MedalsModel { Gold = 2, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""KR"", Name = ""Republic of Korea"", Medals = new MedalsModel { Gold = 6, Silver = 4, Bronze = 10 } }, | ||
| new CountryModel { Code = ""MD"", Name = ""Republic of Moldova"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""RO"", Name = ""Romania"", Medals = new MedalsModel { Gold = 1, Silver = 3, Bronze = 0 } }, | ||
| new CountryModel { Code = ""SM"", Name = ""San Marino"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 2 } }, | ||
| new CountryModel { Code = ""SA"", Name = ""Saudi Arabia"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""RS"", Name = ""Serbia"", Medals = new MedalsModel { Gold = 3, Silver = 1, Bronze = 5 } }, | ||
| new CountryModel { Code = ""SK"", Name = ""Slovakia"", Medals = new MedalsModel { Gold = 1, Silver = 2, Bronze = 1 } }, | ||
| new CountryModel { Code = ""SI"", Name = ""Slovenia"", Medals = new MedalsModel { Gold = 3, Silver = 1, Bronze = 1 } }, | ||
| new CountryModel { Code = ""ZA"", Name = ""South Africa"", Medals = new MedalsModel { Gold = 1, Silver = 2, Bronze = 0 } }, | ||
| new CountryModel { Code = ""ES"", Name = ""Spain"", Medals = new MedalsModel { Gold = 3, Silver = 8, Bronze = 6 } }, | ||
| new CountryModel { Code = ""SE"", Name = ""Sweden"", Medals = new MedalsModel { Gold = 3, Silver = 6, Bronze = 0 } }, | ||
| new CountryModel { Code = ""CH"", Name = ""Switzerland"", Medals = new MedalsModel { Gold = 3, Silver = 4, Bronze = 6 } }, | ||
| new CountryModel { Code = ""SY"", Name = ""Syrian Arab Republic"", Medals = new MedalsModel { Gold = 0, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""TH"", Name = ""Thailand"", Medals = new MedalsModel { Gold = 1, Silver = 0, Bronze = 1 } }, | ||
| new CountryModel { Code = ""TR"", Name = ""Turkey"", Medals = new MedalsModel { Gold = 2, Silver = 2, Bronze = 9 } }, | ||
| new CountryModel { Code = ""TM"", Name = ""Turkmenistan"", Medals = new MedalsModel { Gold = 0, Silver = 1, Bronze = 0 } }, | ||
| new CountryModel { Code = ""UA"", Name = ""Ukraine"", Medals = new MedalsModel { Gold = 1, Silver = 6, Bronze = 12 } }, | ||
| new CountryModel { Code = ""US"", Name = ""United States of America"", Medals = new MedalsModel { Gold = 39, Silver = 41, Bronze = 33 } }, | ||
| new CountryModel { Code = ""UZ"", Name = ""Uzbekistan"", Medals = new MedalsModel { Gold = 3, Silver = 0, Bronze = 2 } }, | ||
| new CountryModel { Code = ""VE"", Name = ""Venezuela"", Medals = new MedalsModel { Gold = 1, Silver = 3, Bronze = 0 } }, | ||
| ]; | ||
|
|
||
| public class CountryModel | ||
| { | ||
| public string Code { get; set; } | ||
| public string Name { get; set; } | ||
| public MedalsModel Medals { get; set; } | ||
| } | ||
|
|
||
| public class MedalsModel | ||
| { | ||
| public int Gold { get; set; } | ||
| public int Silver { get; set; } | ||
| public int Bronze { get; set; } | ||
| public int Total => Gold + Silver + Bronze; | ||
| }"; | ||
|
|
||
| private readonly string example7RazorCode = @" | ||
| <style> | ||
| .row-template-grid .row-template-expand-col { | ||
| width: 2rem; | ||
| } | ||
|
|
||
| .row-template-grid .row-template-detail { | ||
| padding: 0.5rem; | ||
| finally | ||
| { | ||
| loading = false; | ||
| } | ||
| </style> | ||
|
|
||
| <div class=""row-template-grid""> | ||
| <BitDataGrid Items=""@allCountries"" Pagination=""@pagination""> | ||
| <Columns> | ||
| <BitDataGridTemplateColumn Class=""row-template-expand-col""> | ||
| <BitButton Variant=""BitVariant.Text"" Size=""BitSize.Small"" | ||
| IconName=""@(expandedRowTemplateCodes.Contains(context.Code) ? BitIconName.ChevronDown : BitIconName.ChevronRight)"" | ||
| OnClick=""@(() => ToggleRowRendererExpand(context.Code))"" /> | ||
| </BitDataGridTemplateColumn> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Name)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Gold)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Silver)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Bronze)"" /> | ||
| <BitDataGridPropertyColumn Property=""@(c => c.Medals.Total)"" /> | ||
| </Columns> | ||
| <RowTemplate Context=""args""> | ||
| @args.OriginalRow | ||
| @if (expandedRowTemplateCodes.Contains(args.RowItem.Code)) | ||
| { | ||
| <tr> | ||
| <td colspan=""6""> | ||
| <div class=""row-template-detail""> | ||
| <strong>Additional data:</strong> | ||
| Code: [@args.RowItem.Code] - | ||
| Gold: [@args.RowItem.Medals.Gold], | ||
| Silver: [@args.RowItem.Medals.Silver], | ||
| Bronze: [@args.RowItem.Medals.Bronze] | ||
| (Total: @args.RowItem.Medals.Total) | ||
| </div> | ||
| </td> | ||
| </tr> | ||
| } | ||
| </RowTemplate> | ||
| </BitDataGrid> | ||
| <BitDataGridPaginator Value=""@pagination"" /> | ||
| </div>"; | ||
| private readonly string example7CsharpCode = @" | ||
| private IQueryable<CountryModel> allCountries; | ||
| private BitDataGridPaginationState pagination = new() { ItemsPerPage = 7 }; | ||
|
|
||
| protected override async Task OnInitializedAsync() | ||
| { | ||
| allCountries = _countries.AsQueryable(); | ||
| } | ||
| }" + ProductModelCode + SampleDataCode; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Loading snippet should force re-render when loading changes
In example11CsharpCode, LoadData updates loading but never requests a render. Because this is a Func callback, copied code can show stale loading UI. Add explicit renders after setting/resetting loading.
Proposed fix
private async Task<BitDataGridReadResult<Product>> LoadData(BitDataGridReadRequest request)
{
loading = true;
+ await InvokeAsync(StateHasChanged);
try
{
await Task.Delay(250, request.CancellationToken); // simulate a backend round-trip
@@
finally
{
loading = false;
+ await InvokeAsync(StateHasChanged);
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/DataGrid/BitDataGridDemo.razor.samples.cs`
around lines 323 - 376, The LoadData method updates the loading state variable
but does not explicitly request a component re-render, causing the UI to display
stale values when this callback is invoked. Add explicit calls to
StateHasChanged() after setting loading to true (right after the assignment at
the start of the try block) and after setting loading to false (in the finally
block) to ensure the component re-renders when the loading state changes. Use
await InvokeAsync(StateHasChanged) to properly invoke the render on the correct
synchronization context within this async callback.
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
1 participant
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.
closes #12502
Summary by CodeRabbit
New Features
OnRead), infinite (OnLoadMore), and virtualization (including improved empty/loading states).Breaking Changes
Documentation / Demos