fix: ensure file input and name is cleared when crossing out files#38851
fix: ensure file input and name is cleared when crossing out files#38851dodaa08 wants to merge 9 commits into
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
WalkthroughThe change fixes a UI bug where the native file input kept showing a removed filename by adding a React ref to the file input and clearing its value when the staged files array becomes empty. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/client/views/admin/import/NewImportPage.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/import/NewImportPage.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/views/admin/import/NewImportPage.tsx (2)
23-23: LGTM —useRefimport andfileInputRefdeclaration are correct.
useRef<HTMLInputElement>(null)is accurately typed for anHTMLInputElementref.Also applies to: 88-88
294-294: Verify thatInputBoxfrom@rocket.chat/fuselageforwards its ref to the underlying<input>element.The fix at line 294 uses
ref={fileInputRef}onInputBox, but ifInputBoxdoes not useforwardRefto propagate the ref to its native<input>element, thenfileInputRef.currentwill benullat runtime and the chip removal viafileInputRef.current?.click()will silently fail.Check the
@rocket.chat/fuselagepackage source (or test at runtime) to confirm thatInputBoxproperly forwards refs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/views/admin/import/NewImportPage.tsx`:
- Around line 104-112: The updater passed to setFiles inside
handleFileUploadChipClick currently performs a DOM mutation
(fileInputRef.current.value = '') which violates React updater purity; remove
that DOM write from the functional updater so the updater only returns
filteredFiles, and add a useEffect that watches files and, when files.length ===
0 and fileInputRef.current exists, sets fileInputRef.current.value = '' to
synchronise the native input. Update handleFileUploadChipClick to only compute
and return the new files array and rely on the new useEffect to clear the input.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38851 +/- ##
===========================================
+ Coverage 70.55% 70.57% +0.02%
===========================================
Files 3189 3189
Lines 112703 112707 +4
Branches 20429 20423 -6
===========================================
+ Hits 79519 79546 +27
+ Misses 31123 31102 -21
+ Partials 2061 2059 -2 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Resets the native file input value when a file is crossed, This ensures that the UI accurately reflects that no file is selected after a user clicks the "X" to remove a staged file.
The fix utilizes the fileInputRef to explicitly clear the native input element's value.
Screencast.From.2026-02-21.00-02-44.mp4
Issue(s)
Fixes #38850
Steps to test or reproduce
Further comments
Happy to get feedback on the approach, let me know if any code changes are needed here.
Summary by CodeRabbit