-
Notifications
You must be signed in to change notification settings - Fork 13k
chore: Upgrade Fuselage focus handling #37334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughRemoves use of fuselage-polyfills from startup and manifests; switches focus-visible handling from manual class toggling to native Changes
Sequence Diagram(s)sequenceDiagram
participant App as Meteor client bootstrap
participant Login as ./meteor/login
participant ECDH as ./ecdh
participant ImportPkg as ./importPackages
participant Startup as ./startup
Note over App,Login: Polyfills import removed from the startup chain
App->>Login: dynamic import('./meteor/login')
Login-->>App: loaded
App->>ECDH: dynamic import('./ecdh')
ECDH-->>App: loaded
App->>ImportPkg: dynamic import('./importPackages')
ImportPkg-->>App: loaded
App->>Startup: dynamic import('./startup')
Startup-->>App: loaded
Note over App,Startup: Afterwards: Promise.all(views...) preserved
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37334 +/- ##
===========================================
+ Coverage 68.97% 68.98% +0.01%
===========================================
Files 3358 3358
Lines 114228 114224 -4
Branches 20537 20536 -1
===========================================
+ Hits 78784 78794 +10
+ Misses 33351 33343 -8
+ Partials 2093 2087 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
ee/packages/ui-theming/package.json (1)
8-9: Consistent upgrade pattern; verify breaking changes addressed globally.This file follows the same dependency upgrade pattern as other packages (tilde to caret ranges for fuselage/fuselage-hooks). Since the changes are coordinated across the monorepo, verify that all breaking changes from the minor version bumps have been consistently addressed everywhere, not just in isolated components.
packages/ui-avatar/package.json (1)
9-10: Consistent upgrade; defer to primary verification comment.This follows the same upgrade pattern across the monorepo. The primary verification concerns (fuselage-polyfills removal and breaking changes) have been flagged in the first package.json review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
apps/meteor/client/main.ts(1 hunks)apps/meteor/package.json(3 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/gazzodown/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/fuselage-ui-kit/package.json
- packages/ui-client/package.json
- packages/ui-video-conf/package.json
- packages/ui-composer/package.json
- apps/meteor/package.json
- packages/web-ui-registration/package.json
- apps/uikit-playground/package.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Implement proper wait strategies for dynamic content
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/client/main.ts
⏰ 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). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
packages/storybook-config/package.json (3)
7-7: Verify version specifier change from tilde to caret is intentional.The dependency updates change version specifiers from tilde (
~) to caret (^), significantly widening the allowed range:
@rocket.chat/fuselage-hooks:~0.37.2→^0.38.1(patch-only → minor+patch)@rocket.chat/fuselage:~0.66.4→^0.67.0(patch-only → minor+patch)This broadening allows minor version bumps, which may introduce breaking changes depending on the library's stability guarantees for private monorepo packages. Confirm that this is intentional and that downstream code is compatible with any version within these ranges.
Also applies to: 24-24
1-58: Add context to PR description.The PR body contains only template boilerplate with no explanation of:
- Why version specifiers are changing from tilde to caret
- Why
fuselage-polyfillsis being removed (repo-wide rationale)- Compatibility/testing results for the new fuselage versions (0.38.1, 0.67.0)
- Whether these are breaking or non-breaking updates
Consider updating the PR description to document the migration strategy and any testing performed to validate these upgrades.
5-21: Removal of fuselage-polyfills is confirmed and safe to proceed.Verification complete:
- Git commit
d4b119435aexplicitly titled "Remove @rocket.chat/fuselage-polyfills" confirms this is an intentional removal- No lingering imports: Comprehensive codebase search found zero
importorrequirestatements referencing fuselage-polyfills- No active dependencies: The package is not listed in any package.json files, including the root and storybook-config
- References found only in documentation (README.md), maintenance scripts (fuselage.sh), and changelog—not in active code
The removal poses no risk of broken dependencies or missing imports.
packages/ui-voip/package.json (1)
35-36: Polyfills removal is verified safe; version bump concerns cannot be fully verified without code diff review.The removal of
fuselage-polyfillsis complete—no references remain across the codebase. The coordinated update to@rocket.chat/fuselage@^0.67.0and@rocket.chat/fuselage-hooks@^0.38.1has been consistently applied across 15+ packages.However, verification of the original review comment's second concern—confirming that breaking changes between versions have been addressed in code—cannot be completed using automated inspection alone. No documented breaking changes were found for fuselage 0.67.0, but actual code modifications to handle API changes (if any exist) would require reviewing the git diff.
Recommend: Manually verify that any API changes in fuselage 0.67.0 and fuselage-hooks 0.38.1 have been handled in your component code before merging.
apps/meteor/client/main.ts (1)
5-11: Acknowledge the deliberate removal, but recommend manual testing of critical user flows.This removal appears to be intentional and part of a broader polyfill cleanup strategy—earlier commits show deliberate removal of
queueMicrotaskpolyfill and polyfills outside the support range. The browserslist configuration targets modern browsers ("last 2 versions" and "Firefox ESR"), which natively support standard features that fuselage-polyfills likely provided.However, since the exact features in fuselage-polyfills cannot be fully determined from public sources and the change is recent, manual verification of critical user flows (particularly login, admin, and marketplace views) in the minimum supported browser versions would still be prudent as a final confirmation step.
a3055fb to
887a922
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/gazzodown/package.json (1)
35-36:⚠️ Critical: Unresolved issue from previous review—invalid package versions still present.This PR still contains the same critical issue flagged in the past review: the specified dependency versions do not exist in the npm registry.
@rocket.chat/fuselage^0.67.0 does not exist (latest published is 0.66.4)@rocket.chat/fuselage-hooks^0.38.1 does not exist (latest published is 0.38.0)These invalid versions will cause installation failures or pull in unintended versions. Please update to valid published versions before this PR can be merged.
See the past review comment on these lines for detailed analysis and verification steps.
🧹 Nitpick comments (1)
packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx (1)
1-27: Consider consolidating duplicate header button components.The
HeaderTitleButtonimplementation in this file is nearly identical topackages/ui-client/src/components/Header/HeaderTitleButton.tsx. The only meaningful difference is the type constraint (ComponentPropsWithoutRefvsComponentProps), while the styling and rendering logic are duplicated.Consider extracting the shared implementation to reduce maintenance burden and ensure consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
apps/meteor/client/main.ts(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(1 hunks)apps/meteor/package.json(3 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/ui-composer/package.json
- apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
- packages/ui-avatar/package.json
- packages/storybook-config/package.json
- packages/web-ui-registration/package.json
- apps/meteor/client/main.ts
- packages/ui-contexts/package.json
- apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx
- packages/ui-voip/package.json
- packages/ui-video-conf/package.json
- packages/fuselage-ui-kit/package.json
- ee/packages/ui-theming/package.json
- apps/uikit-playground/package.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/package.json
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/package.json
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-19: LGTM! CSS selector updated to use native:focus-visible.The change from
&:focus.focus-visibleto&:focus-visiblecorrectly replaces the polyfill-dependent selector with the native CSS pseudo-class. This aligns with removing the fuselage-polyfills dependency while maintaining identical focus styling behavior.packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx (1)
17-21: LGTM! Consistent CSS selector update.The change to
&:focus-visiblematches the update in the non-V2 header component, ensuring consistent focus behavior across both header implementations.packages/livechat/package.json (1)
35-35: Verify livechat compatibility with fuselage-hooks 0.38.1.The version constraint has been relaxed from
~0.37.2to^0.38.1, expanding the acceptable version range to include minor version updates (0.x where x ≥ 38). While this aligns with the broader repository migration mentioned in the PR context, ensure that livechat components have been tested against the new version and any breaking changes have been addressed.packages/ui-client/package.json (2)
26-27: Verify removal of fuselage-polyfills doesn't break ui-client.According to the AI summary,
@rocket.chat/fuselage-polyfillshas been removed in this update. Confirm that ui-client code does not have hardcoded dependencies on or references to fuselage-polyfills. If polyfills were previously relied upon, ensure they are now provided through the upgraded fuselage version or an alternative approach.
58-68: Verify peerDependencies align with upgraded devDependencies.The peerDependencies at lines 61–62 declare
@rocket.chat/fuselageand@rocket.chat/fuselage-hooksas peer deps. Confirm these peerDependency versions remain compatible with the upgraded devDependency versions (0.67.0 and 0.38.1 respectively), especially if consumers of this package pin to the older versions.apps/meteor/package.json (3)
257-260: Verify code changes align with fuselage package upgrades in apps/meteor.The dependency versions have been updated significantly:
@rocket.chat/fuselage: 0.66.4 → 0.67.0@rocket.chat/fuselage-hooks: 0.37.2 → 0.38.1@rocket.chat/fuselage-forms: 0.1.0 → 0.1.1@rocket.chat/fuselage-toastbar: 0.35.0 → 0.35.1Per the AI summary, code changes are required in omnichannel components (ref extraction for CheckOption) and client/main.ts (dynamic import replacement). However, these code changes are not visible in the provided files. Before merging, ensure:
- All component code consuming these packages has been updated
- Breaking changes in fuselage 0.67.0 have been addressed
- No hardcoded references to removed packages exist
257-257: Clarify version constraint shift from~to^.Most updated fuselage packages transitioned from
~(patch-only) to^(minor+patch) constraints. For example:
~0.66.4→^0.67.0(now allows 0.x.y where x ≥ 67)~0.37.2→^0.38.1(now allows 0.x.y where x ≥ 38)This expands acceptable version ranges significantly. Document whether this change is intentional to:
- Support faster adoption of bugfixes across minor versions, or
- Relax strict version pinning assumptions,
and confirm your CI/testing strategy validates these broader version ranges.
Also applies to: 269-269, 287-287
257-260: Verify fuselage-polyfills removal and replacement.According to the PR summary,
@rocket.chat/fuselage-polyfillshas been removed and its wrapper replaced with a direct dynamic import inapps/meteor/client/main.ts. Confirm:
- Removal of fuselage-polyfills dependency (not visible in provided code)
apps/meteor/client/main.tshas been updated to import polyfills directly- All runtime dependencies on fuselage-polyfills have been satisfied by the updated fuselage or alternative polyfill source
- No broken imports or references remain in the codebase
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.tsapps/meteor/client/views/room/hooks/useMessageListNavigation.tsapps/meteor/client/sidebar/RoomList/useSidebarListNavigation.tsapps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.tsapps/meteor/client/sidebar/RoomList/useSidebarListNavigation.tsapps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
🔇 Additional comments (4)
apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts (1)
67-67: LGTM: Blur handler correctly uses CSS selector-based focus detection.The change from
classList.contains('focus-visible')tomatches(':focus-visible')aligns with modern browser support for the native:focus-visiblepseudo-class.apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts (1)
72-72: LGTM: Focus detection correctly updated to use CSS selectors.Both the blur and focus handlers correctly replace
classList.contains('focus-visible')withmatches(':focus-visible'), leveraging native browser support for the:focus-visiblepseudo-class.Also applies to: 88-88
apps/meteor/client/views/room/hooks/useMessageListNavigation.ts (1)
76-76: LGTM: Focus detection correctly updated to use CSS selectors.Both handlers correctly migrate from class-based focus-visible detection to the native
:focus-visiblepseudo-class selector, consistent with modern accessibility best practices.Also applies to: 92-92
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts (1)
71-71: LGTM: Focus detection correctly updated to use CSS selectors.Both the blur and focus handlers correctly replace class-based checks with
matches(':focus-visible'), aligning with the fuselage upgrade and modern browser capabilities.Also applies to: 87-87
apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
Outdated
Show resolved
Hide resolved
ad27772 to
9fcfc89
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/gazzodown/package.json (1)
35-36:⚠️ Duplicate: Non-existent package versions block installation.This critical issue was flagged in the previous review. The specified versions do not exist in the npm registry and will cause installation failures:
@rocket.chat/fuselage@^0.67.0— does not exist (latest: 0.66.4)@rocket.chat/fuselage-hooks@^0.38.1— does not exist (latest: 0.38.0)Update to valid, published versions before merging. If the intent is to upgrade, confirm the actual available versions and whether the broader caret (
^) range is intentional compared to the previous tilde (~) ranges.Suggested fix:
- "@rocket.chat/fuselage": "^0.67.0", - "@rocket.chat/fuselage-hooks": "^0.38.1", + "@rocket.chat/fuselage": "^0.66.4", + "@rocket.chat/fuselage-hooks": "^0.38.0",
🧹 Nitpick comments (3)
packages/ui-avatar/package.json (1)
9-10: Reconsider using caret (^) versioning for pre-1.0 packages.The version specifiers were changed from tilde (~) to caret (^), which broadens the version range significantly for pre-1.0 packages. For example,
^0.67.0allows any version from0.67.0to0.999.999, whereas~0.66.4allowed only0.66.x.For pre-1.0 packages like fuselage, minor version bumps (e.g.,
0.67.0→0.68.0) can introduce breaking changes. This looser constraint could lead to unexpected failures when transitive dependencies pull in incompatible versions.Consider reverting to tilde (
~0.67.0,~0.38.1) to maintain stricter control over version selection, or document the reasoning if caret is intentional across the monorepo.apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (1)
34-37: LGTM! Clean inline destructuring prevents ref forwarding.The inline destructuring of
refas_refis concise and correctly prevents forwarding to the non-forwardRefCheckOptioncomponent.Optional: Consider consistency with AutoCompleteDepartmentMultiple.tsx. That file uses a separate destructuring statement (lines 58-59), while this uses inline destructuring. Both work fine, but a consistent pattern across similar use cases would improve maintainability.
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
37-60: Inconsistent room object construction between renderSelected and renderItem.The
renderSelectedcallback (line 37) uses{ _id: value, ...label, type: label?.type || 'c' }with a fallback fortype, whilerenderItem(lines 51-55) explicitly constructs{ type: label.type, _id: value, avatarETag: label.avatarETag }without spreadinglabeland without a fallback fortype. Consider using a consistent pattern across both render functions to prevent subtle bugs, especially since the renderItem version doesn't provide a default for type.Apply this diff to make the patterns consistent:
avatar={ <RoomAvatar size='x20' room={{ - type: label.type, _id: value, + ...label, + type: label?.type || 'c', - avatarETag: label.avatarETag, }} {...props} />Or alternatively, use the explicit pattern in both places to avoid the _id override issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(1 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/package.json(3 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/storybook-config/package.json
- ee/packages/ui-theming/package.json
- packages/livechat/package.json
- packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx
- apps/uikit-playground/package.json
- packages/ui-voip/package.json
- packages/web-ui-registration/package.json
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
- packages/ui-contexts/package.json
- packages/ui-video-conf/package.json
- packages/ui-client/package.json
- packages/fuselage-ui-kit/package.json
- apps/meteor/package.json
- apps/meteor/client/views/room/hooks/useMessageListNavigation.ts
- packages/ui-composer/package.json
- apps/meteor/client/main.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
packages/ui-avatar/package.json (1)
1-46: Verify that fuselage-polyfills removal is handled consistently.The
@rocket.chat/fuselage-polyfillsdependency was removed from this file. According to the AI summary, it is replaced with a dynamic import inapps/meteor/client/main.ts. Ensure that:
- The replacement import is present and functioning in all entry points.
- This package (ui-avatar) does not directly depend on polyfills at runtime.
- Consuming packages are not broken by the polyfills removal.
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-15: LGTM! Modern CSS pseudo-class adoption.The change to
:focus-visiblecorrectly replaces the polyfill-based approach and is well-supported in modern browsers.Verify that keyboard focus indicators display correctly across supported browsers and that mouse clicks don't show unwanted focus rings.
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts (1)
71-71: LGTM! Modern approach using native CSS pseudo-class.The changes correctly replace the polyfill-based
classList.contains('focus-visible')checks with the native:focus-visibleCSS pseudo-class selector. This modernizes the codebase by removing dependency on the polyfill while maintaining identical functionality for detecting keyboard-triggered focus.Also applies to: 87-87
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (1)
58-66: LGTM! Correctly prevents ref forwarding to non-forwardRef component.The destructuring of
refas_refproperly prevents it from being passed toCheckOption, which doesn't support ref forwarding. The explanatory comment is helpful for future maintainers.packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (1)
38-38: ****The original review comment flags an _id override risk that does not exist. Verification of the
useChannelsDatahook confirms that thelabelobject is explicitly constructed with onlyname,avatarETag, andtypeproperties—it does not contain an_idfield. Therefore, spreadinglabelin the room object construction cannot override the explicit_id: valueassignment. The code is safe as written.Likely an incorrect or invalid review comment.
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
37-37: False concern: label object does not contain _id property.The reviewed code at line 37 uses the spread pattern
{ _id: value, ...label, type: label?.type || 'c' }. While this pattern could theoretically be problematic iflabelcontained an_idproperty, the actual data structure shows thatlabelonly contains{ name, avatarETag, type }as defined inuseChannelsData.ts. No override of_idoccurs in practice. The code is currently safe and functions as intended.Likely an incorrect or invalid review comment.
9f91d38 to
866f932
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/gazzodown/package.json (1)
35-36: Verify package versions are published before release.The latest published version of @rocket.chat/fuselage is 0.66.4, not 0.67.0. The latest version of @rocket.chat/fuselage-hooks is 0.38.0, not 0.38.1. Both specified versions do not exist in the npm registry and will cause installation failures.
Additionally, the semver range change from
~to^significantly widens the acceptable version window for pre-1.0 packages, which increases the risk of inadvertently pulling breaking changes during future dependency updates.Update these to published versions:
- "@rocket.chat/fuselage": "^0.67.0", - "@rocket.chat/fuselage-hooks": "^0.38.1", + "@rocket.chat/fuselage": "^0.66.4", + "@rocket.chat/fuselage-hooks": "^0.38.0",Alternatively, if these future versions are expected to be released imminently as part of this PR, clarify the timeline and ensure that all related changes are coordinated across the monorepo.
🧹 Nitpick comments (1)
apps/uikit-playground/package.json (1)
21-23: Verify versioning strategy alignment across the codebase.Lines 21–22 shift from tilde (
~) to caret (^) constraints while bumping minor versions. This increases the permissible update range from patch-only to minor+patch for two fuselage packages. Meanwhile,@rocket.chat/fuselage-tokens(line 24) remains pinned with tilde.Confirm whether this versioning inconsistency is intentional or if all fuselage-related packages should adopt a uniform constraint strategy (either all
~or all^). Ensure the broader codebase follows a consistent policy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(1 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/package.json(3 hunks)apps/meteor/tests/e2e/page-objects/admin-settings.ts(1 hunks)apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts(2 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/meteor/tests/e2e/page-objects/admin-settings.ts
- packages/storybook-config/package.json
- packages/ui-composer/package.json
- packages/fuselage-ui-kit/package.json
- apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
- ee/packages/ui-theming/package.json
- packages/ui-client/src/components/Header/HeaderTitleButton.tsx
- packages/ui-client/package.json
- apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- packages/ui-avatar/package.json
- packages/ui-voip/package.json
- packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
🧠 Learnings (17)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.tsapps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.tsapps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/package.json
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/package.json
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Implement proper wait strategies for dynamic content
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/client/main.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts (1)
apps/meteor/tests/e2e/page-objects/admin-settings.ts (1)
AdminSettings(5-49)
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (13)
apps/uikit-playground/package.json (1)
21-23: Verify compatibility and breaking changes in upgraded fuselage packages.Upgrading
@rocket.chat/fuselageto 0.67.0 and@rocket.chat/fuselage-hooksto 0.38.1 (both minor version bumps) may include breaking changes. The AI summary mentions refactoring render props, destructuring changes, and:focus-visibleselector updates elsewhere in the PR, which suggests potential API or behavior shifts.Ensure all consuming code in this package has been tested and adapts to any breaking changes from these dependency upgrades.
packages/livechat/package.json (1)
35-35: Verify the version range change for pre-1.0 package.The version constraint changed from
~0.37.2to^0.38.1. For pre-1.0 packages, the caret range (^) is significantly more permissive than the tilde range (~):
- Tilde allows patches within the minor version (0.37.x only)
- Caret allows any version up to 1.0.0 (0.38.1 through 0.x.x)
This could introduce breaking changes if
@rocket.chat/fuselage-hooksreleases incompatible changes in upcoming minor versions. Confirm this is intentional as part of the broader fuselage upgrade strategy.apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts (1)
67-67: LGTM! Correct transition to selector-based focus detection.The changes correctly replace class-based focus-visible detection with the standard
:focus-visibleCSS pseudo-class selector. Both the blur and focus handlers now usematches(':focus-visible'), which is the appropriate modern approach for detecting keyboard-triggered focus.Also applies to: 83-83
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts (1)
71-71: LGTM! Consistent focus detection across sidebar variants.The changes correctly implement selector-based focus-visible detection using
matches(':focus-visible')in both event handlers, maintaining consistency with the similar changes in the other sidebar navigation file.Also applies to: 87-87
apps/meteor/client/views/room/hooks/useMessageListNavigation.ts (1)
76-76: LGTM! Excellent consistency across navigation hooks.The changes correctly implement the same selector-based focus-visible detection pattern using
matches(':focus-visible')that's applied in the sidebar navigation files. This demonstrates good consistency across different navigation contexts in the codebase.Also applies to: 92-92
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (1)
38-38: The review comment is based on an incorrect assumption about the label object structure.The
useChannelsDatahook shows thatlabelobjects are constructed as{ name: name || fname, avatarETag, type: t }and contain no_idproperty. Therefore, the_id: valueat line 38 cannot be overridden by the spread operator. The current code order is safe and correct—the explicittypeplacement after the spread is defensive programming to ensure the type field is handled appropriately.Likely an incorrect or invalid review comment.
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
37-37: The original review comment is based on an incorrect assumption about thelabelobject structure.Based on the test fixtures and actual usage throughout the codebase, the
labelobject contains only:name,avatarETag, andtype—it never contains an_idproperty. Therefore, the property order{ _id: value, ...label, type: label?.type || 'c' }is safe and causes no override issue with the_idparameter.The
renderItempattern (lines 51–54) uses an alternative approach with explicit properties, but both patterns are equally valid sincelabeldoes not have conflicting_idproperties. The current code is correct.Likely an incorrect or invalid review comment.
apps/meteor/client/main.ts (1)
5-8: The import paths are correct and consistent—no issues found.The review comment flagged an inconsistency that does not exist. The imports follow the correct pattern:
'./meteor/login'imports a directory with a barrel export inapps/meteor/client/meteor/login/'./ecdh'and'./importPackages'import files directly fromapps/meteor/client/'./startup'imports a directory with a barrel export inapps/meteor/client/startup/Each path correctly reflects the actual module location. The change to remove the fuselage-polyfills wrapper is valid.
packages/ui-contexts/package.json (1)
9-9: LGTM - Consistent with broader fuselage upgrades.The upgrade of @rocket.chat/fuselage-hooks from 0.37.2 to 0.38.1 with a version range change from
~to^is consistent with the broader set of fuselage-related dependency updates across this PR.apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts (1)
2-2: LGTM - Excellent refactoring to Page Object Model.The refactoring successfully migrates this test to use the Page Object Model pattern, which aligns perfectly with the coding guidelines for Playwright tests:
- ✅ Imports and uses the AdminSettings page object
- ✅ Replaces direct DOM interactions with page object methods
- ✅ Uses semantic locators (maintained from original code)
- ✅ Properly handles async operations with the savePromise pattern
The wait pattern is particularly well-designed: by setting up
savePromisebefore callingsaveChanges(), you ensure the API response is fully received before navigating back, which prevents race conditions given the 2-second delay in thebeforeEachroute interception.As per coding guidelines
Also applies to: 30-30, 36-40, 42-44
apps/meteor/package.json (1)
257-260: The review comment is based on incorrect assumptions about the PR changes.The verification shows that:
fuselage-polyfillsis not present in the currentpackage.jsonand was not removed in this PRapps/meteor/client/main.tshas not been modified in this PR and contains no polyfill-related imports or dynamic imports- The actual changes are only version bumps to fuselage-related packages (0.67.0, 0.1.1, 0.38.1, 0.35.1)
The AI summary's claim about polyfills being "removed and replaced with a direct dynamic import" does not match the actual code changes. The
:focus-visibleCSS pseudo-selector used throughout the codebase is standard modern browser support and does not require a polyfill package.Likely an incorrect or invalid review comment.
packages/ui-video-conf/package.json (1)
27-28: No issues found after verification.@rocket.chat/fuselage v0.67.0 introduced breaking changes that removed deprecated component namespaces, but investigation confirms this package uses only direct imports (Box, Button, ButtonGroup, Icon, Avatar, IconButton, Palette, Message, MessageDivider, Skeleton, Throbber) with no deprecated namespace patterns. The polyfills package removal does not impact this package, as it never depended on @rocket.chat/fuselage-polyfills. The upgraded versions are compatible with the existing codebase.
packages/web-ui-registration/package.json (1)
27-28: Version upgrades verified — no compatibility issues found.The verification confirms that the version range changes are safe for web-ui-registration:
- @rocket.chat/layout v0.34.0 has no documented breaking changes
- @rocket.chat/fuselage v0.67.0 removed deprecated namespaces and the fuselage-polyfills package, but web-ui-registration:
- Does not use any of the deprecated namespaces
- Does not directly import @rocket.chat/fuselage; it uses layout and fuselage-hooks instead
- Contains no polyfills dependencies
The 12 layout component imports (Form, ActionLink, VerticalWizardLayout, etc.) are compatible with v0.34.0.
866f932 to
7dfc357
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(1 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/package.json(3 hunks)apps/meteor/tests/e2e/page-objects/admin-settings.ts(1 hunks)apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts(2 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/components/UserStatusMenu.tsx
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/ui-client/src/components/Header/HeaderTitleButton.tsx
- apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx
- packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx
- apps/meteor/client/main.ts
- packages/web-ui-registration/package.json
- apps/meteor/client/views/room/hooks/useMessageListNavigation.ts
- packages/fuselage-ui-kit/package.json
- packages/ui-voip/package.json
- packages/ui-client/package.json
- apps/meteor/package.json
- packages/ui-video-conf/package.json
- apps/uikit-playground/package.json
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- apps/meteor/tests/e2e/page-objects/admin-settings.ts
- packages/storybook-config/package.json
- packages/ui-avatar/package.json
- apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
- packages/gazzodown/package.json
🧰 Additional context used
📓 Path-based instructions (3)
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
🧠 Learnings (12)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.tsapps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts (1)
apps/meteor/tests/e2e/page-objects/admin-settings.ts (1)
AdminSettings(5-49)
⏰ 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). (1)
- GitHub Check: CodeQL-Build
🔇 Additional comments (10)
apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (1)
34-37: LGTM! Proper ref handling for non-forwardRef component.The destructuring of
ref: _refcorrectly prevents forwarding the ref toCheckOption, avoiding React warnings. The underscore prefix follows the convention for intentionally unused variables, and the explanatory comment adds helpful context.packages/livechat/package.json (1)
35-35: Upgrade is consistent and correct across the codebase.Verification confirms that
@rocket.chat/fuselage-hookshas been uniformly upgraded to^0.38.1across all packages in the repository. Related dependencies like@rocket.chat/fuselage-tokensremain appropriately at~0.33.2, and@rocket.chat/fuselage-polyfillshas been properly removed. The versioning strategy is intentional and consistent.packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx (1)
17-17: LGTM! Modern focus-visible selector.The migration from the class-based
.focus-visibleto the native:focus-visiblepseudo-class is correct and aligns with modern CSS standards. This removes the dependency on JavaScript-managed focus classes.apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts (1)
71-71: LGTM! Modernized focus detection.Both changes correctly replace class-based focus-visible detection with the native
matches(':focus-visible')check. This approach leverages the browser's built-in focus visibility heuristics and is the standard method for programmatically checking keyboard focus state.Also applies to: 87-87
ee/packages/ui-theming/package.json (1)
8-9: LGTM: Fuselage package upgrades.The upgrade to fuselage ^0.67.0 and fuselage-hooks ^0.38.1 looks good. Note that the version range changed from tilde (~) to caret (^), which allows minor version updates instead of just patch updates.
packages/ui-contexts/package.json (1)
9-9: LGTM: Fuselage-hooks upgrade.The upgrade to ^0.38.1 is consistent with the broader PR upgrade strategy.
packages/ui-composer/package.json (1)
27-28: LGTM: Fuselage package upgrades.The upgrades are consistent with the broader PR changes.
apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts (2)
2-2: Good use of Page Object Model.The introduction of the AdminSettings page object improves test maintainability and follows the established pattern.
Also applies to: 30-30
36-39: Improved coordination of UI and API actions.Using Promise.all to coordinate the save button click with the API response wait is better than sequential operations. This ensures proper synchronization.
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
37-37: Approve defensive reordering; flag inconsistent type handling in renderItem.The reordering ensures
_idandtypeoverride spread properties fromlabel, which is more defensive. However, there's an inconsistency:renderSelected(line 37) useslabel?.type || 'c'with a fallback, butrenderItem(lines 51-52) useslabel.typewithout optional chaining or fallback. Iflabel.typeis undefined,renderItemwill passundefinedinstead of defaulting to'c'.Consider aligning
renderItemwithrenderSelected's pattern—line 49 inMultiChannelsSelectElement.tsxshows the consistent approach.
7dfc357 to
bd529d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/meteor/overrides/ddpOverREST.ts(2 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(1 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/package.json(3 hunks)apps/meteor/tests/e2e/page-objects/admin-settings.ts(1 hunks)apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts(2 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/meteor/client/meteor/overrides/ddpOverREST.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx
- ee/packages/ui-theming/package.json
- packages/fuselage-ui-kit/package.json
- apps/meteor/tests/e2e/page-objects/admin-settings.ts
- packages/ui-client/package.json
- packages/gazzodown/package.json
- packages/web-ui-registration/package.json
- packages/ui-contexts/package.json
- apps/meteor/package.json
- apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
- packages/livechat/package.json
- apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
- packages/ui-avatar/package.json
- packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx
- packages/ui-video-conf/package.json
- packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx
- packages/ui-voip/package.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/main.tsapps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (11)
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-19: LGTM: Clean migration to native:focus-visibleselector.The change from
&:focus.focus-visibleto&:focus-visiblecorrectly adopts the native CSS pseudo-class, eliminating the need for JavaScript-managed focus-visible classes. This is more reliable and aligns with modern CSS practices.apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts (1)
72-72: LGTM: Correct migration to selector-based focus-visible detection.Both the blur handler (line 72) and focus handler (line 88) now correctly use
matches(':focus-visible')instead of class-based detection. This aligns with the native CSS:focus-visiblepseudo-class and is more reliable than checking for a programmatically added class.Also applies to: 88-88
apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts (1)
67-67: LGTM: Focus-visible detection correctly implemented.The blur handler (line 67) and focus handler (line 83) now correctly use
matches(':focus-visible'). The past review comment indicates a typo (:sfocus-visible) was already addressed in commit ad27772, and the current implementation is correct.Also applies to: 83-83
apps/meteor/client/views/room/hooks/useMessageListNavigation.ts (1)
76-76: LGTM: Consistent focus-visible detection across message list navigation.The blur handler (line 76) and focus handler (line 92) correctly use
matches(':focus-visible'), maintaining consistency with the refactor applied to other navigation hooks throughout the codebase.Also applies to: 92-92
packages/ui-composer/package.json (2)
27-28: Verify that the caret constraint is appropriate for these package upgrades.The version specifiers changed from tilde (
~) to caret (^), making constraints more permissive. This allows minor version updates within the same major version (e.g., 0.67.x for fuselage, 0.38.x for fuselage-hooks) rather than only patch updates.Confirm that:
- The fuselage 0.67.0 and fuselage-hooks 0.38.1 releases contain no breaking changes relevant to
@rocket.chat/ui-composer.- The looser constraint aligns with your project's dependency management strategy.
22-62: Verify peerDependencies constraint for @rocket.chat/fuselage.The removal of
@rocket.chat/fuselage-polyfillsfrom devDependencies is confirmed and complete—no remaining references in the codebase or package.json.However, the peerDependencies constraint for
@rocket.chat/fuselageremains"*"while devDependencies specifies^0.67.0. Verify whether this constraint should be tightened to^0.67.0to match the version this package is tested against, or if the broader"*"is intentional to support consumers on other versions.apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (1)
58-62: Correctly prevents ref forwarding to non-forwardRef component.The destructuring pattern properly extracts the
refinto_refand spreads only the remaining props toCheckOption, which doesn't support ref forwarding. The comment clearly explains the rationale.apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (1)
34-37: Clean inline destructuring approach.The inline destructuring of
refin the renderItem parameter achieves the same goal as the related change inAutoCompleteDepartmentMultiple.tsxbut in a more concise manner. Both implementations correctly prevent ref forwarding to the non-forwardRefCheckOptioncomponent.packages/storybook-config/package.json (1)
5-21: Removal of@rocket.chat/fuselage-polyfillsis safe for Storybook setup.Comprehensive verification confirms no issues:
- No direct imports of fuselage-polyfills in storybook-config package or any active code
- Storybook configuration (main.ts) contains only addon setup with no polyfill dependencies
- Other fuselage packages remain intact in dependencies
- The package was not being referenced or used in the storybook configuration
apps/meteor/client/main.ts (1)
6-8: The import paths are correct; no changes needed.The verification confirms that the files are located directly in
apps/meteor/client/(not in a./meteor/subdirectory):
apps/meteor/client/ecdh.tsapps/meteor/client/importPackages.tsapps/meteor/client/startup/startup.tsThe imports in lines 6-8 (
'./ecdh','./importPackages','./startup') correctly resolve to these files. Line 5's different import pattern ('./meteor/login') is expected because that module is in a separate./meteor/subdirectory. There is no module resolution error.Likely an incorrect or invalid review comment.
apps/uikit-playground/package.json (1)
21-23: All package versions verified—no compatibility issues found.All three specified versions exist and are published on npm: @rocket.chat/fuselage@0.67.0, @rocket.chat/fuselage-hooks@0.38.1, and @rocket.chat/fuselage-toastbar@0.35.1. Peer dependencies are compatible, with broad requirements for react, react-dom, react-virtuoso, and other standard packages. The version specifier change from
~to^is standard npm practice and intentional for these minor version bumps.
06a6aa9 to
ac302c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/storybook-config/package.json (1)
7-7: Fix: Specified package versions do not exist on npm and will cause installation failures.Web verification (Nov 11, 2025) confirms:
@rocket.chat/fuselage-hooks: You specified^0.38.1but latest published version is0.37.1@rocket.chat/fuselage: You specified^0.67.0but latest published version is0.66.3Lines 7, 24: Update to the published versions. If you intend to use pre-release or private registry versions, clarify the source and coordinate with your team on version pinning strategy.
Also note: You've shifted from tilde (
~) to caret (^) range specifiers. Confirm this constraint change is intentional and aligns with your monorepo's dependency management approach.
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (1)
58-66: Correct ref handling for non-forwardRef component.The destructuring of
refprevents it from being forwarded toCheckOption, which is not a forwardRef component. This avoids React warnings about unknown props.Note:
AutoCompleteMonitors.tsxuses a more concise inline destructuring pattern in itsrenderItem(ref: _refin the parameter list). Consider using the same pattern for consistency.Apply this diff for consistency with AutoCompleteMonitors.tsx:
- const renderItem = ({ label, value, ...props }: ComponentProps<typeof Option>): ReactElement => { + const renderItem = ({ label, value, ref: _ref, ...props }: ComponentProps<typeof Option>): ReactElement => { if (withCheckbox) { - // CheckOption is not a forwardRef component. - const { ref: _ref, ...restProps } = props; return ( <CheckOption - {...restProps} + {...props} label={<span style={{ whiteSpace: 'normal' }}>{label}</span>} selected={value ? selectedValues.has(value) : false} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(1 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/package.json(5 hunks)apps/meteor/tests/e2e/page-objects/admin-settings.ts(1 hunks)apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts(2 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/ui-contexts/package.json
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/ui-client/package.json
- packages/ui-avatar/package.json
- packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx
- packages/gazzodown/package.json
- packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx
- apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
- packages/livechat/package.json
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
- apps/meteor/client/views/room/hooks/useMessageListNavigation.ts
- apps/meteor/client/main.ts
- apps/meteor/package.json
🧰 Additional context used
📓 Path-based instructions (3)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
🧠 Learnings (12)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.tsapps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.tspackages/ui-voip/package.json
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/page-objects/admin-settings.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Follow the Playwright Testing Guide and Rocket.Chat documentation as primary references
Applied to files:
packages/ui-voip/package.json
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (12)
packages/ui-video-conf/package.json (1)
27-28: Verify semver range expansion from tilde to caret.The version specifier change from
~0.66.4to^0.67.0expands the acceptable range from patch updates only (0.66.x) to minor and patch updates (0.67.x). Please confirm this is intentional and that the new minor version (0.67.0) is backward compatible with your codebase.packages/web-ui-registration/package.json (1)
27-28: Confirm compatibility of multiple minor version bumps.This file updates fuselage, fuselage-hooks, and layout to caret ranges spanning new minor versions. Please verify that all three packages (fuselage 0.67.0, fuselage-hooks 0.38.1, layout 0.34.0) are mutually compatible and contain no breaking changes affecting your UI registration flows.
Also applies to: 32-32
packages/ui-voip/package.json (1)
34-35: Verify fuselage compatibility updates alongside unrelated @playwright/test removal.The fuselage and fuselage-hooks updates follow the expected pattern. However, this file also removes
@playwright/test, which appears unrelated to the fuselage upgrade objective. Please clarify whether this removal is intentional and whether it affects E2E testing for VoIP features.ee/packages/ui-theming/package.json (1)
8-9: LGTM — Dependency updates follow the consistent pattern across the PR.packages/fuselage-ui-kit/package.json (1)
56-57: LGTM — Dependency updates for the core UI kit package are consistent with ecosystem-wide changes.apps/uikit-playground/package.json (1)
21-23: LGTM — Dependency updates follow the consistent pattern. Note that fuselage is independencies(not devDependencies), making these updates visible to consumers of the uikit-playground package.apps/meteor/tests/e2e/page-objects/admin-settings.ts (1)
46-48: LGTM!Clean wrapper method that follows the Page Object Model pattern and DRY principle. The method provides a convenient API for tests while properly returning the promise from the underlying click action.
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-15: LGTM! Good modernization to native:focus-visible.This change removes the dependency on class-based focus-visible polyfills and uses the native CSS pseudo-class, which is now well-supported across modern browsers.
apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (1)
34-37: LGTM! Clean ref handling for non-forwardRef component.The inline destructuring of
refprevents it from being forwarded toCheckOption, which is not a forwardRef component. This pattern is concise and clear.apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts (1)
67-67: LGTM! Focus-visible detection correctly migrated to native CSS pseudo-class.The changes properly replace the polyfill-based class detection with the native
:focus-visiblepseudo-class selector. The logic in both handlers is correct:
- Line 67 (blur): Checks if the element gaining focus matches
:focus-visible- Line 83 (focus): Determines keyboard-triggered focus by checking if the target matches
:focus-visibleThe optional chaining appropriately handles null/undefined cases.
Also applies to: 83-83
packages/ui-composer/package.json (1)
27-28: No critical issues found. Fuselage upgrade is compatible with this package.Verification confirms:
- ✓
@rocket.chat/fuselage-polyfillsremoval is complete—zero polyfill references detected in source code- ✓ All fuselage imports (Box, Button, Icon, IconButton, Divider, etc.) are standard, stable APIs not affected by the version upgrade
- ✓ No polyfill-dependent code or focus-visible references remain
- ✓ Peer dependency constraint (
*) is compatible with^0.67.0The codebase has been properly updated to work without polyfills and is fully compatible with the new versions.
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (1)
38-38: Verify this is an intentional behavioral change required by the fuselage upgrade.The property order change from
{ type: label?.type || 'c', _id: value, ...label }to{ _id: value, ...label, type: label?.type || 'c' }is semantically different, contrary to the AI summary. By movingtypeafter the spread operator, the code now guarantees thattypeis never overridden by properties fromlabeland will always be at least'c'.The old pattern allowed
label.typeto override the fallback, potentially resulting in falsy values. This change appears isolated to fuselage-ui-kit, while similar patterns in other files (e.g.,RoomAutoComplete.tsx,RoomAutoCompleteMultiple.tsx) still use the old pattern. Confirm this change aligns with the fuselage upgrade requirements and determine if other components need the same fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
fuselage.sh (2)
83-98: Critical: Successful operations still exit with error code 1.This issue was flagged in the previous review and remains unresolved. Line 98 exits with code 1 after the "next-all"/"latest-all" upgrade completes successfully, signaling failure to calling processes. This breaks CI/CD pipelines and scripts that rely on standard Unix exit semantics.
The package removals (fuselage-polyfills and message-parser are no longer listed) align well with the PR objectives.
Apply this diff to fix the exit code:
eval "yarn up @rocket.chat/emitter@$targetVersion @rocket.chat/fuselage-toastbar@$targetVersion @rocket.chat/fuselage-tokens@$targetVersion @rocket.chat/css-in-js@$targetVersion @rocket.chat/styled@$targetVersion @rocket.chat/fuselage@$targetVersion @rocket.chat/fuselage-hooks@$targetVersion @rocket.chat/icons@$targetVersion @rocket.chat/logo@$targetVersion @rocket.chat/memo@$targetVersion @rocket.chat/onboarding-ui@$targetVersion @rocket.chat/string-helpers@$targetVersion @rocket.chat/layout@$targetVersion" - exit 1 + exit 0
101-107: Critical: Successful operations exit with error code 1 (second occurrence).The same exit code issue persists on line 106 for the "next"/"latest" upgrade path. After successfully upgrading individual packages, the script exits with code 1 instead of 0, which breaks automation and CI/CD semantics.
Apply this diff to fix the exit code:
eval "yarn up @rocket.chat/$i@$action" done - exit 1 + exit 0
🧹 Nitpick comments (1)
ee/packages/ui-theming/package.json (1)
8-8: Align devDependencies and peerDependencies version constraints.The devDependencies now use tilde (
~) constraints (lines 8, 11), but peerDependencies (lines 40-42) allow any version (*). This inconsistency may cause version mismatch issues for consumers of this package.Consider aligning peerDependencies to match the devDependencies constraints for
@rocket.chat/fuselage,@rocket.chat/fuselage-hooks, and@rocket.chat/icons."peerDependencies": { "@rocket.chat/css-in-js": "*", - "@rocket.chat/fuselage": "*", - "@rocket.chat/fuselage-hooks": "*", + "@rocket.chat/fuselage": "~0.68.1", + "@rocket.chat/fuselage-hooks": "~0.38.1", - "@rocket.chat/icons": "*", + "@rocket.chat/icons": "~0.45.0", "react": "*" }Also applies to: 11-11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(2 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/ee/server/services/package.json(1 hunks)apps/meteor/package.json(3 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/federation-matrix/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)fuselage.sh(1 hunks)packages/core-services/package.json(1 hunks)packages/core-typings/package.json(1 hunks)packages/fuselage-ui-kit/README.md(1 hunks)packages/fuselage-ui-kit/package.json(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-kit/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/web-ui-registration/package.json
- packages/ui-video-conf/package.json
- apps/meteor/ee/server/services/package.json
- packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- apps/meteor/client/views/room/hooks/useMessageListNavigation.ts
- apps/uikit-playground/package.json
- apps/meteor/package.json
- apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
- packages/fuselage-ui-kit/README.md
- packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx
- packages/fuselage-ui-kit/package.json
- packages/ui-avatar/package.json
- packages/ui-client/package.json
- packages/core-services/package.json
- apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx
- packages/ui-voip/package.json
- packages/gazzodown/package.json
- packages/storybook-config/package.json
- ee/packages/federation-matrix/package.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/client/main.ts
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (14)
packages/ui-kit/package.json (1)
43-43: Verify peerDependencies consistency with devDependencies version constraint.The change from
^0.45.0to~0.45.0tightens the version constraint, which is good for stability. However, line 60 specifies"@rocket.chat/icons": "*"in peerDependencies, allowing any version. Consider whether peerDependencies should also be constrained to align with the devDependency specification.If this narrower constraint is driven by breaking changes or stability concerns in
@rocket.chat/icons, the peerDependency should likely be updated to prevent incompatible versions from being installed by consumers:- "peerDependencies": { - "@rocket.chat/icons": "*" - }, + "peerDependencies": { + "@rocket.chat/icons": "~0.45.0" + },If permissive peerDependencies are intentional, please confirm this is consistent across similar package.json files in the PR.
packages/ui-composer/package.json (3)
27-27: Version constraint tightening is pragmatic for stability.The shift from caret (
^) to tilde (~) constraints on@rocket.chat/fuselage(line 27) and@rocket.chat/icons(line 30) restricts updates to patch-level changes only, which is a sound stability practice. Verify that the pinned versions have been tested thoroughly in this package's context before merge.Also applies to: 30-30
28-28: Verify fuselage-hooks 0.38.1 compatibility.The bump from
~0.37.2to~0.38.1for@rocket.chat/fuselage-hooks(line 28) is a minor version upgrade. Confirm that this version introduces no breaking changes to this package's consumers or usage patterns. Cross-check with other packages in the PR to ensure consistent adoption.
56-62: Verify peer dependency constraint alignment.The
peerDependenciessection (line 58) permits*for@rocket.chat/fuselage, while thedevDependencies(line 27) now pins it to~0.68.1. This mismatch could allow downstream consumers to use incompatible versions. Either pin the peer dependency to~0.68.1(or a compatible range) or document the version compatibility rationale.apps/meteor/client/main.ts (1)
5-5: LGTM! Verify browser compatibility across supported environments.The removal of the fuselage-polyfills wrapper simplifies the startup sequence. However, since polyfills typically provide compatibility features for older browsers, please confirm that the application has been tested across all supported browser versions and that no regressions have been observed.
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-15: LGTM! Native:focus-visiblepseudo-class adoption.The change correctly replaces the class-based selector with the native CSS
:focus-visiblepseudo-class, aligning with the removal of focus-visible polyfills across the codebase.packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx (1)
17-17: LGTM! Consistent adoption of native:focus-visible.The change maintains consistency with the non-V2 HeaderTitleButton variant and correctly adopts the native CSS pseudo-class.
apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (2)
3-3: LGTM! Import cleanup aligns with simplified renderItem.The removal of
OptionandComponentPropsWithoutRefimports is consistent with the simplified renderItem implementation that no longer requires explicit type annotations.
34-36: LGTM! Simplified renderItem with intentionally ignored ref.The ref parameter is renamed to
_refto signal it's intentionally unused. This is a valid pattern and consistent with the simplified renderItem implementations across the codebase.apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts (1)
71-71: LGTM! Consistent migration to native focus-visible detection.The changes correctly replace class-based focus-visible detection with the native CSS selector matching. The logic is preserved in both the blur handler (line 71) and focus handler (line 87), with only the detection mechanism updated.
Also applies to: 87-87
packages/livechat/package.json (1)
35-35: Version 0.38.1 does not exist; only 0.38.0 is available on npm.@rocket.chat/fuselage-hooks ^0.38.1 does not exist (latest published is 0.38.0). Additionally, the shell script found no references to
@rocket.chat/fuselage-hooksin any package.json files in the repository. Verify that:
- The correct version (0.38.0) is intended
- This file and line number match the actual PR changes
Likely an incorrect or invalid review comment.
packages/core-typings/package.json (1)
34-34: No actionable changes required.The @rocket.chat/icons v0.45.0 package exists and is the latest version. The version specifier change from
^0.45.0to~0.45.0is functionally equivalent for this version—both allow patch updates only (≥0.45.0 <0.46.0). No known vulnerabilities were found in the package. The package demonstrates healthy maintenance with regular releases.The dependency is consistent with the broader Fuselage upgrade pattern in this PR and poses no compatibility concerns for core-typings.
ee/packages/ui-theming/package.json (2)
8-8: No issues found - fuselage-polyfills removal is clean.Verification confirms that
@rocket.chat/fuselage-polyfillshas been successfully removed and is not imported or used anywhere in the codebase. The only references found are in HISTORY.md changelog entries, which is expected for historical documentation.
8-11: Version pinning strategy is consistent and intentional across the codebase.Verification confirms the changes are part of a coordinated upgrade, not isolated modifications:
- The tilde pinning strategy (
~) for@rocket.chat/fuselage,@rocket.chat/fuselage-hooks,@rocket.chat/fuselage-tokens, and@rocket.chat/iconsis uniformly applied across all UI packages in the codebase (ui-contexts, ui-voip, ui-video-conf, web-ui-registration, ui-composer, ui-avatar, and others).- No references to
fuselage-polyfillsexist anywhere in the codebase, making its removal safe.- The
fuselage-hooks0.38.1 release was published on Oct 24, 2025, and the hooks actively used throughout the codebase (e.g.,useDarkMode,useDebouncedValue,useSafely,useLocalStorage) are stable utility functions with no breaking changes.- The pattern of restricting dependencies with
~while allowing peerDependencies with*is intentional and consistent across all packages.
083bde7 to
e31ade7
Compare
dougfabris
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5011114 to
258fb07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-15: Good modernization to native:focus-visiblepseudo-class.The change from
&:focus.focus-visible(polyfill class) to&:focus-visible(native pseudo-class) correctly modernizes the focus management and aligns with the removal of fuselage-polyfills. This is consistent with the parallel change in HeaderV2/HeaderTitleButton.tsx.Optional: A past reviewer suggested exploring the
focusedBox prop instead of custom focus CSS. If Fuselage's Box component now provides afocusedprop that handles this styling pattern natively, that could further simplify this component. Based on past review comments.packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
43-60: Consider aligning renderItem with renderSelected pattern for consistency.The
renderItemfunction constructs the room object explicitly, whilerenderSelected(line 37) now uses the spread pattern{ _id: value, ...label, type: label?.type || 'c' }. This inconsistency differs fromMultiChannelsSelectElement.tsx, where both functions use the same spread pattern (lines 38 and 49).Consider updating
renderItemto match:renderItem={({ value, label, ...props }) => ( <Option key={value} {...props} label={label.name} - avatar={ - <RoomAvatar - size='x20' - room={{ - type: label.type, - _id: value, - avatarETag: label.avatarETag, - }} - {...props} - /> - } + avatar={<RoomAvatar size='x20' room={{ _id: value, ...label, type: label?.type || 'c' }} />} /> )}This would provide consistent null-safety (
label?.type) and ensure uniform prop handling across both render functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(2 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)fuselage.sh(1 hunks)packages/fuselage-ui-kit/README.md(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx(2 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx
- packages/fuselage-ui-kit/README.md
- apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
- fuselage.sh
- apps/meteor/client/views/room/hooks/useMessageListNavigation.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/client/main.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/client/main.ts
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (2)
4-4: LGTM: Import simplification.The change from
ComponentPropsWithoutRefandReactElementto justComponentPropssimplifies the imports and is sufficient for the type definitions used in this file.
69-81: LGTM: Clean inline renderItem with proper ref handling.The inline renderItem implementation is well-structured:
- The
ref: _refpattern correctly prevents the ref from being spread to child components- The conditional rendering logic is preserved correctly
- The defensive check
value ? selectedValues.has(value) : falsesafely handles potential undefined valuespackages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
37-37: Property ordering ensures type safety.The reordering correctly ensures that
type: label?.type || 'c'takes precedence even iflabelcontains atypeproperty, preventing potential bugs.packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (1)
38-38: LGTM! Consistent pattern across render functions.Both
renderSelectedandrenderItemnow use the same property ordering pattern{ _id: value, ...label, type: label?.type || 'c' }, which ensures type safety and consistency throughout the component.Also applies to: 49-49
apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts (1)
71-71: Refactor is consistent with existing codebase patterns—no issues found.Verification confirms this change aligns with the existing codebase:
- The
:focus-visiblepseudo-class withmatches()is already applied in identical contexts across multiple navigation hooks (useMessageListNavigation.ts,useSidebarListNavigation.tsin other locations)- Client-side TypeScript target is ES2015, which is modern enough for this refactoring
- The pattern is established and idiomatic within the project
6a249f4 to
c848f2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/ui-client/package.json (1)
26-29: Critical: Specified Fuselage dependency versions do not exist in npm registry.The latest published version of @rocket.chat/fuselage is 0.66.4, and the latest published version of @rocket.chat/fuselage-hooks is 0.38.0. The package.json specifies versions that are not available:
- Line 26:
@rocket.chat/fuselage@~0.68.1— does not exist- Line 27:
@rocket.chat/fuselage-hooks@~0.38.1— does not existThis will cause dependency resolution failures. Update to valid, published versions (e.g., ~0.66.4 for fuselage, ~0.38.0 for fuselage-hooks) before merging.
packages/gazzodown/package.json (1)
35-38: Critical: Specified Fuselage dependency versions do not exist in npm registry.This file still contains the critical version issue flagged in the past review. The latest published version of @rocket.chat/fuselage is 0.66.4, and the latest published version of @rocket.chat/fuselage-hooks is 0.38.0. The package.json specifies versions that are not available:
- Line 35:
@rocket.chat/fuselage@~0.68.1— does not exist- Line 36:
@rocket.chat/fuselage-hooks@~0.38.1— does not existUpdate to valid, published versions before merging. The past review comment covers this in detail.
packages/ui-video-conf/package.json (1)
27-30: Critical: Specified Fuselage dependency versions do not exist in npm registry.The latest published version of @rocket.chat/fuselage is 0.66.4, and the latest published version of @rocket.chat/fuselage-hooks is 0.38.0. The package.json specifies versions that are not available:
- Line 27:
@rocket.chat/fuselage@~0.68.1— does not exist- Line 28:
@rocket.chat/fuselage-hooks@~0.38.1— does not existThis will cause dependency resolution failures. Update to valid, published versions before merging.
packages/ui-client/src/components/Header/HeaderTitleButton.tsx (1)
15-19: LGTM! Consider the previous suggestion to use Fuselage'sfocusedprop.The migration to native
&:focus-visibleis correct and consistent with the broader PR changes. As previously suggested, using the Box component'sfocusedprop would provide a cleaner, more maintainable solution that leverages Fuselage's built-in styling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
apps/meteor/client/components/UserStatusMenu.tsx(0 hunks)apps/meteor/client/main.ts(1 hunks)apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts(2 hunks)apps/meteor/client/views/room/hooks/useMessageListNavigation.ts(2 hunks)apps/meteor/ee/server/services/package.json(1 hunks)apps/meteor/package.json(3 hunks)apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts(1 hunks)apps/uikit-playground/package.json(1 hunks)ee/packages/federation-matrix/package.json(1 hunks)ee/packages/ui-theming/package.json(1 hunks)fuselage.sh(1 hunks)packages/core-services/package.json(1 hunks)packages/core-typings/package.json(1 hunks)packages/gazzodown/package.json(1 hunks)packages/livechat/package.json(1 hunks)packages/storybook-config/package.json(2 hunks)packages/ui-avatar/package.json(1 hunks)packages/ui-client/package.json(1 hunks)packages/ui-client/src/components/Header/HeaderTitleButton.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx(1 hunks)packages/ui-composer/package.json(1 hunks)packages/ui-contexts/package.json(1 hunks)packages/ui-kit/package.json(1 hunks)packages/ui-video-conf/package.json(1 hunks)packages/ui-voip/package.json(1 hunks)packages/web-ui-registration/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneSelector.tsx
- apps/meteor/client/components/UserStatusMenu.tsx
✅ Files skipped from review due to trivial changes (3)
- packages/storybook-config/package.json
- ee/packages/federation-matrix/package.json
- packages/core-services/package.json
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/meteor/client/sidebar/RoomList/useSidebarListNavigation.ts
- apps/meteor/client/views/navigation/sidebar/RoomList/useSidebarListNavigation.ts
- fuselage.sh
- apps/meteor/client/main.ts
- apps/meteor/client/sidebarv2/RoomList/useSidebarListNavigation.ts
- apps/meteor/package.json
- apps/meteor/tests/e2e/settings-persistence-on-ui-navigation.spec.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
packages/ui-contexts/package.json
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (11)
apps/meteor/ee/server/services/package.json (1)
51-51: Change is consistent with codebase version strategy.Verification confirms that
~0.45.0for@rocket.chat/iconsis consistently applied across 20+ package.json files throughout the codebase as the primary dependency constraint. The tighter patch-level versioning aligns with the established upgrade strategy across packages/, apps/meteor/, and ee/packages/ directories.packages/ui-composer/package.json (1)
27-30: Version pinning strategy is sound; verify repository conventions.The shift from caret (
^) to tilde (~) ranges for Fuselage packages is reasonable for production stability, as it allows only patch-level updates. The peer dependencies (lines 56–61) use wildcards, which remain compatible with these constraints. Ensure this versioning strategy aligns with your monorepo's dependency management policy.packages/livechat/package.json (1)
35-35: fuselage-hooks version bump is appropriate.The update to
~0.38.1aligns with the broader Fuselage upgrade and maintains compatibility with existing peer dependencies. The past concern about version availability has been validated by the maintainer.ee/packages/ui-theming/package.json (1)
8-11: Version updates are consistent and appropriate.The tilde pinning for Fuselage packages aligns with the repository-wide upgrade strategy. Peer dependencies are compatible with the updated dev dependency versions.
packages/ui-kit/package.json (1)
43-43: icons version update follows established pattern.The tilde constraint for
@rocket.chat/iconsaligns with the broader upgrade strategy and remains compatible with the peer dependency wildcard.packages/core-typings/package.json (1)
34-34: Production dependency version update is appropriate.The tilde constraint for
@rocket.chat/iconsin dependencies (line 34) maintains consistency with the upgrade strategy and allows patch-level updates while preventing minor version breaking changes.packages/ui-voip/package.json (1)
35-38: Fuselage ecosystem version updates are consistent and well-aligned.The tilde constraints for Fuselage, fuselage-hooks, and icons are appropriate for a core UI package with extensive peer dependencies. The wildcard peer dependencies remain compatible with these constraints.
packages/ui-contexts/package.json (1)
9-9: fuselage-hooks version bump is appropriate and verified.The update to
~0.38.1maintains consistency with the broader upgrade effort. This version is available on npm (as confirmed in prior discussions) and the tilde constraint is appropriate for this core contexts package.packages/ui-avatar/package.json (1)
9-12: Fuselage dependency updates follow established pattern.The version constraints for Fuselage packages are consistent with other UI packages in the upgrade effort. Peer dependencies remain compatible with these tilde-constrained versions. The removal of
fuselage-polyfills(noted in the summary) aligns with the PR's objective to modernize the Fuselage setup.apps/meteor/client/views/room/hooks/useMessageListNavigation.ts (1)
76-76: LGTM! Focus-visible detection correctly migrated to native CSS selector.The migration from
classList.contains('focus-visible')tomatches(':focus-visible')correctly aligns with the removal of the polyfill. The logic remains functionally equivalent:
- Line 76: Checks focus-visible state during blur for tracking purposes
- Line 92: Determines keyboard-triggered focus for navigation behavior
Also applies to: 92-92
packages/ui-client/src/components/HeaderV2/HeaderTitleButton.tsx (1)
17-21: :focus-visible support is adequate; verify the "focused" prop suggestion against project requirements.The current implementation using native
&:focus-visibleis correct and aligns with the polyfill removal. Browser support is modern: Chrome 86+, Edge 86+, Firefox 85+, Safari 15.4+, and Opera 72+.However, I cannot verify from the codebase whether the Fuselage Box component (v0.68.1) actually provides a
focusedprop, as there are no existing usages and web documentation is inconclusive. Confirm this prop exists and is recommended before refactoring. Additionally, verify that the project's browser target requirements don't mandate support for older environments requiring the:focus-visiblepolyfill.
c848f2b to
220f7bd
Compare
220f7bd to
652490c
Compare

Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Dependencies
Tests