fix(desktop): warn before closing app during active upload#3262
fix(desktop): warn before closing app during active upload#3262Ram-sah19 wants to merge 13 commits intoRocketChat:developfrom
Conversation
WalkthroughAdds upload-tracking with renderer preload APIs and a main-process quit guard that prompts when uploads are active; integrates tracking into Redux and app startup, adjusts notification silent mapping, enables macOS Kerberos switches, allows plugins for specific webviews, and removes package.json devEngines. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Renderer as Renderer
participant Redux as Redux
participant Main as Main
participant Dialog as Dialog
User->>Renderer: Start upload
Renderer->>Redux: dispatch(UPLOAD_STARTED {id, fileName})
Redux->>Main: preload/IPC -> notifyUploadStarted(id, fileName)
Main->>Main: add id to activeUploads set
User->>Main: Attempt to close app/window
Main->>Main: on-close -> await checkActiveUploads()
alt active uploads exist
Main->>Dialog: show "Upload in progress" (Quit / Cancel)
Dialog->>User: prompt
User-->>Dialog: Cancel or Quit
alt Cancel
Dialog->>Main: return false
Main->>Main: abort close
else Quit
Dialog->>Main: return true
Main->>Main: proceed to quit
end
else no active uploads
Main->>Main: proceed to quit
end
Renderer->>Redux: dispatch(UPLOAD_FINISHED/FAILED {id})
Redux->>Main: preload/IPC -> notifyUploadFinished/Failed(id)
Main->>Main: remove id from activeUploads set
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan for PR comments
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/notifications/main.ts (1)
61-67:⚠️ Potential issue | 🟠 MajorNotifications silenced by default when
silentis not explicitly provided.With
silent !== false, when callers omit thesilentparameter (it'sundefined), the expression evaluates totrue, making all notifications silent. This overrides Electron's default behavior whereundefinedrespects OS notification settings. Calls fromsrc/downloads/main.tsandsrc/injected.ts:269-273do not providesilent, so they will be affected.🔧 Suggested fix
- silent: silent !== false, + silent: typeof silent === 'boolean' ? silent : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/notifications/main.ts` around lines 61 - 67, The Notification is being created with "silent: silent !== false", which makes notifications silent when callers omit the silent parameter; update the Notification creation in src/notifications/main.ts so the silent property is only set when the caller explicitly provides it (i.e., preserve undefined to let Electron/OS defaults apply). Concretely, change the Notification instantiation (the object with title, subtitle, body, icon, silent, hasReply) so that you either omit the silent key when silent is undefined or assign silent directly only if provided (ensure callers that pass true/false still work), leaving resolveIcon(...) and other fields unchanged.
🧹 Nitpick comments (3)
src/ui/main/serverView/index.ts (1)
213-213: Scopepluginsmore narrowly.This turns plugin loading on for every server webview/popup, which increases the capabilities of untrusted remote content globally. If this is only needed for a specific surface, gate it there instead of enabling it on the default attach path.
Also applies to: 254-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/main/serverView/index.ts` at line 213, The code currently enables plugins globally by setting webPreferences.plugins = true; instead make plugins opt-in: set webPreferences.plugins = false by default and only set it to true in the specific webview creation path that requires plugins (e.g., inside the popup/server webview factory or the handler that creates the trusted surface) by checking an explicit flag or trust/origin check (for example use an options.enablePlugins or options.trusted check when creating the webview); update both occurrences of webPreferences.plugins (the instances flagged) so untrusted/default attach paths remain false.UPLOAD_TRACKING_FEATURE.md (2)
189-200: Add language specifier to ASCII diagram code block.The dialog mockup should have a language specifier.
📝 Proposed fix
-``` +```text ┌─────────────────────────────────────────┐🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@UPLOAD_TRACKING_FEATURE.md` around lines 189 - 200, The ASCII dialog mockup code block is missing a language specifier; update the triple-backtick fence that surrounds the box diagram in UPLOAD_TRACKING_FEATURE.md to include a language tag (use "text") so the block becomes ```text and the diagram (the box starting with "┌─────────────────────────────────────────┐") is rendered correctly as plain text.
32-42: Add language specifier to fenced code blocks.The ASCII flow diagrams should have a language specifier (e.g.,
textorplaintext) to satisfy markdown linting rules.📝 Proposed fix
-``` +```text Rocket.Chat Web Client starts uploadApply similarly to lines 46 and 62.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@UPLOAD_TRACKING_FEATURE.md` around lines 32 - 42, The fenced ASCII flow diagrams (the blocks showing "Rocket.Chat Web Client starts upload" through "window.RocketChatDesktop.notifyUploadStarted(id, fileName)" and the subsequent flows referencing symbols like notifyUploadStarted, UPLOAD_STARTED, and activeUploadsCount++) lack a language specifier and fail markdown linting; update each triple-backtick fence for those diagrams to include a language (e.g., ```text or ```plaintext) so the blocks are valid Markdown—apply the same change to the other diagram fences in the file (the ones corresponding to the other flows).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@NOTIFICATION_SOUND_FIX.md`:
- Around line 10-13: Update the documentation and tests to reflect correct
JavaScript semantics and the implemented behavior: change the root cause text to
state that "false ?? undefined" evaluates to false (the nullish coalescing
operator returns the left operand if it's not null/undefined), and explain that
the fix uses "silent !== false" so non‑false values (including undefined) will
result in a silent-enabled behavior only when explicitly false; then align Test
3 ("Non-Silent Notification (Explicitly with Sound)" / the test titled with
explicit sound) to expect a notification WITH sound and update the checklist
entry that currently reads "No sound plays when silent is false" to assert that
sound does play when silent is false.
In `@src/app/main/app.ts`:
- Around line 69-79: The Kerberos/SPNEGO whitelist is being set unconditionally
on macOS via process.platform === 'darwin' and
app.commandLine.appendSwitch('auth-server-whitelist', ...) and
app.commandLine.appendSwitch('auth-negotiate-delegate-whitelist', ...); instead,
gate these calls behind the same domain opt-in checks used for NTLM in
setupApp(): consult isNTLMCredentialsEnabled and allowedNTLMCredentialsDomains
and only call app.commandLine.appendSwitch with the whitelisted domains when
isNTLMCredentialsEnabled is true and allowedNTLMCredentialsDomains contains the
target domains (or otherwise follows the same opt-in logic), ensuring identical
per-domain opt-in behavior for Kerberos/SPNEGO and NTLM.
In `@src/main.ts`:
- Line 39: Move the import of setupUploadTracking so it appears before the
import of ./userPresence/main to satisfy ESLint import ordering; specifically,
in src/main.ts reorder the import statement "import { setupUploadTracking } from
'./uploads/main';" to precede the import that brings in ./userPresence/main
(adjust any grouped imports accordingly) and run lint to ensure ordering is now
correct.
In `@src/ui/main/rootWindow.ts`:
- Line 19: Move the import of checkActiveUploads so it appears after (or before,
per project convention) the import from ../actions to satisfy ESLint
import-order; specifically, in src/ui/main/rootWindow.ts reorder the import
statements so the import that references ../actions appears in the proper
position relative to the import { checkActiveUploads } from
'../../uploads/main', ensuring the symbol checkActiveUploads is still referenced
correctly and tests/compilation pass.
In `@src/uploads/main.ts`:
- Line 3: The import statement brings in unused symbols dispatch and select
which ESLint flags; edit the import to remove dispatch and select and only
import listen (i.e., drop dispatch and select from the import list in
src/uploads/main.ts), or if those functions are actually needed, use them where
appropriate; ensure the final import only contains used symbols to satisfy
ESLint.
In `@src/uploads/reducer.ts`:
- Around line 10-13: The reducer uses Upload action types (UPLOAD_STARTED,
UPLOAD_FINISHED, UPLOAD_FAILED) via ActionOf<...> but their
UploadsActionTypeToPayloadMap is not registered in the global action registry;
import UploadsActionTypeToPayloadMap from the uploads actions module and merge
it into the root ActionTypeToPayloadMap (e.g., via a union/augmentation so
ActionTypeToPayloadMap includes UploadsActionTypeToPayloadMap) so TypeScript
strict FSA typing satisfies the reducer's ActionOf constraint.
In `@UPLOAD_TRACKING_FEATURE.md`:
- Line 300: Update the placeholder created date string "**Created**: 2025-01-XX"
to the actual creation date (replace "2025-01-XX" with the real YYYY-MM-DD) in
the UPLOAD_TRACKING_FEATURE.md file so the metadata is accurate; locate the line
containing the bold "Created" label and substitute the placeholder with the
correct date.
---
Outside diff comments:
In `@src/notifications/main.ts`:
- Around line 61-67: The Notification is being created with "silent: silent !==
false", which makes notifications silent when callers omit the silent parameter;
update the Notification creation in src/notifications/main.ts so the silent
property is only set when the caller explicitly provides it (i.e., preserve
undefined to let Electron/OS defaults apply). Concretely, change the
Notification instantiation (the object with title, subtitle, body, icon, silent,
hasReply) so that you either omit the silent key when silent is undefined or
assign silent directly only if provided (ensure callers that pass true/false
still work), leaving resolveIcon(...) and other fields unchanged.
---
Nitpick comments:
In `@src/ui/main/serverView/index.ts`:
- Line 213: The code currently enables plugins globally by setting
webPreferences.plugins = true; instead make plugins opt-in: set
webPreferences.plugins = false by default and only set it to true in the
specific webview creation path that requires plugins (e.g., inside the
popup/server webview factory or the handler that creates the trusted surface) by
checking an explicit flag or trust/origin check (for example use an
options.enablePlugins or options.trusted check when creating the webview);
update both occurrences of webPreferences.plugins (the instances flagged) so
untrusted/default attach paths remain false.
In `@UPLOAD_TRACKING_FEATURE.md`:
- Around line 189-200: The ASCII dialog mockup code block is missing a language
specifier; update the triple-backtick fence that surrounds the box diagram in
UPLOAD_TRACKING_FEATURE.md to include a language tag (use "text") so the block
becomes ```text and the diagram (the box starting with
"┌─────────────────────────────────────────┐") is rendered correctly as plain
text.
- Around line 32-42: The fenced ASCII flow diagrams (the blocks showing
"Rocket.Chat Web Client starts upload" through
"window.RocketChatDesktop.notifyUploadStarted(id, fileName)" and the subsequent
flows referencing symbols like notifyUploadStarted, UPLOAD_STARTED, and
activeUploadsCount++) lack a language specifier and fail markdown linting;
update each triple-backtick fence for those diagrams to include a language
(e.g., ```text or ```plaintext) so the blocks are valid Markdown—apply the same
change to the other diagram fences in the file (the ones corresponding to the
other flows).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db1b1b42-4b76-498e-903a-e9c6b5f05d8b
📒 Files selected for processing (13)
NOTIFICATION_SOUND_FIX.mdUPLOAD_TRACKING_FEATURE.mdsrc/app/main/app.tssrc/main.tssrc/notifications/main.tssrc/servers/preload/api.tssrc/store/rootReducer.tssrc/ui/main/rootWindow.tssrc/ui/main/serverView/index.tssrc/uploads/actions.tssrc/uploads/main.tssrc/uploads/preload.tssrc/uploads/reducer.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/uploads/main.tssrc/uploads/actions.tssrc/uploads/preload.tssrc/store/rootReducer.tssrc/ui/main/serverView/index.tssrc/ui/main/rootWindow.tssrc/main.tssrc/uploads/reducer.tssrc/servers/preload/api.tssrc/notifications/main.tssrc/app/main/app.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
📚 Learning: 2026-03-06T19:31:11.433Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/preload.ts : Keep preload.ts logging minimal since it cannot access the verbose logging toggle from Redux Store and all logs always appear
Applied to files:
src/uploads/preload.tssrc/servers/preload/api.ts
📚 Learning: 2026-03-06T19:31:11.433Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/*(!preload).ts?(x) : Always use the centralized logger from `logger.ts` (outlookLog, outlookDebug, outlookError, outlookWarn, outlookEventDetail) instead of console.log() for Outlook Calendar module logging
Applied to files:
src/servers/preload/api.ts
📚 Learning: 2026-03-06T19:31:11.433Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/*.{ts,tsx} : Always use outlookError() for errors as it logs regardless of verbose mode settings, ensuring errors are always visible to users
Applied to files:
src/servers/preload/api.ts
🪛 ESLint
src/uploads/main.ts
[error] 3-3: 'dispatch' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 3-3: 'select' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 34-34: Insert ⏎·····
(prettier/prettier)
src/ui/main/rootWindow.ts
[error] 19-19: ../../uploads/main import should occur before import of ../actions
(import/order)
src/main.ts
[error] 39-39: ./uploads/main import should occur before import of ./userPresence/main
(import/order)
src/uploads/reducer.ts
[error] 12-12: Replace typeof·UPLOAD_STARTED·|·typeof·UPLOAD_FINISHED·|·typeof·UPLOAD_FAILED with ⏎····typeof·UPLOAD_STARTED·|·typeof·UPLOAD_FINISHED·|·typeof·UPLOAD_FAILED⏎··
(prettier/prettier)
🪛 markdownlint-cli2 (0.21.0)
NOTIFICATION_SOUND_FIX.md
[warning] 82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
UPLOAD_TRACKING_FEATURE.md
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 189-189: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
src/servers/preload/api.ts (3)
13-17: LGTM!The new upload tracking functions are correctly imported and follow the established pattern used by other preload modules in this file.
84-86: LGTM!Type declarations for the upload tracking API methods are correctly added to
RocketChatDesktopAPI. The signatures match the implementations insrc/uploads/preload.ts.
122-124: LGTM!The new methods are correctly wired into the
RocketChatDesktopexport object, following the same pattern as other API methods.src/ui/main/rootWindow.ts (1)
264-269: LGTM!The upload check is correctly integrated into the close handler. The guard returns early if
checkActiveUploads()returns false (user cancelled), preventing the window from closing during active uploads.src/main.ts (1)
79-79: LGTM!The placement of
setupUploadTracking()is correct — it's called aftercreateMainReduxStore()(line 54), which is required sincesetupUploadTrackinguseslisten()that depends on the Redux store being initialized.src/uploads/main.ts (2)
9-21: LGTM!The upload tracking setup is well-implemented:
- Listeners correctly increment/decrement the counter
Math.max(0, ...)prevents underflow for edge cases where finish/fail might be called without a corresponding start
23-38: LGTM with minor formatting fix needed.The dialog logic is correct. Using
showMessageBoxSyncis appropriate here since it blocks the close event handler until the user responds.Note: Prettier flags line 34 as too long. Consider breaking the message string.
📝 Optional formatting fix for line 34
const choice = dialog.showMessageBoxSync(rootWindow, { type: 'warning', buttons: ['Cancel', 'Quit'], defaultId: 0, title: 'Upload in Progress', - message: 'A file upload is currently in progress. Are you sure you want to quit?', + message: + 'A file upload is currently in progress. Are you sure you want to quit?', });
| **Root Cause**: The `silent` flag in notification options was not being properly enforced. The code used `silent ?? undefined` which meant: | ||
| - If `silent` was `false`, it became `undefined` | ||
| - Electron would then use the system default (play sound) | ||
| - This caused unwanted notification sounds |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the NOTIFICATION_SOUND_FIX.md file
fd -t f NOTIFICATION_SOUND_FIX.mdRepository: RocketChat/Rocket.Chat.Electron
Length of output: 100
🏁 Script executed:
# Read the NOTIFICATION_SOUND_FIX.md file to verify the claims
cat -n NOTIFICATION_SOUND_FIX.mdRepository: RocketChat/Rocket.Chat.Electron
Length of output: 11165
🏁 Script executed:
# Verify the JavaScript semantics with a quick test
node -e "console.log('false ?? undefined =', false ?? undefined)"Repository: RocketChat/Rocket.Chat.Electron
Length of output: 101
The root cause explanation and test expectations contradict the implementation.
Lines 10–13 incorrectly state that false ?? undefined becomes undefined. In JavaScript, the nullish coalescing operator returns the left operand if it is not null or undefined, so false ?? undefined evaluates to false.
Additionally, line 45 shows the fix changes this to silent !== false. When silent is false, this evaluates to false, meaning Electron should play the sound. However, Test 3 (lines 138–141) is titled "Non-Silent Notification (Explicitly with Sound)" but expects "WITHOUT sound," and the checklist at line 153 states "No sound plays when silent is false"—both directly contradicting the implemented logic.
Correct the root cause explanation to reflect actual JavaScript semantics and align the test expectations with the code behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@NOTIFICATION_SOUND_FIX.md` around lines 10 - 13, Update the documentation and
tests to reflect correct JavaScript semantics and the implemented behavior:
change the root cause text to state that "false ?? undefined" evaluates to false
(the nullish coalescing operator returns the left operand if it's not
null/undefined), and explain that the fix uses "silent !== false" so non‑false
values (including undefined) will result in a silent-enabled behavior only when
explicitly false; then align Test 3 ("Non-Silent Notification (Explicitly with
Sound)" / the test titled with explicit sound) to expect a notification WITH
sound and update the checklist entry that currently reads "No sound plays when
silent is false" to assert that sound does play when silent is false.
| // Enable Kerberos/SPNEGO authentication for SAML on macOS | ||
| if (process.platform === 'darwin') { | ||
| app.commandLine.appendSwitch( | ||
| 'auth-server-whitelist', | ||
| '*.local,*.domain.local' | ||
| ); | ||
| app.commandLine.appendSwitch( | ||
| 'auth-negotiate-delegate-whitelist', | ||
| '*.local,*.domain.local' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the structure around the target lines
cat -n src/app/main/app.ts | head -200Repository: RocketChat/Rocket.Chat.Electron
Length of output: 7076
🏁 Script executed:
# Verify function call order and when settings can be read
rg -A 5 "readSetting\(" src/app/main/app.ts | head -40Repository: RocketChat/Rocket.Chat.Electron
Length of output: 933
Gate the Kerberos whitelist behind the existing domain opt-in settings.
The hardcoded auth-server-whitelist and auth-negotiate-delegate-whitelist switches for *.local,*.domain.local are unconditionally enabled on macOS at startup, while NTLM credentials are gated behind isNTLMCredentialsEnabled and allowedNTLMCredentialsDomains checks later in setupApp(). This creates an inconsistent and broader credential surface for Kerberos/SPNEGO than for NTLM. Both authentication mechanisms should respect the same per-domain opt-in model.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/main/app.ts` around lines 69 - 79, The Kerberos/SPNEGO whitelist is
being set unconditionally on macOS via process.platform === 'darwin' and
app.commandLine.appendSwitch('auth-server-whitelist', ...) and
app.commandLine.appendSwitch('auth-negotiate-delegate-whitelist', ...); instead,
gate these calls behind the same domain opt-in checks used for NTLM in
setupApp(): consult isNTLMCredentialsEnabled and allowedNTLMCredentialsDomains
and only call app.commandLine.appendSwitch with the whitelisted domains when
isNTLMCredentialsEnabled is true and allowedNTLMCredentialsDomains contains the
target domains (or otherwise follows the same opt-in logic), ensuring identical
per-domain opt-in behavior for Kerberos/SPNEGO and NTLM.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/main/rootWindow.ts (1)
264-299:⚠️ Potential issue | 🟠 MajorGuard incomplete—other quit paths bypass upload check.
checkActiveUploads()only blocks the window 'close' listener (line 266). Other app-exit paths bypass it entirely:
- Menu Cmd+Q and tray "Quit" call
app.quit()directly without checking- Update installations call
quitAndInstall()ordestroy()windows after removing the close listener- Window-all-closed listener (app.ts:126) calls
app.quit()unconditionallyUsers can lose in-flight uploads via any quit path except the window close button.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/main/rootWindow.ts` around lines 264 - 299, The close-handler currently uses checkActiveUploads() but other quit paths (direct app.quit(), quitAndInstall(), window-all-closed handler, tray/menu Quit) bypass it; create a single guarded quit flow (e.g., guardQuit or performQuit) that calls checkActiveUploads() and only proceeds to app.quit()/app.quitAndInstall()/destroy windows when uploads are clear, then replace all direct calls to app.quit(), quitAndInstall(), and any window destroy or window-all-closed logic to call this guarded function (also use it from the rootWindow.close listener instead of calling app.quit() there); ensure symbols referenced include checkActiveUploads, rootWindow.addListener('close', ...), app.quit, quitAndInstall, and the window-all-closed handler so you update every exit path to use the new guarded quit routine.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/uploads/main.ts`:
- Around line 29-35: The warning message string passed to
dialog.showMessageBoxSync (in the call using rootWindow) is too long for
Prettier; split or wrap the message value across lines so it conforms to
line-length rules (e.g., convert the message to a multi-line template literal or
concatenate shorter strings) inside the same dialog.showMessageBoxSync call and
update only the message property so ESLint/Prettier will pass.
- Around line 7-25: Replace the scalar counter with a Set<string> to track
active upload ids: change activeUploadsCount to activeUploads (a Set), update
setupUploadTracking to listen for UPLOAD_STARTED and add payload.id to
activeUploads, and on UPLOAD_FINISHED and UPLOAD_FAILED remove payload.id from
the Set; update checkActiveUploads to return activeUploads.size === 0 (or
true/false accordingly). Ensure the listeners for UPLOAD_STARTED,
UPLOAD_FINISHED, and UPLOAD_FAILED extract the id from the event payload and
guard for missing ids so removals/additions are safe.
In `@UPLOAD_TRACKING_FEATURE.md`:
- Around line 189-200: The fenced code block in UPLOAD_TRACKING_FEATURE.md is
missing a language tag (triggering markdownlint MD040); update the
triple-backtick fence that wraps the ASCII UI diagram to include a language
identifier such as text (e.g., change ``` to ```text) so the block is explicitly
marked as plain text and the linter warning is resolved.
---
Outside diff comments:
In `@src/ui/main/rootWindow.ts`:
- Around line 264-299: The close-handler currently uses checkActiveUploads() but
other quit paths (direct app.quit(), quitAndInstall(), window-all-closed
handler, tray/menu Quit) bypass it; create a single guarded quit flow (e.g.,
guardQuit or performQuit) that calls checkActiveUploads() and only proceeds to
app.quit()/app.quitAndInstall()/destroy windows when uploads are clear, then
replace all direct calls to app.quit(), quitAndInstall(), and any window destroy
or window-all-closed logic to call this guarded function (also use it from the
rootWindow.close listener instead of calling app.quit() there); ensure symbols
referenced include checkActiveUploads, rootWindow.addListener('close', ...),
app.quit, quitAndInstall, and the window-all-closed handler so you update every
exit path to use the new guarded quit routine.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2cbbdcb8-82ee-44a4-8c22-050aabe2525a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
UPLOAD_TRACKING_FEATURE.mdpackage.jsonsrc/main.tssrc/notifications/main.tssrc/store/actions.tssrc/ui/main/rootWindow.tssrc/uploads/main.ts
💤 Files with no reviewable changes (1)
- package.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/main.tssrc/ui/main/rootWindow.tssrc/notifications/main.tssrc/store/actions.tssrc/uploads/main.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
📚 Learning: 2026-03-11T06:38:40.426Z
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
Applied to files:
src/ui/main/rootWindow.ts
🪛 ESLint
src/uploads/main.ts
[error] 34-34: Insert ⏎·····
(prettier/prettier)
🪛 markdownlint-cli2 (0.21.0)
UPLOAD_TRACKING_FEATURE.md
[warning] 189-189: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/main/rootWindow.ts (1)
271-305:⚠️ Potential issue | 🟠 MajorMove the upload check to only prompt when the app will actually quit.
The
checkActiveUploads()call at the start shows a "Quit" dialog but executes before platform checks. On macOS or with tray enabled, the window only hides (uploads continue); on Windows with minimize enabled, it minimizes. Users are misled about what "Quit" means. Additionally, if execution reachesguardedQuit(),checkActiveUploads()runs again, allowing users to be prompted twice about the same uploads.Restructure the listener: only call
checkActiveUploads()on the path that actually quits (when all platform conditions that hide/minimize are false).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/main/rootWindow.ts` around lines 271 - 305, Remove the initial await checkActiveUploads() at the top of the close listener and instead only call await checkActiveUploads() on the path that actually performs a quit (i.e., just before guardedQuit()). Keep the full-screen exit (rootWindow.isFullScreen / leave-full-screen / setFullScreen(false)) and blur behavior as-is, evaluate isTrayIconEnabled and isMinimizeOnCloseEnabled to decide hide/minimize, and if those conditions would not short-circuit (hide or minimize), then call checkActiveUploads(); if it returns false, return; otherwise call guardedQuit(); this prevents prompting on hide/minimize and avoids a double prompt before guardedQuit().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/main/rootWindow.ts`:
- Around line 22-27: Three direct calls to app.quit() bypass the upload-check
flow; replace those direct quits (the Cmd/Ctrl+Q handler in menuBar.ts, the tray
quit in trayIcon.ts, and the window-all-closed handler in app.ts) so they invoke
guardedQuit() instead of calling app.quit() directly, and modify the before-quit
listener (which currently removes the window close listener) to stop removing
the close handler before upload checks run — alternatively move the upload check
into the before-quit flow so the check runs for all quit paths; ensure
references to guardedQuit, app.quit, the before-quit listener and the window
close handler are updated accordingly.
In `@src/uploads/main.ts`:
- Around line 34-44: getRootWindow() can reject when no window exists, so wrap
the call in a try/catch (or handle the rejection) and fall back to
undefined/null for the parent window before calling dialog.showMessageBoxSync;
this ensures the confirmation flow still runs instead of letting the promise
reject. Update the code that calls getRootWindow() (reference getRootWindow and
dialog.showMessageBoxSync in this module) to catch errors from getRootWindow(),
set rootWindow to undefined/null on failure, then call dialog.showMessageBoxSync
with that value and return the same boolean result (choice === 1).
In `@UPLOAD_TRACKING_FEATURE.md`:
- Around line 30-70: The documentation still describes counter-based tracking
(activeUploadsCount++, activeUploadsCount > 0 and underflow handling) but the
implementation uses a Set<string> (activeUploads) and dedupes by upload id via
notifyUploadStarted and notifyUploadFinished; update the diagrams and prose to
reflect this: replace counter increments/decrements with Set operations (add(id)
/ delete(id)), replace "activeUploadsCount > 0" checks with "activeUploads.size
> 0" or "activeUploads not empty", and remove any mentions of counter underflow
or numeric race conditions; also update the Close Prevention Flow to state
membership/size-based checks and reference the actual handlers
notifyUploadStarted, notifyUploadFinished and checkActiveUploads.
---
Outside diff comments:
In `@src/ui/main/rootWindow.ts`:
- Around line 271-305: Remove the initial await checkActiveUploads() at the top
of the close listener and instead only call await checkActiveUploads() on the
path that actually performs a quit (i.e., just before guardedQuit()). Keep the
full-screen exit (rootWindow.isFullScreen / leave-full-screen /
setFullScreen(false)) and blur behavior as-is, evaluate isTrayIconEnabled and
isMinimizeOnCloseEnabled to decide hide/minimize, and if those conditions would
not short-circuit (hide or minimize), then call checkActiveUploads(); if it
returns false, return; otherwise call guardedQuit(); this prevents prompting on
hide/minimize and avoids a double prompt before guardedQuit().
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eea27b2b-ba89-43ae-b72c-91f97572721b
📒 Files selected for processing (3)
UPLOAD_TRACKING_FEATURE.mdsrc/ui/main/rootWindow.tssrc/uploads/main.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/uploads/main.tssrc/ui/main/rootWindow.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
📚 Learning: 2026-03-11T06:38:40.426Z
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
Applied to files:
src/ui/main/rootWindow.ts
🔇 Additional comments (1)
src/uploads/main.ts (1)
9-27: Nice change: upload tracking is now keyed by upload id.Using a
Set<string>here avoids the drift problems from duplicate start/finish events and keeps concurrent uploads isolated cleanly.
| export const guardedQuit = async (): Promise<void> => { | ||
| const canQuit = await checkActiveUploads(); | ||
| if (canQuit) { | ||
| app.quit(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP --type=ts '\bapp\.quit\s*\(' \
src/ui/main/rootWindow.ts \
src/ui/main/menuBar.ts \
src/ui/main/trayIcon.ts \
src/app/main/app.ts
echo
rg -n -C2 --type=ts 'before-quit|removeAllListeners\(|guardedQuit|checkActiveUploads' \
src/ui/main/rootWindow.tsRepository: RocketChat/Rocket.Chat.Electron
Length of output: 1326
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check menuBar.ts quit path
echo "=== menuBar.ts context ==="
rg -n -B5 -A5 'app\.quit\(' src/ui/main/menuBar.ts | head -20
echo
echo "=== trayIcon.ts context ==="
rg -n -B5 -A5 'app\.quit\(' src/ui/main/trayIcon.ts | head -20
echo
echo "=== app.ts context ==="
rg -n -B5 -A5 'app\.quit\(' src/app/main/app.ts | head -20
echo
echo "=== Checking if close listener prevents window close when canClose=false ==="
sed -n '271,310p' src/ui/main/rootWindow.tsRepository: RocketChat/Rocket.Chat.Electron
Length of output: 2093
guardedQuit() and upload checks are bypassed by three direct quit paths.
Cmd/Ctrl+Q (menuBar.ts:118), tray quit (trayIcon.ts:156), and window-all-closed (app.ts:126) all call app.quit() directly. This triggers the before-quit listener at line 409, which removes the window's close listener before the upload check in the close handler (line 273) can run. Only window close via UI (clicking the X button) reaches the close listener; direct app.quit() paths skip both the upload prompt and guardedQuit() entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ui/main/rootWindow.ts` around lines 22 - 27, Three direct calls to
app.quit() bypass the upload-check flow; replace those direct quits (the
Cmd/Ctrl+Q handler in menuBar.ts, the tray quit in trayIcon.ts, and the
window-all-closed handler in app.ts) so they invoke guardedQuit() instead of
calling app.quit() directly, and modify the before-quit listener (which
currently removes the window close listener) to stop removing the close handler
before upload checks run — alternatively move the upload check into the
before-quit flow so the check runs for all quit paths; ensure references to
guardedQuit, app.quit, the before-quit listener and the window close handler are
updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
UPLOAD_TRACKING_FEATURE.md (1)
165-186:⚠️ Potential issue | 🟡 MinorKeep the tracking model consistent throughout the doc.
Earlier sections describe Set-based tracking (
add(id),delete(id),size > 0), but these sections switch back to a numeric counter, underflow handling, andactiveUploadsCount. That makes the design doc self-contradictory and easy to implement against the wrong model.Also applies to: 209-245, 275-292
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@UPLOAD_TRACKING_FEATURE.md` around lines 165 - 186, The doc mixes two tracking models—Set-based operations (add(id), delete(id), size) and a numeric counter (activeUploadsCount, increment/decrement on UPLOAD_STARTED/UPLOAD_FINISHED, underflow logic)—causing contradictions; standardize on one model across the file by updating tests, examples, and references to match the chosen model (either convert tests and functions like UPLOAD_STARTED/UPLOAD_FINISHED and checkActiveUploads to operate on a Set with add(id)/delete(id)/size > 0, or change earlier sections to describe a numeric counter and remove Set APIs), and ensure all mentions (e.g., activeUploads, activeUploadsCount, checkActiveUploads) use the same API and semantics throughout the doc.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/main/rootWindow.ts`:
- Around line 271-278: The close handler deadlocks because it awaits
'leave-full-screen' before calling rootWindow.setFullScreen(false); change the
sequence in the rootWindow.addListener('close', ...) branch that checks
rootWindow.isFullScreen(): first call rootWindow.setFullScreen(false), then
preventDefault() and (optionally) await the once('leave-full-screen') event
before letting the close continue (or call rootWindow.close() after the event),
so the window actually exits fullscreen and the close path can complete.
---
Duplicate comments:
In `@UPLOAD_TRACKING_FEATURE.md`:
- Around line 165-186: The doc mixes two tracking models—Set-based operations
(add(id), delete(id), size) and a numeric counter (activeUploadsCount,
increment/decrement on UPLOAD_STARTED/UPLOAD_FINISHED, underflow logic)—causing
contradictions; standardize on one model across the file by updating tests,
examples, and references to match the chosen model (either convert tests and
functions like UPLOAD_STARTED/UPLOAD_FINISHED and checkActiveUploads to operate
on a Set with add(id)/delete(id)/size > 0, or change earlier sections to
describe a numeric counter and remove Set APIs), and ensure all mentions (e.g.,
activeUploads, activeUploadsCount, checkActiveUploads) use the same API and
semantics throughout the doc.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29c83a5c-610c-4160-9e7c-554e57fa54cc
📒 Files selected for processing (5)
UPLOAD_TRACKING_FEATURE.mdsrc/ui/main/menuBar.tssrc/ui/main/rootWindow.tssrc/ui/main/trayIcon.tssrc/uploads/main.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/uploads/main.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/ui/main/rootWindow.tssrc/ui/main/menuBar.tssrc/ui/main/trayIcon.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
📚 Learning: 2026-03-11T06:38:40.426Z
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
Applied to files:
src/ui/main/rootWindow.ts
🔇 Additional comments (2)
src/ui/main/trayIcon.ts (1)
155-156: Nice centralization of the tray quit path.Routing the tray menu through
guardedQuit()keeps the upload confirmation in the same quit flow as the other entry points.src/ui/main/menuBar.ts (1)
117-118: Nice: Cmd/Ctrl+Q now uses the guarded quit flow.This makes the menu shortcut respect the active-upload confirmation instead of quitting unconditionally.
| rootWindow.addListener('close', async (event) => { | ||
| if (rootWindow?.isFullScreen()) { | ||
| await new Promise<void>((resolve) => | ||
| rootWindow.once('leave-full-screen', () => resolve()) | ||
| ); | ||
| rootWindow.setFullScreen(false); | ||
| event.preventDefault(); | ||
| return; |
There was a problem hiding this comment.
Don't deadlock the fullscreen close path.
Line 273 waits for leave-full-screen before calling setFullScreen(false), so this branch never resolves and the window can't close from fullscreen.
Suggested fix
rootWindow.addListener('close', async (event) => {
if (rootWindow?.isFullScreen()) {
- await new Promise<void>((resolve) =>
- rootWindow.once('leave-full-screen', () => resolve())
- );
- rootWindow.setFullScreen(false);
event.preventDefault();
+ rootWindow.setFullScreen(false);
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ui/main/rootWindow.ts` around lines 271 - 278, The close handler
deadlocks because it awaits 'leave-full-screen' before calling
rootWindow.setFullScreen(false); change the sequence in the
rootWindow.addListener('close', ...) branch that checks
rootWindow.isFullScreen(): first call rootWindow.setFullScreen(false), then
preventDefault() and (optionally) await the once('leave-full-screen') event
before letting the close continue (or call rootWindow.close() after the event),
so the window actually exits fullscreen and the close path can complete.
1 participant
Fix #3261
Description
This PR prevents the Rocket.Chat Desktop application from closing while a file upload is in progress.
If the user attempts to close the application during an active upload, a confirmation dialog appears asking whether they want to quit or cancel.
Changes
How to Test
Expected result:
A confirmation dialog appears warning that a file upload is still in progress.
Fixes #ISSUE_NUMBER
Summary by CodeRabbit
New Features
Bug Fixes
Chores