feat(updates): titlebar feedback for manual update checks, resilient silent checks, tab strip i18n - #3439
Conversation
Checking for updates from the menus or the About dialog previously gave no indication that anything happened. The titlebar update label now walks through the whole flow: a pulsing "Checking for updates…" pill while the check runs, a green "You're up to date" pill on completion, and the existing blue Update pill when a version is found. Update checks are also made resilient and quiet: - Failed checks retry (2s, 5s backoff) before giving up, riding out GitHub's intermittent empty responses on the release-metadata request that electron-updater misreports as "No published versions on GitHub" - Check failures are not actionable for users, so they resolve to the same state as "no update available"; details go to the log at warn level, including electron-updater's own error-level noise - The startup check no longer blocks the rest of the app setup on the GitHub roundtrip The new updateCheckStatus state is keyed on the user-initiated action rather than the autoUpdater event, so automatic startup checks never flash the titlebar.
Fuselage's Select is backed by react-aria's useSelect, which only recognizes label/aria-label/aria-labelledby — a FieldLabel htmlFor association or a placeholder alone triggers a "you must specify an aria-label" console warning on every render. Add aria-labels to the two Settings selects and the four log viewer filter selects.
The tabBar section (update pill, workspaces, unread tooltip, meatball menu, window controls) existed only in English; every other locale fell back. Add translations for the 16 registered locales, with proper plural forms for the unread count (one/few/many for Russian, Ukrainian and Polish; single form for Japanese and Chinese).
7d6c900 to
87c1a85
Compare
WalkthroughThe update system now retries failed checks, tracks manual-check status, and displays transient title-bar feedback. Localization files add tab-bar strings across supported languages. Selector controls in the log viewer and settings view now include accessible labels. ChangesUpdate check feedback
Accessible selector labels
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant UpdateLabel
participant UpdatesMain
participant Updater
User->>UpdateLabel: start manual update check
UpdateLabel->>UpdatesMain: request update check
UpdatesMain->>Updater: check for updates
Updater-->>UpdatesMain: return result or failure
UpdatesMain-->>UpdateLabel: publish check status
UpdateLabel-->>User: show transient feedback
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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
🧹 Nitpick comments (1)
src/ui/components/TopBar/UpdateLabel.spec.tsx (1)
54-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the update-check status parameter.
Line 62 accepts arbitrary strings. A misspelled status reaches
UpdateLabeland uses its default successful-outcome branch. TypeupdateCheckStatusasUpdateCheckStatusto keep test data aligned with the reducer contract.As per coding guidelines, "Use TypeScript strict mode."
🤖 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/UpdateLabel.spec.tsx` around lines 54 - 65, Update the checkState helper’s updateCheckStatus parameter to use the existing UpdateCheckStatus type instead of string, importing that type from its established module if needed. Preserve the current buildState behavior and overrides while ensuring invalid status values are rejected by TypeScript.Source: Coding guidelines
🤖 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/logViewerWindow/logViewerWindow.tsx`:
- Line 884: Update the server filter control’s aria-label near the existing
t('logViewer.filters.server.all') usage to reference a generic control-label
translation such as “Server filter,” while preserving the existing translation
key for the “all servers” option.
In `@src/ui/components/TopBar/UpdateLabel.tsx`:
- Around line 260-270: Update the checking-state branch in UpdateLabel so it
does not expose an inert focusable button: either disable the Label rendered
when updateCheckStatus === 'checking' or replace it with a non-button live
status element, while preserving the existing checking text and aria-live
behavior.
In `@src/updates/main.ts`:
- Around line 249-272: Replace the shared isUpdateCheckInFlight Boolean with one
shared in-flight Promise or equivalent mutex covering the entire
checkForUpdatesWithRetry retry ladder. Update the startup and manual check
paths, including the handlers around the update error events, to reuse the
existing operation instead of starting concurrent checks, and clear the shared
state only in finally after the owning Promise settles. Add a test that starts a
manual check while startup retry is waiting and verifies only one update check
runs.
---
Nitpick comments:
In `@src/ui/components/TopBar/UpdateLabel.spec.tsx`:
- Around line 54-65: Update the checkState helper’s updateCheckStatus parameter
to use the existing UpdateCheckStatus type instead of string, importing that
type from its established module if needed. Preserve the current buildState
behavior and overrides while ensuring invalid status values are rejected by
TypeScript.
🪄 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: f42bd6bb-78b0-496a-a951-d3e9f47fe6c1
📒 Files selected for processing (28)
src/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/it-IT.i18n.jsonsrc/i18n/ja.i18n.jsonsrc/i18n/no.i18n.jsonsrc/i18n/pl.i18n.jsonsrc/i18n/pt-BR.i18n.jsonsrc/i18n/ru.i18n.jsonsrc/i18n/sv.i18n.jsonsrc/i18n/tr-TR.i18n.jsonsrc/i18n/uk-UA.i18n.jsonsrc/i18n/zh-CN.i18n.jsonsrc/i18n/zh-TW.i18n.jsonsrc/logViewerWindow/logViewerWindow.tsxsrc/store/rootReducer.tssrc/ui/components/SettingsView/features/AvailableBrowsers.tsxsrc/ui/components/SettingsView/features/TelephonyServer.tsxsrc/ui/components/TopBar/UpdateLabel.spec.tsxsrc/ui/components/TopBar/UpdateLabel.tsxsrc/updates/actions.tssrc/updates/common.tssrc/updates/main.tssrc/updates/reducers.tssrc/updates/reducers/__tests__/updates.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (actions)
🧰 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/logViewerWindow/logViewerWindow.tsxsrc/updates/common.tssrc/ui/components/SettingsView/features/TelephonyServer.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.tsxsrc/ui/components/TopBar/UpdateLabel.spec.tsxsrc/store/rootReducer.tssrc/updates/actions.tssrc/updates/reducers/__tests__/updates.spec.tssrc/updates/reducers.tssrc/updates/main.tssrc/ui/components/TopBar/UpdateLabel.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React functional components with hooks.
Files:
src/logViewerWindow/logViewerWindow.tsxsrc/ui/components/SettingsView/features/TelephonyServer.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.tsxsrc/ui/components/TopBar/UpdateLabel.spec.tsxsrc/ui/components/TopBar/UpdateLabel.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs use
*.spec.ts/*.spec.tsx.
Files:
src/ui/components/TopBar/UpdateLabel.spec.tsxsrc/updates/reducers/__tests__/updates.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/UpdateLabel.spec.tsxsrc/updates/reducers/__tests__/updates.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
*.spec.tsfor renderer process tests.
Files:
src/updates/reducers/__tests__/updates.spec.ts
🧠 Learnings (3)
📚 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/zh-TW.i18n.jsonsrc/i18n/pt-BR.i18n.jsonsrc/i18n/hu.i18n.jsonsrc/i18n/sv.i18n.jsonsrc/i18n/pl.i18n.jsonsrc/i18n/no.i18n.jsonsrc/i18n/zh-CN.i18n.jsonsrc/i18n/tr-TR.i18n.jsonsrc/i18n/uk-UA.i18n.jsonsrc/i18n/de-DE.i18n.jsonsrc/i18n/fr.i18n.jsonsrc/i18n/ru.i18n.jsonsrc/i18n/it-IT.i18n.jsonsrc/i18n/es.i18n.jsonsrc/i18n/ja.i18n.jsonsrc/i18n/en.i18n.jsonsrc/i18n/fi.i18n.json
📚 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/TelephonyServer.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.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/TelephonyServer.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.tsx
🔇 Additional comments (31)
src/logViewerWindow/logViewerWindow.tsx (1)
845-845: LGTM!Also applies to: 863-863, 873-873
src/ui/components/SettingsView/features/AvailableBrowsers.tsx (1)
76-76: LGTM!src/ui/components/SettingsView/features/TelephonyServer.tsx (1)
72-72: LGTM!src/updates/common.ts (1)
37-44: LGTM!src/updates/actions.ts (1)
29-34: LGTM!Also applies to: 52-52
src/updates/reducers.ts (1)
10-11: LGTM!Also applies to: 25-25, 290-329
src/updates/reducers/__tests__/updates.spec.ts (1)
7-8: LGTM!Also applies to: 31-31, 484-553
src/updates/main.ts (1)
368-378: LGTM!src/i18n/tr-TR.i18n.json (1)
287-311: LGTM!src/i18n/uk-UA.i18n.json (1)
278-304: LGTM!src/i18n/zh-CN.i18n.json (1)
324-347: LGTM!src/i18n/zh-TW.i18n.json (1)
246-269: LGTM!src/store/rootReducer.ts (1)
69-69: LGTM!Also applies to: 114-114
src/ui/components/TopBar/UpdateLabel.tsx (3)
1-1: LGTM!Also applies to: 17-24, 47-52, 61-70, 99-123, 185-246, 248-259, 274-294
26-29: 🎯 Functional CorrectnessNo change needed.
The
Record<LabelVariant, { default: string; hover: string; press: string }>type is valid; the repeated value type in the snippet is not present.> Likely an incorrect or invalid review comment.
30-43: 📐 Maintainability & Code QualityNo change needed. The referenced button background CSS variables are used as custom properties; the hexadecimal fallbacks still apply when those custom properties are not present.
> Likely an incorrect or invalid review comment.src/i18n/ja.i18n.json (1)
268-293: LGTM!src/i18n/no.i18n.json (1)
519-545: LGTM!src/i18n/pl.i18n.json (1)
308-336: LGTM!src/i18n/pt-BR.i18n.json (1)
477-503: LGTM!src/i18n/ru.i18n.json (1)
444-472: LGTM!src/i18n/sv.i18n.json (1)
497-523: LGTM!src/ui/components/TopBar/UpdateLabel.spec.tsx (2)
2-2: LGTM!
247-361: LGTM!src/i18n/en.i18n.json (1)
534-538: LGTM!src/i18n/de-DE.i18n.json (1)
442-468: LGTM!src/i18n/es.i18n.json (1)
472-498: LGTM!src/i18n/fi.i18n.json (1)
440-466: LGTM!src/i18n/fr.i18n.json (1)
441-467: LGTM!src/i18n/hu.i18n.json (1)
629-655: LGTM!src/i18n/it-IT.i18n.json (1)
49-75: LGTM!
- Serialize update checks: a manual check requested during the startup ladder's backoff window joins the running ladder instead of racing a second one (and its error-suppression flag) against it - Disable the titlebar pill while a check is in flight instead of exposing a focusable button with no operation - Use the generic "Server" label for the log viewer server filter's accessible name instead of the "All Servers" option text
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/updates/main.ts (1)
547-555: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSettle a failed shared check only once.
If a manual request joins a startup request that later rejects, both
catchhandlers run. The app then logs two warnings and dispatchesUPDATES_NEW_VERSION_NOT_AVAILABLEtwice for one check.Convert the final failure to the benign result inside
requestUpdateCheck, or assign one failure owner. Callers should await the settled shared operation.Proposed fix
const requestUpdateCheck = (): Promise<void> => { if (!pendingUpdateCheck) { - pendingUpdateCheck = checkForUpdatesWithRetry().finally(() => { - pendingUpdateCheck = null; - }); + pendingUpdateCheck = checkForUpdatesWithRetry() + .catch((error) => { + console.warn( + 'Update check failed:', + error instanceof Error ? error.message : error + ); + dispatch({ type: UPDATES_NEW_VERSION_NOT_AVAILABLE }); + }) + .finally(() => { + pendingUpdateCheck = null; + }); } return pendingUpdateCheck; };Also applies to: 558-570
🤖 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/updates/main.ts` around lines 547 - 555, Update requestUpdateCheck so the shared operation converts failures into a benign settled result and owns the warning plus UPDATES_NEW_VERSION_NOT_AVAILABLE dispatch once. Make callers, including the startup automatic check and manual request path, await that settled operation without adding separate catch-side effects.
🤖 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.
Outside diff comments:
In `@src/updates/main.ts`:
- Around line 547-555: Update requestUpdateCheck so the shared operation
converts failures into a benign settled result and owns the warning plus
UPDATES_NEW_VERSION_NOT_AVAILABLE dispatch once. Make callers, including the
startup automatic check and manual request path, await that settled operation
without adding separate catch-side effects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e588da1b-36a1-43e2-89c1-ece0a2c4d77a
📒 Files selected for processing (4)
src/logViewerWindow/logViewerWindow.tsxsrc/ui/components/TopBar/UpdateLabel.spec.tsxsrc/ui/components/TopBar/UpdateLabel.tsxsrc/updates/main.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/ui/components/TopBar/UpdateLabel.spec.tsx
- src/logViewerWindow/logViewerWindow.tsx
- src/ui/components/TopBar/UpdateLabel.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/updates/main.ts
🔇 Additional comments (2)
src/updates/main.ts (2)
276-291: LGTM!
390-395: 🎯 Functional CorrectnessNo change needed. The provided logger defines
info,warn,error, and an optionaldebug(message: string)callback, which matches electron-updaterLoggerdeclarations.
1 participant
What
Titlebar feedback for manual update checks
"Check for Updates" (meatball menu, menu bar, About dialog) previously gave no visual feedback. The titlebar update label now covers the whole flow:
The new
updateCheckStatusstate is keyed on the user-initiated action rather than the autoUpdaterchecking-for-updateevent, so automatic startup checks never flash the titlebar. The feedback is also gated onisUpdatingAllowed && isUpdatingEnabled, since builds that cannot self-update never register the check listener.Resilient, quiet update checks
Investigation of the recurring
Error: No published versions on GitHublog entries (traced withDEBUG=electron-builderagainst the real updater flow) showed GitHub intermittently serving empty/stalled responses on the release-metadata request — a 20s stall was observed on/releases/latest— which electron-updater misreports as that error even though releases exist.logger.erroroutput is routed to warn so user-shared logs stop showing errors nobody can act onAccessibility
Fuselage's
Selectis backed by react-aria'suseSelect, which only recognizeslabel/aria-label/aria-labelledby; six selects (2 in Settings, 4 log viewer filters) relied onFieldLabel htmlForor a placeholder and warned on every render. They now havearia-labels.i18n
The whole
tabBarsection (update pill, workspaces, unread tooltip, meatball menu, window controls) existed only in English. Added translations for all 16 registered locales, with correct plural forms for the unread count (one/few/many for ru/uk/pl, single form for ja/zh).Testing
yarn lint,tsc --noEmitcleanupdateCheckStatusreducer transitions (10 cases) and UpdateLabel check-feedback states (checking/up-to-date, click and timed dismissal, gating, pill precedence)Summary by CodeRabbit
New Features
Accessibility
Tests