Skip to content

fix: #3244 sidebar tooltip shows null for servers beyond 9th position - #3326

Merged
jeanfbrito merged 1 commit into
masterfrom
fix/3244-sidebar-tooltip-null
Jul 9, 2026
Merged

fix: #3244 sidebar tooltip shows null for servers beyond 9th position#3326
jeanfbrito merged 1 commit into
masterfrom
fix/3244-sidebar-tooltip-null

Conversation

@jeanfbrito

@jeanfbrito jeanfbrito commented May 10, 2026

Copy link
Copy Markdown
Member

Description

Fixes #3244 — two bugs in sidebar tooltip shortcut formatting.

Bug 1: Off-by-one (index.tsx)

order <= 9  →  order < 9

Server #10 was getting shortcutNumber "10" even though Ctrl+10 doesn't exist. Now only servers 1-9 get shortcuts.

Bug 2: Null tooltip (ServerButton.tsx)

${title} (${mod}+${shortcutNumber})  →  ${title}${shortcutNumber !== null ? ... : ''}

When shortcutNumber is null (servers 10+), the template rendered "(^+null)" in the tooltip. Now the shortcut portion is conditional.

Changes

  • src/ui/components/SideBar/index.tsx: 1 line changed
  • src/ui/components/SideBar/ServerButton.tsx: 1 line changed

Risk

LOW — pure presentation change, no state or logic modifications.

Summary by CodeRabbit

  • Bug Fixes
    • Improved sidebar shortcut display to only show keyboard shortcut information in tooltips when a shortcut is available.
    • Refined server button shortcut assignment to apply only to the first eight items in the sidebar.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4935c597-55ad-4f77-873c-e8a7943449a6

📥 Commits

Reviewing files that changed from the base of the PR and between c2b584b and a5ec12a.

