Apply BitFileUpload improvements (#12778) - #12779
Conversation
WalkthroughBitFileUpload now supports richer file metadata, configurable browser uploads, queued and retryable transfers, accessible rendering, customizable styling, expanded demos, and broader automated coverage. ChangesFileUpload improvements
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 2
🧹 Nitpick comments (7)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.ts (1)
388-416: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional: cap the directory walk.
collectEntryrecurses without a depth or total-file limit, so a deeply nested (or cyclic, via symlinked directories on some platforms) dropped folder can recurse/iterate unbounded before the change event ever fires. A depth cap plus a max-files guard would bound the worst case.♻️ Sketch
- private static async collectEntry(entry: any, files: File[]): Promise<void> { + private static async collectEntry(entry: any, files: File[], depth: number = 0): Promise<void> { + if (depth > 32 || files.length >= 10000) return;🤖 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/Components/Inputs/FileUpload/BitFileUpload.ts` around lines 388 - 416, Optionally bound the directory traversal in FileUpload.collectEntry by adding a recursion-depth limit and a maximum collected-file limit. Propagate the current depth through recursive calls, stop walking when either limit is reached, and prevent files beyond the maximum from being added while preserving normal file and directory traversal behavior.src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.scss (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStylelint flags the
.scssextension in@import.
scss/load-partial-extensionfires on this line. If the rest of the repo keeps the extension, consider disabling the rule instead of touching every file; otherwise drop the extension here.🎨 Extension-less import
-@import "../../../Styles/functions.scss"; +@import "../../../Styles/functions";🤖 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/Components/Inputs/FileUpload/BitFileUpload.scss` at line 1, Update the FileUpload stylesheet import to comply with the repository’s Stylelint convention: either remove the .scss extension from the `@import` path or, if extensions are intentionally retained across the repository, disable scss/load-partial-extension for this file or rule configuration. Keep the referenced functions partial unchanged.Source: Linters/SAST tools
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider splitting the page description into shorter sentences.
Both the
PageOutletDescription(used as the meta description, where search engines truncate at ~160 chars) and theDemoPageDescriptionare single ~1000-word run-on blocks. A short lead sentence plus a few sentences would read better and make the meta description usable.🤖 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/Inputs/FileUpload/BitFileUploadDemo.razor` around lines 6 - 9, Shorten both the PageOutlet Description and DemoPage Description into concise, readable sentences. Make the PageOutlet text a brief meta description of roughly 160 characters, and replace the DemoPage run-on block with a short lead sentence followed by a few focused sentences covering the primary upload capabilities.src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cs (2)
2673-2680: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
HttpClient/handler are never disposed.Each
SetupHttpClientcall leaks a handler for the lifetime of the test context. Registering it so bUnit's container owns disposal (or disposing in cleanup) keeps the suite tidy.🤖 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/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cs` around lines 2673 - 2680, Update SetupHttpClient so the registered HttpClient and its FakeHttpMessageHandler are owned and disposed by the test context, using the existing service-registration or cleanup mechanism. Preserve the method’s current status-code setup and return value while ensuring repeated calls do not leak resources.
2119-2141: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftWall-clock
Task.Delaycouples these tests to internal timing thresholds.This test (and Lines 2154, 2445, 2568, plus the 300 ms
AutoRetryDelayrace at Lines 866-887) depends on real elapsed time exceeding the component's speed-sampling / repaint-throttling windows. On a loaded CI agent the ordering can invert, and the sleeps add seconds to the suite. If the component exposed the sampling interval / a time provider as an injectable seam, these could assert deterministically.🤖 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/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cs` around lines 2119 - 2141, The upload timing tests, including BitFileUploadShouldMeasureTheSpeedAndTheRemainingTimeOfARunningUpload and the other referenced cases, rely on wall-clock delays and are flaky. Expose the component’s speed-sampling, repaint-throttling, and AutoRetryDelay timing through an injectable interval or time-provider seam, then update the tests to control and advance that seam deterministically instead of using Task.Delay or real-time races.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.cs (2)
1340-1344: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueZero-byte file renders as 100% before it is uploaded.
file.Size == 0 → 100makes a freshly selected empty file show a full progress bar. The component itself reports 0% for that case until it settles (seeBitFileUploadShouldNotReportAnEmptyFileAsUploadedBeforeItIsSentinBitFileUploadTests.cs), so the demo helper reads differently from the built-in view. Guarding only the division would keep them aligned. The same logic is duplicated in the embeddedexample27CsharpCode(Line 1948-1953).♻️ Proposed alignment
- if (file.Size == 0 || file.TotalUploadedSize >= file.Size) return 100; + if (file.Size == 0) return file.Status == BitFileUploadStatus.Completed ? 100 : 0; + + if (file.TotalUploadedSize >= file.Size) return 100;🤖 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/Inputs/FileUpload/BitFileUploadDemo.razor.cs` around lines 1340 - 1344, Update GetFileUploadPercent so a zero-byte file returns 0% until upload progress is reported, while preserving the 100% result for files whose uploaded size reaches or exceeds their total size and avoiding division by zero. Apply the same logic to the duplicated helper implementation in example27CsharpCode so both demo paths match the built-in component behavior.
1297-1338: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the now-dead
Filesnull guards.
Filesis documented in this same file (Line 1151-1152) as non-nullIReadOnlyList<BitFileInfo>defaulting to[], sobitFileUpload.Files?... ?? trueandif (bitFileUpload.Files is null) return;can never trigger. Since these snippets are copied by users, the simpler form is worth showing (the embeddedexample27CsharpCodeat Line 1939-1946 carries the same guards).♻️ Proposed simplification
- private bool FileUploadIsEmpty() => !bitFileUpload.Files?.Any(f => f.Status != BitFileUploadStatus.Removed) ?? true; + private bool FileUploadIsEmpty() => !bitFileUpload.Files.Any(f => f.Status != BitFileUploadStatus.Removed); @@ - private async Task HandleUploadOnClick() - { - if (bitFileUpload.Files is null) return; - - await bitFileUpload.Upload(); - } + private Task HandleUploadOnClick() => bitFileUpload.Upload();🤖 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/Inputs/FileUpload/BitFileUploadDemo.razor.cs` around lines 1297 - 1338, Remove the unreachable null guards for the non-null Files collection: simplify FileUploadIsEmpty to evaluate the collection directly without ?. or ?? true, and remove the bitFileUpload.Files null-return check from HandleUploadOnClick while preserving the upload call. Apply the same simplification to the duplicated example27CsharpCode snippet.
🤖 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/Components/Inputs/FileUpload/BitFileUpload.razor.cs`:
- Around line 1664-1666: The headers conditional in the file upload logic is not
target-typed because it uses var with an empty collection expression. Declare
headers explicitly as Dictionary<string, string> while preserving the existing
null and copied-fileHeaders branches.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.ts`:
- Around line 227-238: Update the paste handling in setupDragDrop and onPaste so
paste events are captured from window or document rather than only the
non-focusable dropZoneElement. Accept the clipboard files when the
focused/active target belongs to the upload component or its input, while
preserving the existing allowPaste and disabled checks, then register the
listener on the global target.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.scss`:
- Line 1: Update the FileUpload stylesheet import to comply with the
repository’s Stylelint convention: either remove the .scss extension from the
`@import` path or, if extensions are intentionally retained across the repository,
disable scss/load-partial-extension for this file or rule configuration. Keep
the referenced functions partial unchanged.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.ts`:
- Around line 388-416: Optionally bound the directory traversal in
FileUpload.collectEntry by adding a recursion-depth limit and a maximum
collected-file limit. Propagate the current depth through recursive calls, stop
walking when either limit is reached, and prevent files beyond the maximum from
being added while preserving normal file and directory traversal behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor`:
- Around line 6-9: Shorten both the PageOutlet Description and DemoPage
Description into concise, readable sentences. Make the PageOutlet text a brief
meta description of roughly 160 characters, and replace the DemoPage run-on
block with a short lead sentence followed by a few focused sentences covering
the primary upload capabilities.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.cs`:
- Around line 1340-1344: Update GetFileUploadPercent so a zero-byte file returns
0% until upload progress is reported, while preserving the 100% result for files
whose uploaded size reaches or exceeds their total size and avoiding division by
zero. Apply the same logic to the duplicated helper implementation in
example27CsharpCode so both demo paths match the built-in component behavior.
- Around line 1297-1338: Remove the unreachable null guards for the non-null
Files collection: simplify FileUploadIsEmpty to evaluate the collection directly
without ?. or ?? true, and remove the bitFileUpload.Files null-return check from
HandleUploadOnClick while preserving the upload call. Apply the same
simplification to the duplicated example27CsharpCode snippet.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cs`:
- Around line 2673-2680: Update SetupHttpClient so the registered HttpClient and
its FakeHttpMessageHandler are owned and disposed by the test context, using the
existing service-registration or cleanup mechanism. Preserve the method’s
current status-code setup and return value while ensuring repeated calls do not
leak resources.
- Around line 2119-2141: The upload timing tests, including
BitFileUploadShouldMeasureTheSpeedAndTheRemainingTimeOfARunningUpload and the
other referenced cases, rely on wall-clock delays and are flaky. Expose the
component’s speed-sampling, repaint-throttling, and AutoRetryDelay timing
through an injectable interval or time-provider seam, then update the tests to
control and advance that seam deterministically instead of using Task.Delay or
real-time races.
🪄 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 Plus
Run ID: 6779f57b-0544-42ac-82ba-3885a57364f2
📒 Files selected for processing (15)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileInfo.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadStatus.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Utils/Theme/component-css-variables.md
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileInfo.cs (1)
55-58: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard
LastModifiedDateagainst out-of-range timestamps.
DateTimeOffset.FromUnixTimeMillisecondsthrowsArgumentOutOfRangeExceptionoutside the range -62135596800000 to 253402300799999. Browsers can report a bogusFile.lastModifiedvalue from a corrupt filesystem timestamp. A throwing property getter is hard to handle in a Razor render tree. Clamp the value or return null instead.♻️ Proposed guard
- [JsonIgnore] public DateTimeOffset LastModifiedDate => DateTimeOffset.FromUnixTimeMilliseconds(LastModified); + [JsonIgnore] + public DateTimeOffset LastModifiedDate => DateTimeOffset.FromUnixTimeMilliseconds( + Math.Clamp(LastModified, + DateTimeOffset.MinValue.ToUnixTimeMilliseconds(), + DateTimeOffset.MaxValue.ToUnixTimeMilliseconds()));🤖 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/Components/Inputs/FileUpload/BitFileInfo.cs` around lines 55 - 58, Update the LastModifiedDate property in BitFileInfo to handle LastModified values outside DateTimeOffset.FromUnixTimeMilliseconds’ supported range without throwing. Clamp out-of-range timestamps to the valid Unix-millisecond bounds or return a nullable value, while preserving normal conversion for valid browser timestamps.src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cs (1)
1779-1795: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard
UpdateChunkSizeagainst an index outside_files.
UpdateChunkSizeindexes_files[fileIndex]after only checking that the list is not empty. The current caller validates the index, but the method is reachable from a JS-driven callback path. Add the same bounds check used by the other index-based helpers.♻️ Proposed guard
- if (_files.Any() is false || AutoChunkSize is false || ChunkedUpload is false) return; + if (AutoChunkSize is false || ChunkedUpload is false) return; + if (fileIndex < 0 || fileIndex >= _files.Count) return;🤖 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/Components/Inputs/FileUpload/BitFileUpload.razor.cs` around lines 1779 - 1795, Update UpdateChunkSize to return early when fileIndex is outside the valid range of _files, matching the bounds-check pattern used by the other index-based helpers before accessing _files[fileIndex].src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.ts (1)
491-505: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType
uploadUrlas nullable to match the interop contract.
BitFileUpload.razor.cspassesrequestUrlfromGetRequestUploadUrl, which returnsstring?and returns null when no provider or override applies. The parameter is declaredstring, so the fallback at line 505 is the only reason this works. Widen the type so the contract is explicit.♻️ Proposed change
- upload(from: number, to: number, uploadUrl: string, headers: Record<string, string>, formFields: Record<string, string>): void { + upload(from: number, to: number, uploadUrl: string | null | undefined, headers: Record<string, string>, formFields: Record<string, string>): void {Apply the same widening to the
uploadUrlparameter ofFileUpload.upload.🤖 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/Components/Inputs/FileUpload/BitFileUpload.ts` around lines 491 - 505, Update the uploadUrl parameter of FileUpload.upload to accept nullable strings, matching the nullable interop value returned by GetRequestUploadUrl. Preserve the existing fallback to this.uploadUrl when uploadUrl is null or empty.src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor (1)
63-117: 📐 Maintainability & Code Quality | 🔵 TrivialVerify keyboard focus is preserved across button-state transitions.
The Upload/Retry button (Line 76), Pause button (Line 91), and Cancel button (Line 108) live in separate
@ifblocks. When status changes (for example,PendingtoInProgress), the Upload button's block stops rendering while the Pause button's block starts rendering as a new element. Without an explicit key or focus-management step, a keyboard or screen-reader user focused on the Upload button loses focus when it disappears, and focus is not moved to the newly rendered Pause button.Confirm whether Blazor's diffing preserves focus here in practice. If not, restore focus to the newly rendered control (for example, via
ElementReference.FocusAsync()inOnAfterRenderAsync) after each transition.🤖 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/Components/Inputs/FileUpload/_BitFileUploadItem.razor` around lines 63 - 117, Verify focus behavior for the controls rendered by the Upload/Retry, Pause, and Cancel conditional blocks in _BitFileUploadItem.razor. If transitions replace the focused button and Blazor does not preserve focus, add lifecycle-based focus management that tracks the active control and calls ElementReference.FocusAsync() in OnAfterRenderAsync after each state transition, while avoiding unnecessary refocusing.
🤖 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/Components/Inputs/FileUpload/BitFileUpload.razor.cs`:
- Around line 1588-1597: Update the upload flow in UploadOneFile around the
BitFileUploadUpload interop call so any thrown exception clears
fileInfo.IsRequestInFlight before propagating or handling the failure. Preserve
the existing true assignment for an active request and ensure later retries can
proceed after JSDisconnectedException or other interop errors.
- Around line 1332-1357: Update the upload setup flow after
`_files.AddRange(newFiles)` so `UploadStatus` is set to `Pending` only for
non-append selections; preserve the existing `InProgress` status when `Append`
is true and uploads are still active, allowing `CheckAllUploadsComplete` to
invoke `OnAllUploadsComplete` after all files settle.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.ts`:
- Around line 29-40: Update the rejection handling in setup and the status
cleanup logic around Release so files marked NotAllowed have their previewUrl
revoked and associated resources removed, matching the existing Removed cleanup
behavior. Ensure rejected items cannot retain object URLs or remain in the
renderer, while preserving normal handling for accepted files.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cs`:
- Around line 2424-2450: Remove the wall-clock-dependent negative repaint
assertion from BitFileUploadShouldNotRepaintTheFileListOnEveryProgressReport,
specifically the check that the rendered percentage remains "40%" immediately
after the second progress report. Keep the LastChunkUploadedSize assertion and
the later delayed "80%" assertion so the test still verifies progress reception
and eventual repaint behavior without relying on scheduler timing.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor`:
- Around line 63-117: Verify focus behavior for the controls rendered by the
Upload/Retry, Pause, and Cancel conditional blocks in _BitFileUploadItem.razor.
If transitions replace the focused button and Blazor does not preserve focus,
add lifecycle-based focus management that tracks the active control and calls
ElementReference.FocusAsync() in OnAfterRenderAsync after each state transition,
while avoiding unnecessary refocusing.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileInfo.cs`:
- Around line 55-58: Update the LastModifiedDate property in BitFileInfo to
handle LastModified values outside DateTimeOffset.FromUnixTimeMilliseconds’
supported range without throwing. Clamp out-of-range timestamps to the valid
Unix-millisecond bounds or return a nullable value, while preserving normal
conversion for valid browser timestamps.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cs`:
- Around line 1779-1795: Update UpdateChunkSize to return early when fileIndex
is outside the valid range of _files, matching the bounds-check pattern used by
the other index-based helpers before accessing _files[fileIndex].
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.ts`:
- Around line 491-505: Update the uploadUrl parameter of FileUpload.upload to
accept nullable strings, matching the nullable interop value returned by
GetRequestUploadUrl. Preserve the existing fallback to this.uploadUrl when
uploadUrl is null or empty.
🪄 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 Plus
Run ID: 7d3a474c-994a-41c0-883f-7c9da4115aaa
📒 Files selected for processing (15)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileInfo.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadStatus.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Utils/Theme/component-css-variables.md
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor (1)
61-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the repeated action-button markup.
The Upload/Retry (Lines 76-84), Pause (Lines 91-99), Cancel (Lines 108-116), and Remove (Lines 132-140) buttons share the same structure:
type="button",disabled,title,aria-label,style,class="bit-upl-usi ...", and an inner<i>icon. Only the callback, icon, title, and class suffix differ.Extract a small private helper (a child component or a
RenderFragment-returning method taking the callback, icon, title, and style/class pair) to keep the four buttons in sync as this markup evolves.🤖 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/Components/Inputs/FileUpload/_BitFileUploadItem.razor` around lines 61 - 140, Extract the shared action-button markup in _BitFileUploadItem into a private child component or RenderFragment helper accepting the click callback, icon, title, and style/class values. Replace the Upload/Retry, Pause, Cancel, and Remove button blocks with calls to that helper while preserving their existing conditions, callbacks, accessibility labels, and per-action customization.
🤖 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/Components/Inputs/FileUpload/_BitFileUploadItem.razor`:
- Around line 118-140: Update the successful removal flow in RemoveFile and the
rendered elements in _BitFileUploadItem so focus moves to a nearby stable target
after the removed row and its focused button are unmounted. Prefer the next
available item action, the file-list container, or the browse button, and apply
focus only after the removal render completes while preserving the existing
announcement behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.cs`:
- Around line 1338-1346: Update GetFileUploadPercent and its duplicated
implementation in the embedded example27CsharpCode string to clamp the combined
TotalUploadedSize plus LastChunkUploadedSize against file.Size before
calculating the percentage, ensuring the returned width never exceeds 100 while
preserving the existing empty-file behavior.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor`:
- Around line 61-140: Extract the shared action-button markup in
_BitFileUploadItem into a private child component or RenderFragment helper
accepting the click callback, icon, title, and style/class values. Replace the
Upload/Retry, Pause, Cancel, and Remove button blocks with calls to that helper
while preserving their existing conditions, callbacks, accessibility labels, and
per-action customization.
🪄 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 Plus
Run ID: 89bf8480-5252-4a7f-b737-0d0ea5964aad
📒 Files selected for processing (15)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileInfo.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.tssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUploadStatus.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/FileUpload/BitFileUploadDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/FileUpload/BitFileUploadTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Utils/Theme/component-css-variables.md
| else if (FileUpload.ShowRemoveButton) | ||
| { | ||
| @if (FileUpload.IsRemoving) | ||
| @if (Item.IsRemoving) | ||
| { | ||
| <div class="bit-upl-ldg"> | ||
| @* the spinner carries no text of its own, and the live region of the component already | ||
| announces the outcome of the removal, so it is left out of the accessibility tree. *@ | ||
| <div class="bit-upl-ldg" aria-hidden="true"> | ||
| <div class="bit-upl-spn"></div> | ||
| </div> | ||
| } | ||
| else | ||
| { | ||
| var removeIcon = BitIconInfo.From(FileUpload.RemoveIcon, FileUpload.RemoveIconName ?? "Delete"); | ||
| <div class="bit-upl-usi" @onclick="() => FileUpload.RemoveFile(Item)"> | ||
| <i title="remove" class="@removeIcon!.GetCssClasses()" aria-hidden="true" /> | ||
| </div> | ||
| var removeTitle = FileUpload.RemoveButtonTitle ?? "Remove"; | ||
| <button @onclick="() => FileUpload.RemoveFile(Item)" | ||
| type="button" | ||
| title="@removeTitle" | ||
| disabled="@(FileUpload.IsEnabled is false)" | ||
| aria-label="@($"{removeTitle} {Item.Name}")" | ||
| style="@FileUpload.Styles?.RemoveButton" | ||
| class="bit-upl-usi @FileUpload.Classes?.RemoveButton"> | ||
| <i style="@FileUpload.Styles?.RemoveIcon" class="@removeIcon!.GetCssClasses() @FileUpload.Classes?.RemoveIcon" aria-hidden="true" /> | ||
| </button> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C6 'RemoveFile' src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cs
rg -n 'FocusAsync|ElementReference' src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUploadRepository: bitfoundation/bitplatform
Length of output: 1983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File upload component outline/files =="
fd -a '(_BitFileUploadItem\.razor|BitFileUpload\.razor(\.cs)?|BitFileUpload.razor)' src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload | sed 's#^\./##'
echo
echo "== BitFileUpload.razor.cs relevant RemoveFile implementation =="
sed -n '790,940p' src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/BitFileUpload.razor.cs
echo
echo "== FocusAsync/ElementReference references around file upload =="
rg -n -C3 'FocusAsync|ElementReference|_file|_files|Browse|RemoveFile|Remove' src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload --glob '!**/*.js'
echo
echo "== Item markup relevant accessibility focus/removal section =="
sed -n '1,180p' src/BlazorUI/Bit.BlazorUI/Components/Inputs/FileUpload/_BitFileUploadItem.razorRepository: bitfoundation/bitplatform
Length of output: 50381
Restore focus after a file removal unmounts the remove button.
RemoveFile only announces the removal and then renders the remaining list. When a removed item is filtered out, its row and focused remove button are unmounted, leaving keyboard/screen-reader users at <body>. Move focus to a nearby stable target after removal succeeds, such as the browse button, the file-list container, or the next item’s action button.
🤖 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/Components/Inputs/FileUpload/_BitFileUploadItem.razor`
around lines 118 - 140, Update the successful removal flow in RemoveFile and the
rendered elements in _BitFileUploadItem so focus moves to a nearby stable target
after the removed row and its focused button are unmounted. Prefer the next
available item action, the file-list container, or the browse button, and apply
focus only after the removal render completes while preserving the existing
announcement behavior.
| private static int GetFileUploadPercent(BitFileInfo file) | ||
| { | ||
| int uploadedPercent; | ||
| if (file.TotalUploadedSize >= file.Size) | ||
| { | ||
| uploadedPercent = 100; | ||
| } | ||
| else | ||
| { | ||
| uploadedPercent = (int)((file.TotalUploadedSize + file.LastChunkUploadedSize) / (float)file.Size * 100); | ||
| } | ||
| // an empty file has no byte whose progress could be measured, so it is either done or not started. | ||
| if (file.Size == 0) return file.Status is BitFileUploadStatus.Completed ? 100 : 0; | ||
|
|
||
| return uploadedPercent; | ||
| if (file.TotalUploadedSize >= file.Size) return 100; | ||
|
|
||
| return (int)((file.TotalUploadedSize + file.LastChunkUploadedSize) / (float)file.Size * 100); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp the percentage on the combined byte count.
Line 1343 compares only TotalUploadedSize against Size. Line 1345 then adds LastChunkUploadedSize to TotalUploadedSize. The sum can exceed Size, because the progress events count the multipart overhead as well. The test BitFileUploadShouldNotShowAnUploadedSizeLargerThanTheFile documents that behavior. The template at line 682 writes the result into style="width:@fileUploadPercent%", so a value above 100 overflows the progress bar.
The same code is duplicated in the embedded example27CsharpCode string at lines 1947-1955, so apply the fix in both places.
🐛 Proposed fix for the percentage clamp
private static int GetFileUploadPercent(BitFileInfo file)
{
// an empty file has no byte whose progress could be measured, so it is either done or not started.
if (file.Size == 0) return file.Status is BitFileUploadStatus.Completed ? 100 : 0;
- if (file.TotalUploadedSize >= file.Size) return 100;
+ var uploaded = file.TotalUploadedSize + file.LastChunkUploadedSize;
- return (int)((file.TotalUploadedSize + file.LastChunkUploadedSize) / (float)file.Size * 100);
+ if (uploaded >= file.Size) return 100;
+
+ return (int)(uploaded / (float)file.Size * 100);
}🤖 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/Inputs/FileUpload/BitFileUploadDemo.razor.cs`
around lines 1338 - 1346, Update GetFileUploadPercent and its duplicated
implementation in the embedded example27CsharpCode string to clamp the combined
TotalUploadedSize plus LastChunkUploadedSize against file.Size before
calculating the percentage, ensuring the returned width never exceeds 100 while
preserving the existing empty-file behavior.
1 participant
closes #12778
Summary by CodeRabbit
New Features
Bug Fixes
Documentation