feat(sidebar): add SidebarRail collapsed icon navigation rail - #40558
Closed
ggazzo wants to merge 18 commits into
Closed
feat(sidebar): add SidebarRail collapsed icon navigation rail#40558ggazzo wants to merge 18 commits into
ggazzo wants to merge 18 commits into
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
|
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:
WalkthroughAdds a SidebarRail UI and integrates it into the main layout; implements a MediaCallWidget slot so the VoIP widget can render inline inside the sidebar with adjusted styling and UI behavior. ChangesSidebarRail Navigation Feature
Media Call Widget Inline Mode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 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 |
ggazzo
marked this pull request as draft
May 15, 2026 15:53
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx (1)
13-13: 💤 Low valueConsider narrowing the type assertion.
The type assertion
as string | number | null | undefinedis very broad. If the session value has a more specific expected shape, consider typing it more precisely or using a type guard for runtime validation.🤖 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 `@apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx` at line 13, The broad type assertion on unread from useSession in SidebarRailInbox should be narrowed: update the call to useSession or the unread variable to a more specific type (e.g., useSession<number | null> or useSession<string | null>) matching the actual session shape, or add a runtime type guard that validates the value before using it; modify the unread declaration and any subsequent logic that reads it (references to unread and the useSession call in SidebarRailInbox) so the compiler knows the exact expected type and you handle null/undefined safely.
🤖 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 `@apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx`:
- Around line 18-20: The Inbox button's click handler (handleClick using
useEffectEvent) currently calls router.navigate('/home') which mismatches the
active-state check for '/inbox' and no '/inbox' route exists; update handleClick
to navigate to an actual inbox route that your app uses (e.g.,
'/admin/email-inboxes' or whichever inbox route you add), or else remove the
'/inbox' active-route check if this button is intended as a home shortcut;
specifically modify the function referenced as handleClick (and its
router.navigate(...) call) to point to the correct route that matches the
existing active-route logic.
---
Nitpick comments:
In `@apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx`:
- Line 13: The broad type assertion on unread from useSession in
SidebarRailInbox should be narrowed: update the call to useSession or the unread
variable to a more specific type (e.g., useSession<number | null> or
useSession<string | null>) matching the actual session shape, or add a runtime
type guard that validates the value before using it; modify the unread
declaration and any subsequent logic that reads it (references to unread and the
useSession call in SidebarRailInbox) so the compiler knows the exact expected
type and you handle null/undefined safely.
🪄 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: 9d711b83-6927-4fd0-bec0-b5f3d241bc7a
📒 Files selected for processing (15)
apps/meteor/.storybook/main.tsapps/meteor/.storybook/mocks/meteor.jsapps/meteor/.storybook/mocks/sdk.tsapps/meteor/client/components/HorizontalDivider/HorizontalDivider.tsxapps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/components/ImageGallery/ImageGallery.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.stories.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailCreateNew.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailLoginPage.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailPhone.tsxapps/meteor/client/sidebar/SidebarRail/index.tspackages/i18n/src/locales/en.i18n.json
📜 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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/sidebar/SidebarRail/index.tsapps/meteor/client/components/HorizontalDivider/HorizontalDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailLoginPage.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.stories.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailPhone.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsxapps/meteor/client/components/ImageGallery/ImageGallery.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailCreateNew.tsx
🧠 Learnings (6)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/sidebar/SidebarRail/index.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/sidebar/SidebarRail/index.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/sidebar/SidebarRail/index.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/sidebar/SidebarRail/index.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/components/HorizontalDivider/index.tsapps/meteor/client/sidebar/SidebarRail/index.tsapps/meteor/client/components/HorizontalDivider/HorizontalDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailLoginPage.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.stories.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailPhone.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsxapps/meteor/client/components/ImageGallery/ImageGallery.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailCreateNew.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/HorizontalDivider/HorizontalDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailLoginPage.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailDivider.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.stories.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailPhone.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsxapps/meteor/client/components/ImageGallery/ImageGallery.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRail.tsxapps/meteor/client/sidebar/SidebarRail/SidebarRailCreateNew.tsx
🪛 Biome (2.4.15)
apps/meteor/.storybook/mocks/meteor.js
[error] 44-47: Illegal use of an export declaration outside of a module
(parse)
🔇 Additional comments (14)
apps/meteor/.storybook/main.ts (1)
53-53: LGTM!apps/meteor/.storybook/mocks/sdk.ts (1)
1-21: LGTM!apps/meteor/client/components/ImageGallery/ImageGallery.tsx (1)
8-13: LGTM!apps/meteor/client/components/HorizontalDivider/HorizontalDivider.tsx (1)
1-8: LGTM!apps/meteor/client/components/HorizontalDivider/index.ts (1)
1-1: LGTM!apps/meteor/.storybook/mocks/meteor.js (1)
44-47: The mock's export shape is already compatible with the codebase. All code importsDDPCommonas a named export and accessesparseDDPandstringifyDDPas object properties (DDPCommon.parseDDP,DDPCommon.stringifyDDP), which matches the mock's structure exactly.apps/meteor/client/sidebar/SidebarRail/SidebarRail.tsx (1)
1-57: LGTM!apps/meteor/client/sidebar/SidebarRail/SidebarRailCreateNew.tsx (1)
1-22: LGTM!apps/meteor/client/sidebar/SidebarRail/SidebarRailDivider.tsx (1)
1-11: LGTM!apps/meteor/client/sidebar/SidebarRail/SidebarRailLoginPage.tsx (1)
1-15: LGTM!apps/meteor/client/sidebar/SidebarRail/SidebarRailPhone.tsx (1)
1-17: LGTM!apps/meteor/client/sidebar/SidebarRail/index.ts (1)
1-1: LGTM!apps/meteor/client/sidebar/SidebarRail/SidebarRail.stories.tsx (1)
1-65: LGTM!packages/i18n/src/locales/en.i18n.json (1)
2664-2664: LGTM!
Comment on lines
+18
to
+20
| const handleClick = useEffectEvent(() => { | ||
| router.navigate('/home'); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for inbox route definitions and other inbox navigation patterns
# Search for inbox route definitions
echo "=== Inbox route definitions ==="
rg -n --type=ts --type=tsx 'path.*inbox|route.*inbox' -i -C2
# Search for other inbox navigation patterns
echo -e "\n=== Other inbox navigation calls ==="
rg -n --type=ts --type=tsx "navigate\(['\"].*inbox" -i -C2Repository: RocketChat/Rocket.Chat
Length of output: 210
🏁 Script executed:
#!/bin/bash
# Search without unrecognized file types
echo "=== Searching for inbox route definitions ==="
rg -n 'path.*inbox|route.*inbox' -i -g '*.ts' -g '*.js' -g '*.tsx' -g '*.jsx' -C2
echo -e "\n=== Searching for inbox navigation calls ==="
rg -n "navigate\(['\"].*inbox" -i -g '*.ts' -g '*.js' -g '*.tsx' -g '*.jsx' -C2
echo -e "\n=== Checking SidebarRailInbox.tsx content ==="
cat apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx
echo -e "\n=== Searching for /home navigation calls to understand pattern ==="
rg -n "navigate\(['\"]\/home" -g '*.ts' -g '*.js' -g '*.tsx' -g '*.jsx' -C1 | head -30Repository: RocketChat/Rocket.Chat
Length of output: 5628
🏁 Script executed:
#!/bin/bash
# Search for /inbox route specifically
echo "=== Searching for /inbox route ==="
rg -n "path.*\/inbox|pattern.*\/inbox|route.*\/inbox|\'/inbox\'|\"/inbox\"" -g '*.ts' -g '*.js' -g '*.tsx' -g '*.jsx' -C2
echo -e "\n=== Searching for route registrations ==="
rg -n "registerRoute|createRoute" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' | head -30
echo -e "\n=== Searching for all route patterns containing inbox ==="
rg -n "pattern|pathname" -g 'routes*.ts' -g 'routes*.tsx' -C1Repository: RocketChat/Rocket.Chat
Length of output: 26869
🏁 Script executed:
#!/bin/bash
# Search for /inbox without regex escaping
echo "=== Searching for /inbox string directly ==="
rg "/inbox" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -C2
echo -e "\n=== Searching for inbox string in route files ==="
rg "inbox" apps/meteor/client/startup/routes.tsx apps/meteor/client/views/admin/routes.tsx
echo -e "\n=== Check SidebarRailInbox again with context ==="
cat -n apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx | sed -n '10,25p'Repository: RocketChat/Rocket.Chat
Length of output: 2214
Navigation target should be /inbox (or equivalent inbox route).
The Inbox button checks for a route containing /inbox (line 16) but navigates to /home instead. Additionally, no /inbox route exists in the codebase—only /admin/email-inboxes for admin settings. The route detection logic on line 16 will never match, breaking the active state indicator. Update the navigation to match an actual inbox route or remove the route detection if this is intentionally a home shortcut.
🤖 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 `@apps/meteor/client/sidebar/SidebarRail/SidebarRailInbox.tsx` around lines 18
- 20, The Inbox button's click handler (handleClick using useEffectEvent)
currently calls router.navigate('/home') which mismatches the active-state check
for '/inbox' and no '/inbox' route exists; update handleClick to navigate to an
actual inbox route that your app uses (e.g., '/admin/email-inboxes' or whichever
inbox route you add), or else remove the '/inbox' active-route check if this
button is intended as a home shortcut; specifically modify the function
referenced as handleClick (and its router.navigate(...) call) to point to the
correct route that matches the existing active-route logic.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 15 files
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
ggazzo
force-pushed
the
feat/local-sidebar-collapsed
branch
from
May 15, 2026 20:35
bc28b3f to
f1a6775
Compare
ggazzo
changed the base branch from
develop
to
refactor/userstatuses-streamer-injection
May 15, 2026 20:47
Base automatically changed from
refactor/userstatuses-streamer-injection
to
develop
May 16, 2026 03:02
ggazzo
force-pushed
the
feat/local-sidebar-collapsed
branch
from
May 18, 2026 13:20
be3f89b to
19c3b53
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dmv/develop #40558 +/- ##
===============================================
+ Coverage 69.06% 69.17% +0.10%
===============================================
Files 3655 3660 +5
Lines 143366 143505 +139
Branches 25725 25716 -9
===============================================
+ Hits 99018 99268 +250
+ Misses 40041 39930 -111
Partials 4307 4307
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
force-pushed
the
feat/local-sidebar-collapsed
branch
17 times, most recently
from
May 19, 2026 20:21
74a16dd to
8fb05f6
Compare
ggazzo
added a commit
that referenced
this pull request
Jul 2, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 2, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 3, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 3, 2026
ggazzo
force-pushed
the
feat/local-sidebar-collapsed
branch
from
July 3, 2026 13:24
cfc265e to
7e5217e
Compare
ggazzo
added a commit
that referenced
this pull request
Jul 3, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 4, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 7, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 7, 2026
ggazzo
added a commit
that referenced
this pull request
Jul 7, 2026
`client/lib/sdk/ddpProtocol.ts` destructures `parseDDP` and `stringifyDDP` from `DDPCommon` at module load. Without a matching entry in the meteor mock any story whose bundle reaches the SDK chain crashes with: Cannot destructure property 'parseDDP' of 'DDPCommon' as it is undefined Add a narrow `DDPCommon` no-op (parseDDP/stringifyDDP) so the destructuring resolves. Scoped change — does not re-introduce the broader SDKClient stub.
Thin wrapper that forces `vertical={false}` on `@rocket.chat/fuselage`
Divider, providing a single horizontal-only entrypoint for callers
that want to express intent at the type level without re-implementing
the visual style.
The widget is currently a floating, draggable portal. The sidebar rail needs to host the same widget anchored inside a slot, so add a slot-based mount mechanism and let the widget render in place without dragging or border styles. Slot mechanism - `MediaCallWidgetSlotContext`: shared context exposing the active slot element + an `inline` boolean. - `MediaCallWidgetSlot`: drop-in component that registers its own `<div>` as the target. Removing it from the tree restores the floating portal automatically. The slot aligns its child to the top so the anchored widget keeps its intrinsic height instead of stretching to the container. - `MediaCallProvider`: hosts the slot state above the view provider so consumers in `children` and the widget itself share the same context value. `MediaCallProvider` now wraps `children` with `MediaCallViewProvider` so `useMediaCallView` resolves in the app tree. - `MediaCallViewProvider`: when a slot is registered the widget is rendered via `createPortal(widget, slot)`; otherwise it falls back to the previous `AnchorPortal` mount. Widget composition - `WidgetBase`: inline branch drops the fixed positioning, border, and z-index. Background, border-radius, shadow and width are kept so the anchored render looks the same as the floating one. - `Widget`: read inline from context, skip drag wiring when inline. - `WidgetHandle`: hide itself when inline (drag is disabled). - `NewCall`: render the numeric Keypad below the autocomplete when inline so the dialer matches the sidebar layout in the design. Visibility rules - Only "undock" the widget for active call states (calling, ringing, ongoing). When the slot is gone and the session is idle the widget renders nothing instead of popping out as a floating NewCall. Snapshot tests regenerated to reflect the inline-aware render tree.
44px-wide vertical navigation rail (Material Design "navigation rail" pattern) matching the Figma "Local Sidebar / Collapsed" design. Composes existing NavBar items (Home, Directory, Marketplace, Administration menu, UserMenu) and adds rail-specific variants: - `SidebarRailCreateNew`: pencil-box menu reusing `useCreateNewMenu`. - `SidebarRailPhone`: phone icon that navigates to `/call-history` and reflects the route in its `pressed` state. - `SidebarRailCallPanel`: Sidepanel-based second-level container hosting a `MediaCallWidgetSlot` for the dialer body. - `SidebarRailCallRouteSync`: keeps the dialer open while on `/call-history` (bouncing back from `closed` to `new`) and closes it on leave when the dialer is idle. - `SidebarRailHeader`: slim top bar reusing the Fuselage NavBar with the workspace 28px icon and the existing `NavBarNavigation` (search + back/forward), suppressing the legacy NavBar groups. - `SidebarRailLoginPage`: icon-only login NavBarItem for the avatar slot when no user is signed in. - `SidebarRailDivider`: horizontal divider with rail-specific tokens. Includes an `Inbox` i18n key reused later and a Storybook file with Anonymous, LoggedIn, WithUnreadBadge and WithCreatePermissions variants.
When `USE_SIDEBAR_RAIL` is true (and the layout is not embedded /
mobile), replace the top horizontal `NavBar` with the slim
`SidebarRailHeader` and render `SidebarRail` as the first column of
`#rocket-chat`. The legacy NavBar path remains for the other cases
so the change is reversible by flipping a single constant.
- `sidebarRailFlag.ts`: dev-only constant, intended to be replaced
by `useFeaturePreview('sidebarRail')` in a follow-up before any
rollout.
- `LayoutWithSidebar.tsx`: pick the header, render the rail, and
drive the secondary slot through a new `SecondaryPanel`. While on
`/call-history` the slot renders the rail call panel; otherwise
it falls back to the existing Sidebar / NavigationRegion feature
preview split.
- `SecondaryPanel.tsx`: extracted into its own file to comply with
`react/no-multi-comp`.
- `MainLayoutStyleTags.tsx`: extend the dark palette selector to
cover `.rcx-sidebar-rail` so the rail keeps the dark surface in
both themes.
Expose the SidebarRail flag on window.USE_SIDEBAR_RAIL (default false) so it can be toggled at runtime via a custom script, instead of a hardcoded build-time constant. Read on every render through isSidebarRailEnabled().
The global Window interface augmentation must keep the lib name 'Window', which violates the I-prefix naming-convention rule. Disable the rule on that line.
The spec still mocked the removed layout modules (Sidebar, NavigationRegion, FeaturePreview) while LayoutWithSidebar now imports SidebarRail, SidebarRailHeader, SidebarRailCallRouteSync and SecondaryPanel. The unmocked real SidebarRail pulled app/utils/client -> rocketchat.info into the jest graph, which jest cannot parse (SyntaxError). Mock the new imports instead.
Render the DTMF dialpad while a voice call is ongoing so users can drive IVRs, conference bridges and other keypad-driven phone services. - Gate the dialpad on `peerInfo.external` (SIP calls only) — internal user-to-user calls don't need DTMF. - Inline (sidebar rail) renders the keypad permanently expanded; the floating widget keeps the collapsible toggle. - `OngoingCallWithScreen`: show the dialpad when screen sharing is supported but hide it while a local share is active, preserving the screen-sharing layout. - `Keypad` gains an `autoFocus` prop (default true) so the always-on inline keypad does not steal focus on mount; `useKeypad` exposes an `alwaysOpen` option for that layout. - Cover the visibility rules with a unit test.
The SidebarRail call widget was driven by SidebarRailCallRouteSync, a global effect that opened/closed the dialer by toggling the session state in reaction to the current route. Because the toggle ran after paint and could not tell a route-opened idle dialer from a user-opened one, it produced three wrong behaviors: - Leaving the telephony screen briefly popped the idle dialer out as a floating widget (the reported issue). - Clicking the call icon on a message header off the telephony screen opened the prefilled dialer and the watcher immediately closed it again (flicker), instead of keeping it open like production. - Finishing a call from the telephony screen left the panel empty instead of resetting it back to the dialer. Make the call panel own the dialer lifecycle instead. Since the panel only mounts on /call-history, the route logic is implicit: - While mounted and idle, open the dialer (covers initial open and the post-call reset). - On unmount, drop an idle dialer in the layout phase, in sync with the slot teardown, so it never leaks out as a floating widget. Off the telephony screen the session is never forced into `new`, so the header-initiated floating dialer is left untouched and widget visibility falls out of (slot present) × (session state) declaratively. Remove SidebarRailCallRouteSync and its test mock.
The DMV-16 change gated the floating widget's dialpad toggle on the call being external (SIP), which removed it from internal user-to-user calls and broke the voice-calls-ee e2e suite (acceptCall expects the "Dialpad" button visible during an internal call). The floating widget keeps its dialpad for every ongoing call, as before. The SIP gate stays only on the inline sidebar dialpad (MediaCallDialpad), which is the surface DMV-16 adds. The toggle is suppressed only while inline so it doesn't duplicate the expanded inline keypad.
…all panel The sidebar call panel opened the idle dialer with the toggleWidget() toggle whenever the session was 'closed'. React StrictMode double-invokes the layout effect on mount, so the two toggles cancelled out (closed -> new -> closed) and the dialer never showed in dev. Add explicit, idempotent openDialer/closeDialer intents (reducer actions + useMediaSession + external-control signals) that only act on the closed<->new (idle dialer) transition and never touch an active call. SidebarRailCallPanel uses them instead of toggleWidget, so repeated calls are safe. toggleWidget is kept for genuine toggle affordances (e.g. the message toolbar call button).
The null-instance sync paths dispatched `reset`, which wiped the idle
dialer state ('new') on the instance's autoSync emit right after a call
ended — leaving the sidebar call panel empty. Introduce a `call_cleared`
intent that tears down call-derived state but preserves a user-driven
idle dialer, mirroring the deeplink pre-fill fix. The genuine unmount
path (no instance) still fully resets.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Navigating away from the call-history panel unmounted its widget slot, which reparented the idle dialer into the floating portal instead of dismissing it. Mark dialers opened via `openDialer` (the panel) as `docked` and skip rendering them once their slot is gone, so they disappear with the panel. Free-floating composers opened via `toggleWidget` (room/user "call" actions) are unaffected and still float. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(breaks telto links desktop integration)
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/sidebar/SidebarRail/SidebarRailCallPanel.tsx">
<violation number="1" location="apps/meteor/client/sidebar/SidebarRail/SidebarRailCallPanel.tsx:24">
P2: The cleanup effect now calls `closeDialer()` unconditionally on unmount instead of only when the dialer is idle (`state === 'new'`). The adjacent comment still describes a guard that no longer exists. During an ongoing call, navigating away from the rail will now close the dialer/interrupt the call UI — the previous guard intentionally scoped this to the idle state only. In development mode with StrictMode, the fake unmount/remount will close then reopen the dialer, which can cause visual flicker or state-transition issues if `closeDialer` transitions to a state other than 'closed'. Either restore the idle-only guard or update the comment to reflect the new behavior.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // the just-issued open has not re-rendered yet. | ||
| useLayoutEffect( | ||
| () => () => { | ||
| closeDialer(); |
Contributor
There was a problem hiding this comment.
P2: The cleanup effect now calls closeDialer() unconditionally on unmount instead of only when the dialer is idle (state === 'new'). The adjacent comment still describes a guard that no longer exists. During an ongoing call, navigating away from the rail will now close the dialer/interrupt the call UI — the previous guard intentionally scoped this to the idle state only. In development mode with StrictMode, the fake unmount/remount will close then reopen the dialer, which can cause visual flicker or state-transition issues if closeDialer transitions to a state other than 'closed'. Either restore the idle-only guard or update the comment to reflect the new behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/SidebarRail/SidebarRailCallPanel.tsx, line 24:
<comment>The cleanup effect now calls `closeDialer()` unconditionally on unmount instead of only when the dialer is idle (`state === 'new'`). The adjacent comment still describes a guard that no longer exists. During an ongoing call, navigating away from the rail will now close the dialer/interrupt the call UI — the previous guard intentionally scoped this to the idle state only. In development mode with StrictMode, the fake unmount/remount will close then reopen the dialer, which can cause visual flicker or state-transition issues if `closeDialer` transitions to a state other than 'closed'. Either restore the idle-only guard or update the comment to reflect the new behavior.</comment>
<file context>
@@ -1,38 +1,27 @@
- if (stateRef.current === 'new') {
- closeDialer();
- }
+ closeDialer();
},
[closeDialer],
</file context>
Contributor
|
Replaced by #41712 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SidebarRail— a 44px icon-only navigation rail (Material Design "navigation rail" pattern) that mirrors the Figma "Local Sidebar / Collapsed" node and composes existing NavBar items (Home, Directory, Marketplace, Administration menu, UserMenu) plus rail-specific variants (Create-new pencil-box menu, Inbox with unread badge, Phone viauseMediaCallAction, icon-only Login fallback, horizontal Divider with rail tokens).HorizontalDividerwrapper around FuselageDividerso callers can express horizontal-only intent at the type level.develop: switchesImageGalleryto Swiper's publicexportspaths, expands the meteor mock to cover the SDKsubscribe/callAsyncsurface, and adds aNormalModuleReplacementPluginthat swaps the liveSDKClientfor a no-op mock so stories that touch hooks likeuseStatusItemsoruseUnreadrender without a DDP connection.Test plan
cd apps/meteor && CI=1 yarn storybookopens cleanly without webpack build failuresSidebar/SidebarRail/Anonymousrenders the rail with the icon-only login buttonSidebar/SidebarRail/LoggedInrenders the rail with the user avatar (noMeteor.connection.subscriberuntime error)Sidebar/SidebarRail/WithUnreadBadgeshows the danger badge on the inbox itemSidebar/SidebarRail/WithCreatePermissionsshows the create-new menu populatedyarn tsc --noEmit --skipLibCheckpasses for the new filesSummary by CodeRabbit
New Features
Documentation
Localization
Chores
DMV-76
DMV-73