📒 Files selected for processing (2)
  • src/ui/components/SideBar/ServerButton.tsx
  • src/ui/components/SideBar/index.tsx
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: check (windows-latest)
  • GitHub Check: check (macos-latest)
  • GitHub Check: check (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript for all new code in this codebase unless explicitly told otherwise
Use Fuselage components from @rocket.chat/fuselage for all UI work — only create custom components when Fuselage doesn't provide the needed functionality
Check Theme.d.ts for valid color tokens when working with Fuselage components
Use optional chaining with fallbacks for platform-specific APIs instead of mocks (e.g., process.getuid?.() ?? 1000) to ensure code works across all platforms without requiring mocks
TypeScript code must use strict mode
Use React functional components with hooks instead of class components
Redux actions must follow the FSA (Flux Standard Action) pattern
Use camelCase for file naming
Use PascalCase for component file names (React components)
Write self-documenting code through clear naming — avoid unnecessary comments

Files:

  • src/ui/components/SideBar/index.tsx
  • src/ui/components/SideBar/ServerButton.tsx
🔇 Additional comments (2)
src/ui/components/SideBar/ServerButton.tsx (1)

146-146: Tooltip shortcut rendering fix is correct.

Conditionally appending the shortcut suffix only when shortcutNumber is non-null resolves the (^+null) tooltip issue without changing other tooltip content paths.

src/ui/components/SideBar/index.tsx (1)

106-108: Shortcut boundary update matches expected behavior.

Using order < 9 correctly limits numeric shortcuts to servers 1–9 and prevents assigning a shortcut to the 10th server and beyond.


Walkthrough

Two coordinated fixes address sidebar tooltip rendering for servers beyond the 9th position: the shortcut assignment logic is corrected from order <= 9 to order < 9, and the tooltip rendering adds a null check to avoid displaying null in the user interface.

Changes

Sidebar Tooltip Fix

Layer / File(s) Summary
Shortcut Assignment Logic
src/ui/components/SideBar/index.tsx
Condition for shortcut number eligibility changes from order <= 9 to order < 9, limiting keyboard shortcuts to servers 1–9 only.
Tooltip Rendering
src/ui/components/SideBar/ServerButton.tsx
Tooltip header now conditionally includes shortcut suffix only when shortcutNumber is not null, preventing (^+null) in the UI.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: fixing the sidebar tooltip to not display null for servers beyond the 9th position, directly addressing the issue number referenced.
Linked Issues check ✅ Passed Both bugs identified in issue #3244 are addressed: the off-by-one condition in index.tsx changed from order <= 9 to order < 9, and the null check added in ServerButton.tsx to conditionally render the shortcut portion.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the two bugs in issue #3244; no unrelated modifications are present in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- index.tsx: order <= 9 -> order < 9 (off-by-one, server #10 showed shortcut)
- ServerButton.tsx: conditional shortcut string in tooltip (null rendered as '(^+null)')
@jeanfbrito
jeanfbrito force-pushed the fix/3244-sidebar-tooltip-null branch from 8ef84d6 to a5ec12a Compare May 10, 2026 17:33
@jeanfbrito
jeanfbrito merged commit e405a6a into master Jul 9, 2026
12 of 13 checks passed
@jeanfbrito
jeanfbrito deleted the fix/3244-sidebar-tooltip-null branch July 9, 2026 14:38
jeanfbrito added a commit that referenced this pull request Jul 15, 2026
…icker, sidebar tooltip) (#3417)

* fix: #3244 sidebar tooltip null in shortcut formatting (#3326)

- index.tsx: order <= 9 -> order < 9 (off-by-one, server #10 showed shortcut)
- ServerButton.tsx: conditional shortcut string in tooltip (null rendered as '(^+null)')

(cherry picked from commit e405a6a)

* fix: keep screen picker sources stable under Electron 42 macOS capture stack (#3414)

Electron 42's macOS ScreenCaptureKit backend bounds getSources() at ~3s
(upstream hang/crash fixes), returning an empty array or empty thumbnails
when enumerations run back-to-back. The picker's 3s polling plus the
post-selection re-enumeration turned those empty results into a blank
"No windows found" list and denied valid share attempts.

- Split the desktop capturer cache into per-type buckets: screen-only
  enumeration is fast (~700ms) and reliable; window enumeration is paced
  with a 4s post-completion cooldown (cold-start chain bypasses it once)
- Never overwrite a non-empty bucket with an empty enumeration result;
  keep the bucket stale so it retries on the next opportunity
- Merge thumbnails by source id so a source arriving with an empty
  thumbnail keeps its last good preview instead of being dropped
- Validate the selected source against the cache (the same list the
  picker rendered from) instead of re-enumerating on Share; fall back
  to one direct enumeration only when the cache is empty
- Keep the previous source list when a renderer fetch fails

Measured on macOS with Electron 42.5.0: tight-loop getSources returned
0 sources in ~75% of calls; screen-only calls never failed; alternating
per-type calls with 4s gaps returned complete results every round.

Documented in docs/KNOWN_ISSUES.md with upstream refs
(electron/electron#51128, electron/electron#50960) and the macOS 15+
useSystemPicker follow-up.

(cherry picked from commit dddd2cc)

* fix: externalize dependency subpath imports in rollup bundles (#3411)

rollup externals matched module ids by exact name only, so subpath
entrypoints like `react-dom/client` (used by createRoot in
rootWindow.ts, log-viewer-window.tsx, and screenSharePickerMount.tsx)
did not match the `react-dom` external and got bundled as
build-time-NODE_ENV ReactDOM, while `react` stayed external and
resolved to a different (dev) copy from the asar at runtime — mixing
incompatible React internals and crashing the renderer
(SUP-1072, ReferenceError: exports is not defined).

makeExternal() now matches `id === moduleName || id.startsWith(moduleName + '/')`
so subpath imports are externalized alongside their base package.
scripts/check-bundle-externals.mjs guards the build against regressing
back to bundled React internals, wired into `yarn build`.

(backported to 4.15.x)

Adapted from master commit 0dafe39: 4.15.4 has no
`screen-picker-window.js` rollup entry (the screen picker mounts
inside the video-call-window bundle via screenSharePickerMount.tsx,
not a separate window/entry point in this release), so that entry and
its assertions were omitted from check-bundle-externals.mjs. All other
bundle configs and the check script are ported as-is.

* fix: correct auto-merge artifact in ScreenSharingRequestTracker backport

The cherry-pick of dddd2cc (#3414) auto-merged with references to
master's entry/finishActive queue API, which doesn't exist in 4.15.4's
flat cb/markComplete structure — this failed typecheck. Replaced with
4.15.4's existing cb(...) calling convention, matching the fallback
path directly below it in the same function.

Also fixed the corresponding cherry-picked spec assertion: it expected
cb to be called with `null` (master's deny convention), but 4.15.4's
DisplayMediaCallback deny convention is `{ video: false }` throughout
this file — updated the assertion to match.

* fix: remove leftover merge-conflict marker fragment from KNOWN_ISSUES.md

The docs/KNOWN_ISSUES.md merge during the backport left a trailing
`>>>>>>> dddd2cc (...)` conflict-marker remnant appended to the last
line of the file. Removed; no other content was affected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sidebar tooltip shows "(^+null)" or "(^+10)" for servers beyond the 9th position

1 participant