feat(downloads): rework tab bar downloads indicator - #3443
Conversation
- replace the progress pill with an icon button matching the meatball menu button style - move the indicator to the right of the update label (macOS/Linux), next to the meatball menu - show a single averaged percentage for concurrent downloads instead of a count plus percent - gate the percentage text behind a new persisted setting (isDownloadsPercentageEnabled, default on) with a General settings toggle - show an unseen-completed dot on the button until the popup is opened
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesThe change adds a persisted Downloads features
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DeveloperMenu
participant Store
participant DownloadsMain
participant DownloadsIndicator
DeveloperMenu->>Store: Dispatch DOWNLOADS_SIMULATION_REQUESTED
Store->>DownloadsMain: Notify simulation listener
DownloadsMain->>Store: Dispatch progress and completion updates
Store->>DownloadsIndicator: Provide download state
DownloadsIndicator->>DownloadsIndicator: Calculate and render progress
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 3
🤖 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/i18n/de-DE.i18n.json`:
- Line 220: Correct the spelling in the description value by replacing
“Fortschrittsprozenzsatz” with “Fortschrittsprozentsatz”, leaving the
surrounding translation unchanged.
In `@src/ui/components/TopBar/DownloadsIndicator.spec.tsx`:
- Around line 446-468: Update the test around DownloadsIndicator so it initially
renders with no downloads, then dispatches or otherwise updates the store after
mount with a completed download whose endTime is the actual current/past
completion timestamp. Preserve the assertions that the unseen dot appears before
opening the popup and is cleared after clicking the downloads button.
In `@src/ui/components/TopBar/DownloadsIndicator.tsx`:
- Around line 180-188: Update handleToggle so the setIsOpen updater only
computes and returns the next open state; move the setSeenAt(Date.now()) call
outside that updater and trigger it based on the resulting isOpen transition,
ensuring timestamp updates occur once when the indicator opens.
🪄 Autofix
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 Plus
Run ID: 91996ce7-fe4a-431f-b4f4-eec11cdcf893
📒 Files selected for processing (22)
src/app/PersistableValues.tssrc/app/selectors.tssrc/i18n/de-DE.i18n.jsonsrc/i18n/en.i18n.jsonsrc/i18n/es.i18n.jsonsrc/i18n/fi.i18n.jsonsrc/i18n/fr.i18n.jsonsrc/i18n/hu.i18n.jsonsrc/i18n/no.i18n.jsonsrc/i18n/pt-BR.i18n.jsonsrc/i18n/ru.i18n.jsonsrc/i18n/sv.i18n.jsonsrc/i18n/zh-CN.i18n.jsonsrc/store/rootReducer.tssrc/ui/actions.tssrc/ui/components/SettingsView/GeneralTab.tsxsrc/ui/components/SettingsView/features/DownloadsPercentage.tsxsrc/ui/components/Shell/index.tsxsrc/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/components/TopBar/DownloadsIndicator.tsxsrc/ui/reducers/__tests__/featureFlags.spec.tssrc/ui/reducers/isDownloadsPercentageEnabled.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; create custom components only when Fuselage lacks the required functionality.
Import Fuselage components from@rocket.chat/fuselage.
Use only valid color tokens documented byTheme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such asprocess.getuid(),getgid(),geteuid(), andgetegid().
Use TypeScript strict mode.
Redux actions must follow the Flux Standard Action pattern.
Use camelCase for file names and PascalCase for component names.
Avoid unnecessary comments; prefer self-documenting code through clear naming.
Do not commit or push without explicit user permission.
Verify library APIs, props, tokens, and types against official documentation and.d.tsfiles instead of assuming they are valid.
Files:
src/ui/components/SettingsView/GeneralTab.tsxsrc/ui/reducers/isDownloadsPercentageEnabled.tssrc/ui/actions.tssrc/ui/components/SettingsView/features/DownloadsPercentage.tsxsrc/ui/components/Shell/index.tsxsrc/app/selectors.tssrc/app/PersistableValues.tssrc/store/rootReducer.tssrc/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/components/TopBar/DownloadsIndicator.tsxsrc/ui/reducers/__tests__/featureFlags.spec.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React functional components with hooks.
Files:
src/ui/components/SettingsView/GeneralTab.tsxsrc/ui/components/SettingsView/features/DownloadsPercentage.tsxsrc/ui/components/Shell/index.tsxsrc/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/components/TopBar/DownloadsIndicator.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs use
*.spec.ts/*.spec.tsx.
Files:
src/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/reducers/__tests__/featureFlags.spec.ts
src/**/*.{spec.ts,spec.tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Renderer test files should be placed in nested module paths such as
src/<module>/<subdir>/*.spec.ts(x)so Jest discovers them.
Files:
src/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/reducers/__tests__/featureFlags.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
*.spec.tsfor renderer process tests.
Files:
src/ui/reducers/__tests__/featureFlags.spec.ts
🧠 Learnings (5)
📚 Learning: 2026-06-26T18:14:15.295Z
Learnt from: jeanfbrito
Repo: RocketChat/Rocket.Chat.Electron PR: 3358
File: src/i18n/it-IT.i18n.json:39-42
Timestamp: 2026-06-26T18:14:15.295Z
Learning: In the i18n JSON files, the translation key `minimizeOnClose.disabledHint` is intentionally displayed when `isTrayIconEnabled` is true and the minimize-on-close toggle is disabled. The hint text should therefore instruct the user to disable the tray icon to make the setting available. During reviews, don’t “correct” this translation for seeming mismatches with the toggle state—first confirm it matches the component’s intended behavior; only update the wording if the underlying product logic/UX requirement changes.
Applied to files:
src/i18n/pt-BR.i18n.jsonsrc/i18n/no.i18n.jsonsrc/i18n/ru.i18n.jsonsrc/i18n/es.i18n.jsonsrc/i18n/fi.i18n.jsonsrc/i18n/de-DE.i18n.jsonsrc/i18n/zh-CN.i18n.jsonsrc/i18n/en.i18n.jsonsrc/i18n/hu.i18n.jsonsrc/i18n/fr.i18n.jsonsrc/i18n/sv.i18n.json
📚 Learning: 2026-05-19T20:49:24.859Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat.Electron PR: 3329
File: src/ui/reducers/e2ePdfPreviewSizeLimit.ts:14-16
Timestamp: 2026-05-19T20:49:24.859Z
Learning: In Rocket.Chat.Electron’s reducer files under src/ui/reducers/, reducers should not re-implement validation for action payloads. Assume the caller (UI component or dispatch site) has already validated the action payload and type/shape; reducers should trust the payload and update state directly. If validation is needed, add it at the dispatch site/caller rather than inside the reducer.
Applied to files:
src/ui/reducers/isDownloadsPercentageEnabled.tssrc/ui/reducers/__tests__/featureFlags.spec.ts
📚 Learning: 2026-05-19T20:49:24.859Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat.Electron PR: 3329
File: src/ui/reducers/e2ePdfPreviewSizeLimit.ts:14-16
Timestamp: 2026-05-19T20:49:24.859Z
Learning: In the Rocket.Chat.Electron UI reducers under src/ui/reducers/, do not add/repeat input validation for action payloads inside reducers. Follow the existing codebase pattern: validate the action payload in the caller (e.g., the UI component or dispatch site) before dispatching. Reducers should trust the incoming payload and apply it directly to state. If adding/updating a reducer, ensure the corresponding caller performs the necessary validation (e.g., check numeric constraints like !isNaN(value) && value > 0 before dispatching the action).
Applied to files:
src/ui/reducers/isDownloadsPercentageEnabled.tssrc/ui/reducers/__tests__/featureFlags.spec.ts
📚 Learning: 2026-06-26T18:14:11.817Z
Learnt from: jeanfbrito
Repo: RocketChat/Rocket.Chat.Electron PR: 3358
File: src/ui/components/SettingsView/features/E2ePdfPreviewSizeLimit.tsx:47-55
Timestamp: 2026-06-26T18:14:11.817Z
Learning: In the Rocket.Chat Electron App SettingsView features under `src/ui/components/SettingsView/features/`, treat full-width selects/inputs (including full-width numeric inputs) as intentional for the stacked label/description layout. Per the UXDQA Figma spec (and macOS 1:1 verification), reviews should not flag these as layout regressions as long as they match the expected form-column stretching behavior.
Applied to files:
src/ui/components/SettingsView/features/DownloadsPercentage.tsx
📚 Learning: 2026-06-26T18:14:13.838Z
Learnt from: jeanfbrito
Repo: RocketChat/Rocket.Chat.Electron PR: 3358
File: src/ui/components/SettingsView/features/ToggleField.tsx:1-8
Timestamp: 2026-06-26T18:14:13.838Z
Learning: In Rocket.Chat Electron App settings field UIs that use the Fuselage three-tier pattern, keep the `FieldLabel` / `FieldDescription` / `FieldHint` structure separate. Use `FieldDescription` for the regular secondary body text, and reserve `FieldHint` for the smaller, dimmer subline content (e.g., restart caveats). Do not collapse `FieldDescription` and `FieldHint` into a single hint tier, as this violates the intended UXDQA spec.
Applied to files:
src/ui/components/SettingsView/features/DownloadsPercentage.tsx
🪛 React Doctor (0.9.3)
src/ui/components/TopBar/DownloadsIndicator.tsx
[error] 181-181: This state updater performs the nested state update "setSeenAt()". React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.
Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.
(no-impure-state-updater)
[error] 184-184: This side-effecting call runs inside a state updater, which React may invoke more than once. Move it outside the setter after computing the next state.
React may replay a state updater, so callbacks, analytics, and persistence inside it can run more than once. Compute state purely, then perform the side effect outside the setter.
(no-side-effect-in-state-updater-function)
🔇 Additional comments (22)
src/app/PersistableValues.ts (1)
125-131: LGTM!Also applies to: 249-252
src/store/rootReducer.ts (1)
35-35: LGTM!Also applies to: 121-121
src/ui/components/SettingsView/GeneralTab.tsx (1)
4-4: LGTM!Also applies to: 36-36
src/ui/components/SettingsView/features/DownloadsPercentage.tsx (1)
1-46: LGTM!src/i18n/en.i18n.json (1)
271-274: LGTM!src/i18n/es.i18n.json (1)
237-240: LGTM!src/i18n/fi.i18n.json (1)
225-228: LGTM!src/i18n/fr.i18n.json (1)
225-228: LGTM!src/i18n/hu.i18n.json (1)
250-253: LGTM!src/i18n/no.i18n.json (1)
257-260: LGTM!src/ui/actions.ts (1)
131-132: LGTM!Also applies to: 275-275
src/ui/reducers/isDownloadsPercentageEnabled.ts (1)
1-24: LGTM!src/app/selectors.ts (1)
41-42: LGTM!src/ui/reducers/__tests__/featureFlags.spec.ts (1)
12-12: LGTM!Also applies to: 167-210
src/i18n/pt-BR.i18n.json (1)
232-235: LGTM!src/i18n/ru.i18n.json (1)
219-222: LGTM!src/i18n/sv.i18n.json (1)
246-249: LGTM!src/i18n/zh-CN.i18n.json (1)
161-164: LGTM!src/ui/components/TopBar/DownloadsIndicator.tsx (2)
34-86: LGTM!Also applies to: 131-152, 190-192, 204-212
2-2: 📐 Maintainability & Code QualityNo API changes needed.
Fuselage
0.80.0declaresIconButtonPropswithIconButtonSize,icon, andRefAttributes<HTMLElement>, so themedium,icon='download', andref={reference}usage matches the installed declarations.src/ui/components/Shell/index.tsx (1)
91-92: LGTM!Also applies to: 105-106
src/ui/components/TopBar/DownloadsIndicator.spec.tsx (1)
68-71: LGTM!Also applies to: 394-444
| }, | ||
| "downloadsPercentage": { | ||
| "title": "Download-Prozentsatz", | ||
| "description": "Zeigt den Download-Fortschrittsprozenzsatz in der Tab-Leiste an." |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the German spelling error.
Replace Fortschrittsprozenzsatz with Fortschrittsprozentsatz.
Proposed correction
- "description": "Zeigt den Download-Fortschrittsprozenzsatz in der Tab-Leiste an."
+ "description": "Zeigt den Download-Fortschrittsprozentsatz in der Tab-Leiste an."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "description": "Zeigt den Download-Fortschrittsprozenzsatz in der Tab-Leiste an." | |
| "description": "Zeigt den Download-Fortschrittsprozentsatz in der Tab-Leiste an." |
🤖 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/i18n/de-DE.i18n.json` at line 220, Correct the spelling in the
description value by replacing “Fortschrittsprozenzsatz” with
“Fortschrittsprozentsatz”, leaving the surrounding translation unchanged.
| const handleToggle = (): void => { | ||
| setIsOpen((current) => { | ||
| const next = !current; | ||
| if (next) { | ||
| setSeenAt(Date.now()); | ||
| } | ||
| return next; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Official React documentation: Are useState updater functions required to be pure, and can React invoke an updater function more than once?
💡 Result:
According to the official React documentation, yes, useState updater functions are required to be pure [1][2]. An updater function must take the pending state as its only argument and return the next state without performing any side effects [1][2]. Regarding invocation, React may indeed call your updater function more than once [1][3]. Specifically, in development mode with Strict Mode enabled, React will call your updater function twice to help identify accidental impurities [1][3]. React ignores the result of one of these calls and uses the other, provided the function is pure and produces the same result regardless of how many times it is executed [1][4]. This behavior is development-only and does not affect production [1][3].
Citations:
- 1: https://react.dev/reference/react/useState
- 2: https://react.dev/learn/queueing-a-series-of-state-updates
- 3: https://uk.react.dev/reference/react/useState
- 4: https://react.dev/learn/keeping-components-pure
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file around the reported updater usage.
if [ -f "src/ui/components/TopBar/DownloadsIndicator.tsx" ]; then
echo "== file exists =="
wc -l src/ui/components/TopBar/DownloadsIndicator.tsx
echo "== relevant lines 120-220 =="
sed -n '1,260p' src/ui/components/TopBar/DownloadsIndicator.tsx | nl -ba | sed -n '150,225p'
echo "== state/effect/toggle summary =="
rg -n "useState|useEffect|handleToggle|setSeenAt|setIsOpen|seenAt|isOpen" src/ui/components/TopBar/DownloadsIndicator.tsx
else
echo "file missing"
fiRepository: RocketChat/Rocket.Chat.Electron
Length of output: 308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== import/hooks/open state pattern =="
grep -E -n "useState|useEffect|handleToggle|setSeenAt|setIsOpen|seenAt|isOpen" src/ui/components/TopBar/DownloadsIndicator.tsx || true
echo "== relevant lines 160-200 =="
awk 'NR>=160 && NR<=200 { printf "%6d %s\n", NR, $0 }' src/ui/components/TopBar/DownloadsIndicator.tsx
echo "== relevant imports =="
sed -n '1,20p' src/ui/components/TopBar/DownloadsIndicator.tsxRepository: RocketChat/Rocket.Chat.Electron
Length of output: 3180
Keep setIsOpen updaters side-effect free.
setIsOpen((current) => { ... setSeenAt(Date.now()); ... }) queues setSeenAt inside a React state updater. React can run updaters more than once, so this can queue duplicate updates for the same toggle. Update seenAt after isOpen changes instead.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 181-181: This state updater performs the nested state update "setSeenAt()". React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.
Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.
(no-impure-state-updater)
[error] 184-184: This side-effecting call runs inside a state updater, which React may invoke more than once. Move it outside the setter after computing the next state.
React may replay a state updater, so callbacks, analytics, and persistence inside it can run more than once. Compute state purely, then perform the side effect outside the setter.
(no-side-effect-in-state-updater-function)
🤖 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/ui/components/TopBar/DownloadsIndicator.tsx` around lines 180 - 188,
Update handleToggle so the setIsOpen updater only computes and returns the next
open state; move the setSeenAt(Date.now()) call outside that updater and trigger
it based on the resulting isOpen transition, ensuring timestamp updates occur
once when the indicator opens.
Source: Linters/SAST tools
PanelLayer was position: relative, so on popup mount it joined the tab bar Strip's flex flow as a zero-width item and its gap pushed the button a few pixels left. position: fixed takes it out of the flow.
SVG arc circumscribing the icon glyph, driven by stroke-dashoffset with a 200ms transition; indeterminate downloads (unknown total size) show a spinning quarter arc. The ring always shows while downloading — the settings toggle only controls the percentage text.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/ui/components/TopBar/DownloadsIndicator.tsx (1)
262-283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove hard-coded color fallbacks after token verification.
The fallback hex values can diverge from themed colors. Verify both CSS variables in
Theme.d.ts, then use the documented tokens without raw color fallbacks.#!/bin/bash set -euo pipefail fd -i 'Theme\.d\.ts$' . -0 | xargs -0 -r rg -n 'stroke-extra-light|font-info'Proposed fix
- stroke='var(--rcx-color-stroke-extra-light, `#ebecef`)' + stroke='var(--rcx-color-stroke-extra-light)' ... - stroke='var(--rcx-color-font-info, `#095ad2`)' + stroke='var(--rcx-color-font-info)'As per coding guidelines, “Use only valid color tokens documented by
Theme.d.ts” and “Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.”🤖 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/ui/components/TopBar/DownloadsIndicator.tsx` around lines 262 - 283, Update the two circle elements in the DownloadsIndicator progress ring to use the documented CSS color tokens verified in Theme.d.ts, removing the raw `#ebecef` and `#095ad2` fallback values from their stroke declarations. Preserve the existing token names and all other SVG attributes unchanged.
🤖 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/ui/components/TopBar/DownloadsIndicator.tsx`:
- Around line 187-192: Update isIndeterminate in
src/ui/components/TopBar/DownloadsIndicator.tsx:187-192 to use some so any
active download with totalBytes <= 0 selects indeterminate mode, while retaining
the empty-download guard. Add a mixed known-size and unknown-size download test
in src/ui/components/TopBar/DownloadsIndicator.spec.tsx:488-510 that asserts the
ring uses the indeterminate offset.
---
Nitpick comments:
In `@src/ui/components/TopBar/DownloadsIndicator.tsx`:
- Around line 262-283: Update the two circle elements in the DownloadsIndicator
progress ring to use the documented CSS color tokens verified in Theme.d.ts,
removing the raw `#ebecef` and `#095ad2` fallback values from their stroke
declarations. Preserve the existing token names and all other SVG attributes
unchanged.
🪄 Autofix
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 Plus
Run ID: 63adc488-63e1-41f8-b642-ee521e6e2a25
📒 Files selected for processing (2)
src/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/components/TopBar/DownloadsIndicator.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; create custom components only when Fuselage lacks the required functionality.
Import Fuselage components from@rocket.chat/fuselage.
Use only valid color tokens documented byTheme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such asprocess.getuid(),getgid(),geteuid(), andgetegid().
Use TypeScript strict mode.
Redux actions must follow the Flux Standard Action pattern.
Use camelCase for file names and PascalCase for component names.
Avoid unnecessary comments; prefer self-documenting code through clear naming.
Do not commit or push without explicit user permission.
Verify library APIs, props, tokens, and types against official documentation and.d.tsfiles instead of assuming they are valid.
Files:
src/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/components/TopBar/DownloadsIndicator.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs use
*.spec.ts/*.spec.tsx.
Files:
src/ui/components/TopBar/DownloadsIndicator.spec.tsx
src/**/*.{spec.ts,spec.tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Renderer test files should be placed in nested module paths such as
src/<module>/<subdir>/*.spec.ts(x)so Jest discovers them.
Files:
src/ui/components/TopBar/DownloadsIndicator.spec.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React functional components with hooks.
Files:
src/ui/components/TopBar/DownloadsIndicator.spec.tsxsrc/ui/components/TopBar/DownloadsIndicator.tsx
🔇 Additional comments (3)
src/ui/components/TopBar/DownloadsIndicator.tsx (2)
230-238: Keep thesetIsOpenupdater pure.This still calls
setSeenAtinside asetIsOpenupdater. React can invoke an updater more than once. Move the timestamp update into the click handler before setting the next open state.React 19 official documentation: must useState updater functions be pure, and can Strict Mode invoke them more than once?
43-83: LGTM!Also applies to: 120-129
src/ui/components/TopBar/DownloadsIndicator.spec.tsx (1)
430-467: LGTM!Also applies to: 469-486
The track circle rendered as a prominent full ring that visually drowned the progress arc, reading as a static circle. currentColor at 0.2 opacity keeps it faint in any theme so the blue percentage arc is what stands out.
…leanup - 'Simulate Download' developer-mode menu item mirroring the update simulation: replays the real DOWNLOAD_CREATED/UPDATED/completed lifecycle for two staggered fake downloads, demoing the averaged percentage, progress ring, and unseen-completed dot - re-triggering removes the previous simulation's entries - on startup, persisted downloads still marked progressing/paused with no live DownloadItem are marked interrupted (they can never resume), preventing a permanently frozen active indicator after an app quit mid-download
The button group is right-anchored in the strip, so text on the right pushed the icon leftward as the number widened. On the left the text grows leftward and the icon stays put. Also anchors the unseen dot to the button container instead of the outer wrapper.
Adds variant='chrome' to DownloadsIndicator: Material arrow-with-tray glyph at rest; while downloading the arrow shrinks inside a progress arc with a gentle bounce and the tray becomes an accent dash, matching Chrome's toolbar animation. Rendered side by side with the default variant only when developer mode is enabled, so the two designs can be compared with the simulate-download flow before picking one.
- glyph color comes from the Fuselage font token instead of inherit, which resolved to black in dark theme - the ring group's unconditional CSS transform-origin stacked onto the origin already baked into the rotate(-90 12 10) attribute transform, displacing the circles below the viewBox where they were clipped — the rotation now lives on the arc circle alone and the indeterminate spin uses transform-box: fill-box to self-center - downloading arrow uses the theme color like Chrome; accent stays on the arc and tray
…downloading The chrome variant's svg is now 28px in all states so the full-size arrow fits inside the same ring geometry the default variant uses (r=12, stroke 2); the tray bar renders only in the idle rest glyph.
Uses the same color resolution chain Fuselage applies to icon buttons so both variants' arrows always render identically, and removes the arrow bounce animation — the glyph stays still while downloading.
The chrome glyph's circles lived inside the button element, which the tab bar wrapper dims to 0.6 opacity at rest — the ring looked translucent next to the default variant's full-opacity overlay. Both variants now render the same ProgressRing overlay outside the button; the chrome svg is back to a 24px glyph (arrow only while downloading, arrow+tray at rest).
variant='fuselage' keeps the stock download IconButton and overlays only an accent arc positioned exactly on the glyph's built-in circle (annulus r11-13 in 32-space -> r9, stroke 1.5 at the 24px icon scale), so the icon's circle doubles as the progress track. ProgressRing is generalized to take a size prop; dev-mode comparison row is now chrome -> fuselage -> ring.
variant='redraw' rebuilds the download icon from its SVG source: the arrow subpath verbatim, and the circle natively split into a subtle track (currentColor at 0.2, dark in dark theme) plus the accent arc. The button un-dims while downloading (specificity override over the tab bar wrapper's 0.6 rest opacity) so the drawn circle renders at full vibrancy; at rest it draws the original glyph verbatim and dims like any other tab bar button. Dev comparison row is now chrome -> fuselage -> redraw -> ring.
The developer-mode Simulate Update Flow / Simulate Download items lived only in the Help menu, unreachable from the tab bar's meatball popup (which on macOS is a reduced menu). The item definitions are now a shared factory used by both menus, appended after Check for Updates in the popup on all platforms when developer mode is enabled.
…er menu The titlebar's App settings dropdown (selectServerSwitcherMenuTemplate) now includes Check for Updates after Add new server, plus the shared developer-mode simulate items behind a separator.
The TopBar titlebar is 28px on macOS (32px Windows) while the indicators were sized for the taller tab bar strip (32px buttons, 28px ring, dot overhanging the top) and got clipped. A compact prop, passed only by the TopBar layouts, scales all four variants by 0.75: tiny 24px buttons, 21px/18px rings, 6px dot kept inside the button box, and slightly smaller percentage text.
…bar spacing Shell now renders a single redraw-variant indicator in every layout (the dev-mode comparison row is gone; the other variant implementations remain in the component for now). The TopBar trailing slot gets a flex wrapper with a 6px gap and, on macOS, 10px of end padding so the indicator no longer hugs the window's rounded corner — Windows stays flush for the window controls.
status-background-success is a pale badge-background token that washes out on a light titlebar; the status-bullet-online green is designed for a solid dot and reads on both themes.
Review feedback pass: - the three experimental variants (chrome/fuselage/ring) and the variant prop are removed; the redrawn glyph is the indicator, with identifiers renamed accordingly - the percentage is now text inside the button, so hover and click cover text and glyph as one control - unseen dot moved 2px down and left, anchored to the glyph box, and colored with the Fuselage badge blue (badge-background-level-2) instead of the presence green - the macOS titlebar right margin is removed
Same font-info token as the arc, so the dot and the downloading circle stay one color in every theme.
…dged The full-opacity override only applied while downloading, so a finished download instantly fell back to the tab bar's 0.6 idle dimming, darkening the full circle and the dot. A third data-downloads-status value, 'unseen', keeps the button at full brightness from completion until the user opens the popup — matching Chrome's behavior.
The Download percentage settings toggle now defaults to disabled; the setting is new in this branch so no migration compat is needed.
…he ring - a finished download keeps the circle rendered as the accent arc at 100% (same font-info token, so it theme-tracks) until the user opens the popup; only then does the glyph revert to the plain idle drawing - the full-size unseen dot moved to the ring's tangent point (offset 1px) matching the compact placement; the indeterminate spin now only runs while actually downloading
…percent The TopBar trailing-slot wrapper (gap 6px) is removed so both bars use the Strip's own 3px gap, and the percentage reserves min-width 3ch — the same mechanism UpdateLabel uses — so the button width holds steady between one and two digits.
…button Voice-note feedback round: - the percentage renders to the right of the glyph (icon left, text right) - the text slot is always mounted and animates max-width/margin/opacity at Fuselage's micro-interaction speed (0.18s, the .rcx-box--animated standard) with a prefers-reduced-motion guard, so the button grows when a download starts and contracts when it finishes - icon-only buttons are exactly square (24x24 compact, 32x32 full); the compact size drops the horizontal padding that made it 32x24
Compact drops the button's horizontal padding to stay square when icon-only, which left the expanded text flush against the button edge. The animated slot now carries its own trailing margin in compact expanded state, collapsing with the rest of the animation.
* docs: desktop UI guidelines and dev-app-verify skill Distills the lessons from the downloads indicator work (PRs #3441 and #3443) into permanent project knowledge: - docs/desktop-ui-guidelines.md: Fuselage token semantics and traps (runtime-injected tokens, status-background vs bullet vs badge, the icon-button color chain), icon/button geometry facts, the .18s micro-interaction standard, tab bar button conventions (the 0.6 dimming rule and its override, square icon-only buttons, compact titlebar sizing, flex-flow traps), and the SVG transform-origin clipping pitfall - skills/dev-app-verify/SKILL.md: ready-made script to drive and screenshot the running dev app through the port-9339 inspector, with the three pitfalls that produce false alarms (occlusion, watcher restarts, singleton wedges) — plain Markdown, usable by any agent like skills/desktop-qa-flows - CLAUDE.md: pointers to both from the UI and Testing sections * docs: point AGENTS.md at the UI guidelines and dev-app-verify skill CLAUDE.md got the pointers but AGENTS.md — what non-Claude agents read — did not; both files now direct agents to docs/desktop-ui-guidelines.md from UI Work and to skills/dev-app-verify from Testing. * chore: share Claude Code skills with the repo The blanket .claude/ gitignore was hiding project knowledge: CLAUDE.md references .claude/skills/gitnexus/* six times, and skills like electron-build, ship-release, and boot-wedge-debug encode repo processes any Claude Code user needs. The ignore is now selective — only genuinely personal/machine state stays out (settings.local.json, mytasks/, worktrees/, rules/, commands/, scheduled_tasks.lock) — and the skills plus the hooks-only settings.json are committed. The dev-app-verify Claude stub points at the canonical skills/dev-app-verify/SKILL.md so the two copies cannot drift. * docs: AGENTS.md becomes the canonical agent guide, CLAUDE.md imports it The two files carried parallel, drifting versions of the same guidance that had to be edited twice. AGENTS.md is now the single source (union of both, restoring details each copy had dropped: KMS two-phase rationale, worktree commands, working principles, the screen-capture VM constraint, the GitNexus block with its tool-managed markers), and CLAUDE.md is a thin @AGENTS.md import with a marked spot for genuinely Claude-specific additions. * docs: CLAUDE.md stub directs writes to AGENTS.md explicitly The stub's guidance was an HTML comment a rushed session could ignore; it is now a visible imperative rule — loaded into every session's context — telling agents to record shared guidance in AGENTS.md and reserve this file for genuinely Claude-only content. * docs: reindexing guidance and gitnexus-interference pitfall - AGENTS.md Working Principles: reindex with --index-only at quiet points — a plain background analyze rewrites the tracked gitnexus doc blocks (stats churn) and mutates worktree git state (drops staged index entries, touches watched sources restarting the dev app) - dev-app-verify skill: the same interference documented as pitfall 4 * chore: read-only permission allowlist for agent verification loops Derived from transcript analysis (29 recent sessions): the DoD verification commands agents run constantly (yarn test/lint, npx tsc --noEmit, the gitnexus runner) and the read-only GitNexus/context-mode MCP tools that AGENTS.md mandates. No interpreters, task-runner wildcards, or mutating commands — those still prompt. * docs: Fuselage Select needs a react-aria-recognized label Salvaged from the retired OpenWolf buglog: Fuselage Select is backed by react-aria useSelect, which only accepts a visible label, aria-label, or aria-labelledby — a labeled wrapper does not satisfy it. * docs: address CodeRabbit review feedback - ship-release: semver-sorted baseline tags, worktree-pinned commands, approval gate before first push, fail-closed manual tag fallback, prerelease flag applied while still a draft - dev-app-verify: per-request CDP timeouts + pending rejection on socket loss, fail-closed menu trigger (developerMode gate + item checks), stable root-window predicate, macOS-only scope note - electron-bump: downgrade guard, unused-API list as hint not exclusion, drop unresolved PR placeholder from commit message - desktop-ui-guidelines: min-width 3ch wording, progress normalization note - AGENTS.md: drop point-in-time GitNexus counts - CLAUDE.md: route shared vs Claude-only guidance by content - markdown fence language identifiers across skill docs
…cator-improvements
…n Shell test The percentage slot's transition was gated behind a bare '@media (prefers-reduced-motion)' query, which matches whenever the browser supports the feature at all (true in headless Chromium CI), not only when the user has opted into reduced motion. That always disabled the transition, including for real users without the preference set. Switch to '(prefers-reduced-motion: reduce)' so the transition is on by default and only removed as an explicit opt-out. Also pin process.platform in the Shell 'regardless of developer mode' downloads-indicator test, matching every sibling test in the same describe block — it was the only one relying on the CI runner's actual platform, which is why it only failed on the Ubuntu runner (Shell's non-tabs TopBar branches are darwin/win32 only, a pre-existing split unrelated to developer mode).
The 0.18s micro-interaction transition test read getComputedStyle, which evaluates the '@media (prefers-reduced-motion: reduce)' rule against the runner's actual OS-level motion preference. GitHub's Windows and macOS CI VMs report 'reduce' (animations disabled at the OS level), so the computed transition there is legitimately 'none' — correct behavior, but an environment-dependent assertion. Ubuntu (and most local machines) report no preference, so the same test passed there. Assert against the parsed CSSOM instead: walk document.styleSheets for the emotion-generated class's base rule and its reduced-motion override rule. Both rules exist in the stylesheet regardless of whether the media query currently matches, so the assertion is deterministic in every environment, and it now also proves the reduced-motion opt-out exists (transition: none under prefers-reduced-motion: reduce) instead of only proving the happy path. Verified window.matchMedia mocking does not affect getComputedStyle's media-query evaluation (the two are independent in Electron/Chromium), confirming the prior assertion was unfixable from the test side without this change.
1 participant
What
Follow-up to #3441, reworking how the downloads indicator looks and behaves in the tab bar:
IconButtonstyled like the meatball menu button, sitting next to it on the right side of the tab bar (macOS/Linux). The update label now appears to its left.isDownloadsPercentageEnabled(default on) with a switch in General settings ("Download percentage"). When off, only the icon button shows.Why
The pill design read as a badge rather than a control, showing a download count next to a percentage was ambiguous, and there was no signal when a download finished in the background.
Implementation notes
PersistableValues_4_16_1type +>=4.16.1migration defaulting the setting to true.Verification
npx tsc --noEmit: 0 errors;yarn lint: pass.yarn test --runTestsByPathon DownloadsIndicator, Shell, and featureFlags specs: 3 suites, 81 tests, 0 failures — including new tests for averaged progress, hidden percentage when disabled, and the unseen-dot lifecycle.Summary by CodeRabbit