-
Notifications
You must be signed in to change notification settings - Fork 13k
refactor: Adjusts HeaderToolbarAction props
#37108
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 not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughRefactors header/toolbox actions to use icon/title/tooltip props and onClick handlers, introduces RoomToolbar and omnichannel quick-action toolbars for e2e page objects, adds modal page-objects, updates many e2e tests to use the toolbar API, and adds accessibility aria-labels to two modals. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as E2E Test
participant PO as PageObject
participant Toolbar as RoomToolbar
participant UI as UI
Note over Test,UI: Old tabs-based flow
Test->>PO: tabs.btnTabMembers.click()
PO->>UI: click tab member button
UI-->>PO: members tab shown
Note over Test,UI: New toolbar-based flow
Test->>PO: roomToolbar.openMembersTab()
PO->>Toolbar: openMembersTab()
Toolbar->>UI: trigger members view
UI-->>Toolbar: members view shown
sequenceDiagram
participant Caller as Component/Hook
participant HTA as HeaderToolbarAction
participant IconBtn as IconButton
Note over Caller,IconBtn: Before
Caller->>HTA: props { id, index, action, icon, data-qa-id, tooltip }
HTA->>IconBtn: render with onClick={action}, title/tooltip
Note over Caller,IconBtn: After
Caller->>HTA: props { icon, title, tooltip, onClick }
HTA->>IconBtn: render with title={tooltip || title} and onClick={onClick}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
🔇 Additional comments (2)
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 #37108 +/- ##
===========================================
- Coverage 67.59% 67.57% -0.02%
===========================================
Files 3297 3339 +42
Lines 112740 113919 +1179
Branches 20466 20659 +193
===========================================
+ Hits 76201 76977 +776
- Misses 33864 34269 +405
+ Partials 2675 2673 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
a1b48b2 to
05533b3
Compare
05533b3 to
2c88ade
Compare
353c486 to
9162e22
Compare
f294850 to
a5a7831
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
🧹 Nitpick comments (7)
apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-message-bubble-color.spec.ts (1)
99-100: Close via toolbar looks good; add a post-condition assertionAfter
quickActionsRoomToolbar.closeChat(...), assert a stable indicator (e.g., “Closed” badge/status or presence of a “Reopen” action) to ensure the room actually closed.packages/ui-client/src/components/Header/HeaderToolbar/HeaderToolbarAction.tsx (2)
8-13: Add accessible name; prefer settingaria-label(and keep title for tooltip)Ensure screen readers have an explicit accessible name. Map tooltip/title to
aria-labelas well.- return <IconButton ref={ref} icon={icon} small position='relative' overflow='visible' title={tooltip ?? title} {...props} />; + return ( + <IconButton + ref={ref} + icon={icon} + small + position='relative' + overflow='visible' + aria-label={tooltip ?? title} + title={tooltip ?? title} + {...props} + /> + );
4-6: Optionally tighten prop typing to requireicon/titleIf this action should always show an icon, make it explicit in the type to catch mistakes at compile time.
-type HeaderToolbarActionProps = ComponentProps<typeof IconButton> & { - tooltip?: string; -}; +type HeaderToolbarActionProps = Omit<ComponentProps<typeof IconButton>, 'icon' | 'title'> & { + icon: NonNullable<ComponentProps<typeof IconButton>['icon']>; + title: string; + tooltip?: string; +};apps/meteor/client/views/room/HeaderV2/RoomToolbox/hooks/useRoomToolboxActions.spec.ts (1)
51-51: Consider using a placeholder icon or documenting the undefined cast.The double type cast
undefined as unknown as RoomToolboxActionConfig['icon']is awkward. According to the type definition inapps/meteor/client/views/room/contexts/RoomToolboxContext.ts,iconis a requiredIconName.Consider either:
- Using a valid placeholder icon (e.g.,
'apps') for test data- Adding a comment explaining why
undefinedis acceptable in this test contextApply this diff if using a placeholder icon:
{ id: 'app1', title: 'app-42212581-0966-44aa-8366-b3e92aa00df4.action_button_label_files', groups: ['group', 'channel', 'live', 'team', 'direct', 'direct_multiple'], type: 'apps', - icon: undefined as unknown as RoomToolboxActionConfig['icon'], + icon: 'apps', }, { id: 'app2', title: 'app-42212581-0966-44aa-8366-b3e92aa00df4.action_button_label_files', groups: ['group', 'channel', 'live', 'team', 'direct', 'direct_multiple'], type: 'apps', - icon: undefined as unknown as RoomToolboxActionConfig['icon'], + icon: 'apps', },Also applies to: 58-58
apps/meteor/client/views/room/HeaderV2/RoomToolbox/RoomToolbox.tsx (1)
42-49: Avoid shadowing: rename map param to clarifyactionvs.actionUsing
actionas both the param and a field hurts readability. Rename the param.-const mapToToolboxItem = (action: RoomToolboxActionConfig) => { - return (action.renderToolboxItem ?? renderDefaultToolboxItem)?.({ - ...action, - action: action.action ?? (() => toolbox.openTab(action.id)), +const mapToToolboxItem = (config: RoomToolboxActionConfig) => { + return (config.renderToolboxItem ?? renderDefaultToolboxItem)?.({ + ...config, + action: config.action ?? (() => toolbox.openTab(config.id)), className, toolbox, }); }apps/meteor/tests/e2e/team-management.spec.ts (1)
159-159: Team toolbar usage is consistent; ensure robust waits in POMThe move to roomToolbar.openTeamInfo/openTeamChannels is clear and consistent.
- Ensure these POM methods wait for the corresponding flex tab to be visible/idle before returning.
- Consider a small helper (e.g., openTeamChannelsFor(room)) to reduce repetition in tests. As per coding guidelines.
Also applies to: 175-175, 193-193, 211-211, 239-239, 253-253, 281-281, 291-291, 316-316, 335-335, 343-343, 355-355, 365-365, 390-390, 409-409, 417-417, 424-424, 447-447, 463-463, 479-479
apps/meteor/client/views/room/Header/Omnichannel/QuickActions/QuickActionOptions.tsx (1)
3-3: Icon prop passthrough is correct; improve prop typing and ref typesForwarded props aren’t reflected in the component’s public type, and refs are untyped.
-import type { Keys as IconName } from '@rocket.chat/icons'; +import type { Keys as IconName } from '@rocket.chat/icons'; +import type { ComponentProps } from 'react';-type QuickActionOptionsProps = { - options: QuickActionsActionOptions; - action: (id: string) => void; - room: IOmnichannelRoom; - icon: IconName; -}; +type QuickActionOptionsProps = { + options: QuickActionsActionOptions; + action: (id: string) => void; + room: IOmnichannelRoom; + icon: IconName; +} & Omit<ComponentProps<typeof HeaderToolbarAction>, 'icon' | 'onClick' | 'ref'>;-const QuickActionOptions = ({ options, room, action, icon, ...props }: QuickActionOptionsProps) => { +const QuickActionOptions = ({ options, room, action, icon, ...props }: QuickActionOptionsProps) => { - const reference = useRef(null); - const target = useRef(null); + const reference = useRef<HTMLButtonElement | null>(null); + const target = useRef<HTMLDivElement | null>(null);Ensure all call sites now provide icon, and no external props (e.g., title/aria-label) were unintentionally dropped from the public type.
Also applies to: 15-16, 18-18, 31-31
📜 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 (2)
packages/ui-client/src/components/Header/__snapshots__/Header.spec.tsx.snapis excluded by!**/*.snappackages/ui-client/src/components/HeaderV2/__snapshots__/Header.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (70)
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx(1 hunks)apps/meteor/client/components/Omnichannel/modals/CloseChatModal.tsx(1 hunks)apps/meteor/client/hooks/roomActions/useAppsRoomStarActions.tsx(1 hunks)apps/meteor/client/hooks/roomActions/useThreadRoomAction.tsx(1 hunks)apps/meteor/client/hooks/roomActions/useVideoCallRoomAction.tsx(1 hunks)apps/meteor/client/views/room/Header/Omnichannel/QuickActions/QuickActionOptions.tsx(3 hunks)apps/meteor/client/views/room/Header/Omnichannel/QuickActions/QuickActions.tsx(1 hunks)apps/meteor/client/views/room/Header/RoomToolbox/RoomToolbox.tsx(2 hunks)apps/meteor/client/views/room/Header/RoomToolbox/RoomToolboxE2EESetup.tsx(1 hunks)apps/meteor/client/views/room/Header/RoomToolbox/hooks/useRoomToolboxActions.spec.ts(1 hunks)apps/meteor/client/views/room/Header/RoomToolbox/hooks/useRoomToolboxActions.ts(1 hunks)apps/meteor/client/views/room/HeaderV2/Omnichannel/QuickActions/QuickActionOptions.tsx(3 hunks)apps/meteor/client/views/room/HeaderV2/Omnichannel/QuickActions/QuickActions.tsx(1 hunks)apps/meteor/client/views/room/HeaderV2/RoomToolbox/RoomToolbox.tsx(2 hunks)apps/meteor/client/views/room/HeaderV2/RoomToolbox/RoomToolboxE2EESetup.tsx(1 hunks)apps/meteor/client/views/room/HeaderV2/RoomToolbox/hooks/useRoomToolboxActions.spec.ts(1 hunks)apps/meteor/client/views/room/HeaderV2/RoomToolbox/hooks/useRoomToolboxActions.ts(1 hunks)apps/meteor/client/views/room/contexts/RoomToolboxContext.ts(2 hunks)apps/meteor/client/views/room/lib/quickActions.tsx(1 hunks)apps/meteor/client/views/room/providers/hooks/useAppsRoomActions.ts(1 hunks)apps/meteor/tests/e2e/channel-management.spec.ts(10 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(3 hunks)apps/meteor/tests/e2e/feature-preview.spec.ts(4 hunks)apps/meteor/tests/e2e/federation/page-objects/channel.ts(2 hunks)apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.ts(0 hunks)apps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab.ts(0 hunks)apps/meteor/tests/e2e/federation/tests/ce-version/ce.spec.ts(1 hunks)apps/meteor/tests/e2e/federation/tests/channel/dm.spec.ts(19 hunks)apps/meteor/tests/e2e/federation/tests/channel/private.spec.ts(28 hunks)apps/meteor/tests/e2e/federation/tests/channel/public.spec.ts(31 hunks)apps/meteor/tests/e2e/federation/tests/messaging/dm.spec.ts(1 hunks)apps/meteor/tests/e2e/federation/tests/messaging/private.spec.ts(1 hunks)apps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts(1 hunks)apps/meteor/tests/e2e/federation/tests/messaging/threads.spec.ts(1 hunks)apps/meteor/tests/e2e/federation/tests/user-account/user.spec.ts(2 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-auto-onhold-chat-closing.spec.ts(4 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-canned-responses-sidebar.spec.ts(3 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-chat-history.spec.ts(1 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts(6 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-close-chat.spec.ts(2 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-close-inquiry.spec.ts(1 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-contact-conflict-review.spec.ts(1 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-contact-info.spec.ts(3 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.ts(3 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-avatar-visibility.spec.ts(1 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-message-bubble-color.spec.ts(1 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-livechat.spec.ts(2 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.ts(2 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.ts(2 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts(1 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-transfer-to-another-agents.spec.ts(2 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-content.ts(0 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts(0 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-omnichannel-content.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/omnichannel-close-chat-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts(1 hunks)apps/meteor/tests/e2e/page-objects/home-channel.ts(3 hunks)apps/meteor/tests/e2e/page-objects/home-omnichannel.ts(3 hunks)apps/meteor/tests/e2e/page-objects/home-team.ts(2 hunks)apps/meteor/tests/e2e/page-objects/modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/omnichannel-transcript.ts(0 hunks)apps/meteor/tests/e2e/retention-policy.spec.ts(9 hunks)apps/meteor/tests/e2e/team-management.spec.ts(17 hunks)packages/ui-client/src/components/Header/Header.spec.tsx(1 hunks)packages/ui-client/src/components/Header/Header.stories.tsx(5 hunks)packages/ui-client/src/components/Header/HeaderToolbar/HeaderToolbarAction.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/Header.spec.tsx(1 hunks)packages/ui-client/src/components/HeaderV2/Header.stories.tsx(6 hunks)packages/ui-client/src/components/HeaderV2/HeaderToolbar/HeaderToolbarAction.tsx(1 hunks)
💤 Files with no reviewable changes (5)
- apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
- apps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
- apps/meteor/tests/e2e/page-objects/omnichannel-transcript.ts
🧰 Additional context used
📓 Path-based instructions (4)
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/retention-policy.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-conflict-review.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-close-chat.spec.tsapps/meteor/tests/e2e/feature-preview.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-transfer-to-another-agents.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/federation/tests/channel/private.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-message-bubble-color.spec.tsapps/meteor/tests/e2e/federation/tests/ce-version/ce.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-history.spec.tsapps/meteor/tests/e2e/federation/tests/channel/dm.spec.tsapps/meteor/tests/e2e/federation/tests/user-account/user.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-info.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-avatar-visibility.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/threads.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-canned-responses-sidebar.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-auto-onhold-chat-closing.spec.tsapps/meteor/tests/e2e/federation/tests/channel/public.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-close-inquiry.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/retention-policy.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-conflict-review.spec.tsapps/meteor/tests/e2e/page-objects/home-team.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-close-chat.spec.tsapps/meteor/tests/e2e/feature-preview.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-transfer-to-another-agents.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/federation/tests/channel/private.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-message-bubble-color.spec.tsapps/meteor/tests/e2e/federation/tests/ce-version/ce.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/tests/e2e/page-objects/home-omnichannel.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-history.spec.tsapps/meteor/tests/e2e/federation/tests/channel/dm.spec.tsapps/meteor/tests/e2e/federation/tests/user-account/user.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-info.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-avatar-visibility.spec.tsapps/meteor/tests/e2e/page-objects/fragments/omnichannel-close-chat-modal.tsapps/meteor/tests/e2e/page-objects/modal.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/threads.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-canned-responses-sidebar.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-auto-onhold-chat-closing.spec.tsapps/meteor/tests/e2e/federation/tests/channel/public.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.tsapps/meteor/tests/e2e/page-objects/fragments/home-omnichannel-content.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/federation/page-objects/channel.tsapps/meteor/tests/e2e/omnichannel/omnichannel-close-inquiry.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/retention-policy.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-conflict-review.spec.tsapps/meteor/tests/e2e/page-objects/home-team.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-close-chat.spec.tsapps/meteor/tests/e2e/feature-preview.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-transfer-to-another-agents.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/federation/tests/channel/private.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-message-bubble-color.spec.tsapps/meteor/tests/e2e/federation/tests/ce-version/ce.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/tests/e2e/page-objects/home-omnichannel.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-history.spec.tsapps/meteor/tests/e2e/federation/tests/channel/dm.spec.tsapps/meteor/tests/e2e/federation/tests/user-account/user.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-info.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-avatar-visibility.spec.tsapps/meteor/tests/e2e/page-objects/fragments/omnichannel-close-chat-modal.tsapps/meteor/tests/e2e/page-objects/modal.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/threads.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-canned-responses-sidebar.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-auto-onhold-chat-closing.spec.tsapps/meteor/tests/e2e/federation/tests/channel/public.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.tsapps/meteor/tests/e2e/page-objects/fragments/home-omnichannel-content.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/federation/page-objects/channel.tsapps/meteor/tests/e2e/omnichannel/omnichannel-close-inquiry.spec.ts
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/home-team.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/home-omnichannel.tsapps/meteor/tests/e2e/page-objects/fragments/omnichannel-close-chat-modal.tsapps/meteor/tests/e2e/page-objects/modal.tsapps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.tsapps/meteor/tests/e2e/page-objects/fragments/home-omnichannel-content.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
🧠 Learnings (3)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#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/home-team.tsapps/meteor/tests/e2e/page-objects/home-omnichannel.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#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/channel-management.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/tests/e2e/team-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#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/fragments/toolbar.ts
🧬 Code graph analysis (20)
apps/meteor/tests/e2e/page-objects/home-team.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (1)
RoomToolbar(14-106)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (1)
RoomToolbar(14-106)
packages/ui-client/src/components/HeaderV2/Header.spec.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/client/views/room/HeaderV2/RoomToolbox/RoomToolbox.tsx (1)
apps/meteor/client/views/room/contexts/RoomToolboxContext.ts (2)
RenderToolboxItemParams(7-10)RoomToolboxActionConfig(12-31)
apps/meteor/client/views/room/Header/RoomToolbox/hooks/useRoomToolboxActions.spec.ts (1)
apps/meteor/client/views/room/contexts/RoomToolboxContext.ts (1)
RoomToolboxActionConfig(12-31)
apps/meteor/tests/e2e/page-objects/home-omnichannel.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (2)
OmnichannelRoomToolbar(108-124)OmnichannelQuickActionsRoomToolbar(126-165)
apps/meteor/tests/e2e/omnichannel/omnichannel-contact-info.spec.ts (2)
apps/meteor/tests/e2e/page-objects/home-omnichannel.ts (1)
HomeOmnichannel(15-74)apps/meteor/tests/e2e/page-objects/omnichannel-contacts-list.ts (1)
OmnichannelContacts(6-159)
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx (1)
apps/meteor/tests/e2e/page-objects/modal.ts (1)
Modal(6-28)
apps/meteor/tests/e2e/page-objects/fragments/omnichannel-close-chat-modal.ts (2)
apps/meteor/tests/e2e/page-objects/modal.ts (1)
Modal(6-28)apps/meteor/tests/e2e/page-objects/fragments/toast-messages.ts (1)
ToastMessages(3-14)
packages/ui-client/src/components/Header/Header.spec.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/tests/e2e/omnichannel/omnichannel-auto-onhold-chat-closing.spec.ts (1)
apps/meteor/tests/e2e/page-objects/home-omnichannel.ts (1)
HomeOmnichannel(15-74)
apps/meteor/client/components/Omnichannel/modals/CloseChatModal.tsx (1)
apps/meteor/tests/e2e/page-objects/modal.ts (1)
Modal(6-28)
apps/meteor/client/views/room/HeaderV2/RoomToolbox/hooks/useRoomToolboxActions.spec.ts (1)
apps/meteor/client/views/room/contexts/RoomToolboxContext.ts (1)
RoomToolboxActionConfig(12-31)
apps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.ts (1)
apps/meteor/tests/e2e/page-objects/modal.ts (1)
Modal(6-28)
apps/meteor/client/views/room/providers/hooks/useAppsRoomActions.ts (1)
apps/meteor/client/views/room/contexts/RoomToolboxContext.ts (1)
RoomToolboxActionConfig(12-31)
apps/meteor/client/views/room/Header/RoomToolbox/RoomToolbox.tsx (1)
apps/meteor/client/views/room/contexts/RoomToolboxContext.ts (2)
RenderToolboxItemParams(7-10)RoomToolboxActionConfig(12-31)
packages/ui-client/src/components/Header/Header.stories.tsx (2)
packages/livechat/src/components/Header/index.tsx (1)
Header(25-42)packages/ui-client/src/components/HeaderV2/Header.stories.tsx (1)
WithBurger(115-138)
packages/ui-client/src/components/HeaderV2/Header.stories.tsx (1)
packages/livechat/src/components/Header/index.tsx (1)
Header(25-42)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (2)
apps/meteor/tests/e2e/page-objects/fragments/omnichannel-close-chat-modal.ts (1)
OmnichannelCloseChatModal(6-35)apps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.ts (1)
OmnichannelOnHoldModal(5-18)
apps/meteor/tests/e2e/federation/page-objects/channel.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (1)
RoomToolbar(14-106)
🔇 Additional comments (65)
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx (1)
24-25: LGTM! Accessibility improvement aligns with e2e test expectations.The addition of
aria-labelto the Modal provides an accessible name for the dialog role, which is required by the e2e test page object that usesgetByRole('dialog', { name }). The TODO comment appropriately documents the planned migration to GenericModal for future refactoring.apps/meteor/tests/e2e/federation/tests/messaging/threads.spec.ts (1)
356-356: Approve selector update Confirmed thatroomToolbar.btnThreadsis defined in the page object and the change aligns with the refactoring.apps/meteor/tests/e2e/feature-preview.spec.ts (1)
348-349: Good move to RoomToolbar API; ensure built-in waits for stabilityThe new
roomToolbar.openTeamChannels()andopenRoomInfo()improve test readability and DRY. Ensure these methods include web-first waits (e.g., expect target panel/tab to be visible) to avoid flakiness.Also applies to: 384-386, 406-407, 511-515
apps/meteor/tests/e2e/page-objects/modal.ts (1)
3-5: Good practice to document the planned consolidation.The TODO comment clearly identifies a potential refactor opportunity to merge duplicate Modal implementations.
apps/meteor/tests/e2e/page-objects/home-team.ts (2)
4-4: LGTM!The RoomToolbar integration is clean and consistent with the broader refactor to toolbar-based interactions across test page objects.
Also applies to: 18-18, 25-25
6-8: Good to document the redundancy concern.The TODO appropriately flags that HomeTeam may be redundant, which aligns with the goal of standardizing room interactions.
apps/meteor/client/views/room/Header/RoomToolbox/hooks/useRoomToolboxActions.spec.ts (1)
51-51: LGTM!The addition of the
iconproperty aligns the test data with the updatedRoomToolboxActionConfigtype signature. The type cast appropriately satisfies the type requirement for test mocks.Also applies to: 58-58
apps/meteor/client/views/room/Header/RoomToolbox/hooks/useRoomToolboxActions.ts (2)
24-30: LGTM on object property syntax cleanup.The change from quoted property names to bare identifiers is a clean refactor. The onClick logic correctly preserves the existing behavior.
23-31: Restore data-qa-id attributes used by e2e tests
The removal ofdata-qa-idin useRoomToolboxActions.ts (lines 23–31) breaks selectors in apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.ts (menu-more-actions,reaction-message). Re-add these attributes or update the tests to use current selectors.⛔ Skipped due to learnings
Learnt from: dougfabris PR: RocketChat/Rocket.Chat#36987 File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20 Timestamp: 2025-09-23T19:22:59.217Z Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.apps/meteor/client/views/room/HeaderV2/RoomToolbox/RoomToolboxE2EESetup.tsx (1)
29-29: LGTM!The prop refactoring from
actiontoonClickimproves clarity, and removing the unusedindexparameter reduces noise. The behavior is preserved with the fallback totoolbox.openTab(id).Also applies to: 37-37
apps/meteor/tests/e2e/omnichannel/omnichannel-auto-onhold-chat-closing.spec.ts (2)
7-7: LGTM!The migration from
HomeChanneltoHomeOmnichannelis semantically correct for omnichannel-specific tests and improves test clarity.Also applies to: 16-16, 27-27
59-59: LGTM!Using
quickActionsRoomToolbar.placeChatOnHold()instead of direct button clicks improves maintainability by abstracting the interaction through the toolbar API.apps/meteor/tests/e2e/omnichannel/omnichannel-transfer-to-another-agents.spec.ts (1)
75-75: LGTM!The migration to
quickActionsRoomToolbar.forwardChat()improves maintainability by using the toolbar abstraction instead of direct DOM element interactions. Both occurrences are updated consistently.Also applies to: 87-87
apps/meteor/client/hooks/roomActions/useAppsRoomStarActions.tsx (1)
42-42: LGTM!Passing the
iconprop explicitly toHeaderToolbarActionaligns with the broader refactor to icon-driven action rendering and improves clarity.apps/meteor/client/views/room/HeaderV2/RoomToolbox/hooks/useRoomToolboxActions.ts (1)
24-25: LGTM!The shift to unquoted object keys is cleaner and more idiomatic. The removal of
data-qa-id(mentioned in the summary) aligns with the broader refactoring toward explicit action handlers and modern test patterns.apps/meteor/client/hooks/roomActions/useVideoCallRoomAction.tsx (1)
54-54: LGTM!Using
undefinedinstead of an empty string for the disabled tooltip is semantically clearer and prevents ambiguity for consumers and assistive technologies.apps/meteor/tests/e2e/omnichannel/omnichannel-contact-conflict-review.spec.ts (1)
80-80: LGTM!The migration to
roomToolbar.openContactInfo()follows the new page object abstraction pattern and improves test maintainability by using semantic methods instead of direct element clicks.apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-avatar-visibility.spec.ts (1)
92-92: LGTM!Consolidating the chat closure flow into a single
closeChat({ comment })call reduces test brittleness and improves readability by abstracting modal interaction details into the page object.apps/meteor/client/components/Omnichannel/modals/CloseChatModal.tsx (1)
155-159: Good accessibility improvement.Adding
aria-label={t('Wrap_up_conversation')}improves screen reader support for this dialog. ThewrapperFunctionmaintains the existing form submission behavior while preparing for the future migration toGenericModal(as noted in the TODO).apps/meteor/tests/e2e/federation/tests/user-account/user.spec.ts (1)
100-100: LGTM!The migration to
roomToolbar.openMembersTab()aligns with the new page object abstraction pattern and improves test maintainability by using semantic methods instead of direct tab clicks.Also applies to: 124-124
apps/meteor/client/views/room/lib/quickActions.tsx (1)
14-14: Verify all QuickActionsActionConfig instances include the now-required icon. Confirm that every consumer—especially the return objects inapps/meteor/client/hooks/quickActions/*QuickAction.ts*and the array inapps/meteor/client/ui.ts—supplies aniconproperty.apps/meteor/tests/e2e/retention-policy.spec.ts (1)
53-53: LGTM!The migration from
tabs.btnRoomInfo.click()toroomToolbar.openRoomInfo()(and similar for team info) centralizes navigation through the toolbar API, improving maintainability and consistency across tests. The changes follow the Page Object Model pattern effectively.Also applies to: 88-88, 100-100, 130-130, 140-140, 150-150, 172-172, 189-189, 206-206
apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts (1)
141-141: LGTM!The consolidation of the forward chat action into
quickActionsRoomToolbar.forwardChat()is a clean abstraction that improves test maintainability. The change is applied consistently across all test scenarios for both Monitor and Manager roles.Also applies to: 184-184, 229-229, 366-366, 409-409, 454-454
apps/meteor/client/views/room/HeaderV2/Omnichannel/QuickActions/QuickActions.tsx (1)
35-35: LGTM!The shift from passing
actionin the shared props object to explicit prop forwarding (Line 35) and usingonClickhandlers (Line 38) is a cleaner API design. This pattern:
- Provides clearer intent for button interactions
- Maintains the same behavior while improving code readability
- Aligns with standard React onClick patterns
Also applies to: 38-38
apps/meteor/tests/e2e/federation/tests/ce-version/ce.spec.ts (1)
79-79: LGTM!The migration to
roomToolbar.openMembersTab()is consistent with the broader refactoring and improves test maintainability through better abstraction.apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts (1)
58-58: LGTM!The consolidation of the multi-step close chat flow into
closeChat({ downloadPDF: true })significantly simplifies the test code. This abstraction improves maintainability by encapsulating the interaction details within the toolbar method.Note: The explicit toast assertion has been removed. Ensure the
closeChatmethod handles success verification internally if needed.apps/meteor/client/views/room/Header/RoomToolbox/RoomToolboxE2EESetup.tsx (1)
35-35: LGTM!The changes align with the updated
HeaderToolbarActionAPI:
- Removing the
indexparameter from the map callback (Line 35) simplifies the code- Migrating from
actiontoonClick(Line 43) follows standard React patterns for button interactions- The fallback behavior
() => toolbox.openTab(id)is preserved whenactionis undefinedAlso applies to: 43-43
apps/meteor/tests/e2e/omnichannel/omnichannel-chat-history.spec.ts (1)
60-60: LGTM!The consolidation of the close chat flow into
quickActionsRoomToolbar.closeChat()improves test maintainability. The removal of the explicit toast assertion suggests that success verification is either handled internally by the method or is not critical for this test scenario.apps/meteor/tests/e2e/federation/page-objects/channel.ts (1)
6-6: LGTM!The addition of the
roomToolbarproperty toFederationChannelcorrectly implements the toolbar API abstraction. The implementation:
- Imports
RoomToolbarfrom the appropriate fragments module- Declares it as a readonly property for immutability
- Initializes it properly in the constructor
This enables federation tests to use the standardized toolbar navigation pattern.
Also applies to: 17-17, 24-24
apps/meteor/tests/e2e/omnichannel/omnichannel-close-inquiry.spec.ts (1)
57-57: LGTM!The refactor to use
quickActionsRoomToolbar.closeChat()simplifies the test flow by consolidating multiple UI interactions into a single method call. This improves maintainability and aligns with the broader architectural shift to toolbar-based abstractions.apps/meteor/tests/e2e/omnichannel/omnichannel-close-chat.spec.ts (2)
7-7: LGTM!Removing the unused
expectimport is appropriate since the test no longer includes explicit toast assertions.
49-49: LGTM!The refactor consolidates the close-chat flow into a single method call, improving test maintainability. The previous toast verification has been removed, which is acceptable if the toolbar method implementation handles this internally or if toast behavior is tested elsewhere.
apps/meteor/client/views/room/Header/Omnichannel/QuickActions/QuickActions.tsx (1)
22-39: LGTM!The refactor clarifies action invocation by:
- Explicitly passing
action={action}toQuickActionOptionswhen options are present- Using an
onClickcallback that invokesaction(id)for the non-options path withHeaderToolbarActionThis separation of concerns aligns with the updated component API and improves code clarity.
packages/ui-client/src/components/HeaderV2/Header.spec.tsx (1)
1-20: LGTM!The test file follows excellent practices:
- Uses parameterized tests with
test.eachto test all story variants- Combines snapshot testing with accessibility validation using jest-axe
- Properly wraps components with
mockAppRoot()for context- Ensures comprehensive coverage of HeaderV2 component behavior and accessibility
packages/ui-client/src/components/Header/Header.stories.tsx (1)
2-2: LGTM!The systematic addition of
titleprops toHeaderStateandHeaderToolbarActioncomponents, along with theIconButtonreplacement for the burger menu, improves accessibility by providing clear, descriptive labels for all interactive elements. This aligns with the new title-driven labeling approach in the component API.Also applies to: 98-98, 107-109, 117-117, 124-124, 133-135, 147-147, 154-163
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
4-4: LGTM!The addition of
roomToolbarcleanly extends theHomeChannelpage object's public API, following the established pattern for other members. This enables toolbar-based interactions in tests and maintains consistency with the broader architectural refactor.Also applies to: 21-21, 31-31
apps/meteor/tests/e2e/channel-management.spec.ts (1)
50-50: LGTM!The systematic migration from direct UI interactions (e.g.,
btnTabMembers.click(),btnRoomInfo.click()) to toolbar-based methods (e.g.,roomToolbar.openMembersTab(),roomToolbar.openRoomInfo()) is consistently applied across all test cases. This improves test maintainability by centralizing UI interaction logic in the page object, while preserving the original test goals and assertions.Also applies to: 60-60, 66-66, 74-74, 80-80, 87-87, 93-93, 100-100, 115-115, 189-189, 203-203, 217-217, 226-226, 232-232, 242-242, 253-253, 292-292
apps/meteor/tests/e2e/omnichannel/omnichannel-livechat.spec.ts (1)
105-105: LGTM!The refactor consolidates the close-chat flow into a single, expressive method call with a
commentparameter:quickActionsRoomToolbar.closeChat({ comment: 'this_is_a_test_comment' }). This improves test readability and maintainability by encapsulating the multi-step modal interaction within the page object, while preserving the test's original intent.Also applies to: 158-158
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (1)
196-199: LGTM! Consistent migration to roomToolbar API.The changes successfully migrate from the tabs-based API to the new roomToolbar abstraction while preserving all test assertions and control flow. The use of
waitFor()before assertions is appropriate for ensuring elements are ready.Also applies to: 241-244, 311-314
apps/meteor/tests/e2e/page-objects/home-omnichannel.ts (1)
4-4: LGTM! Clean addition of toolbar page objects.The new
roomToolbarandquickActionsRoomToolbarproperties are properly typed, initialized, and follow the existing patterns in the page object class. The imports fromfragments/toolbaralign with the broader toolbar abstraction introduced in this PR.Also applies to: 44-46, 63-64
packages/ui-client/src/components/Header/Header.spec.tsx (1)
1-20: LGTM! Well-structured component tests.The test suite properly covers:
- Rendering validation via snapshots
- Accessibility compliance via jest-axe
The use of
composeStoriesand parameterized tests withtest.eachfollows best practices for testing Storybook stories. ThemockAppRootwrapper ensures proper test isolation.apps/meteor/tests/e2e/federation/tests/channel/dm.spec.ts (1)
87-91: LGTM! Comprehensive migration to roomToolbar API.The changes consistently migrate from the legacy tabs-based API to the new roomToolbar abstraction across all DM test scenarios. The migration:
- Replaces direct button references with toolbar methods (e.g.,
openUserInfo(),openMembersTab())- Updates assertions to use toolbar button properties
- Maintains all test logic and expectations
This aligns with the Page Object Model pattern specified in the coding guidelines.
Also applies to: 127-131, 181-186, 220-221, 268-273, 305-306, 354-358, 410-414, 458-459, 473-474, 524-528, 581-586, 619-620, 671-676, 709-710, 772-777, 802-805, 835-838, 866-869
apps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.ts (1)
138-144: LGTM! Excellent abstraction of complex UI interactions.The migration to
quickActionsRoomToolbarmethods provides cleaner, more maintainable test code:
placeChatOnHold()abstracts the multi-step on-hold flowcloseChat()encapsulates the comment/PDF confirmation dialog- Direct toolbar button references improve test readability
This reduces test brittleness and follows the DRY principle specified in the coding guidelines.
Also applies to: 150-151, 154-156
apps/meteor/client/views/room/Header/RoomToolbox/RoomToolbox.tsx (2)
26-38: LGTM! Cleaner prop handling for HeaderToolbarAction.The refactoring successfully:
- Removes unused
indexparameter from the render flow- Replaces
actionprop with standardonClickhandler- Maintains
idfor internal use (key and pressed state)This aligns with standard React patterns and improves code clarity.
56-56: No E2E usage ofdata-qa-idon GenericMenu. Searches fordata-qa-idandGenericMenuinapps/meteor/tests/e2eyielded no matches—removal won’t break tests.apps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts (1)
1015-1023: LGTM! Consistent toolbar API migration.The changes successfully migrate visual element tests from the tabs API to the roomToolbar abstraction:
- File list button accessed via
roomToolbar.btnFiles- Kebab menu replaced with semantic
openMoreOptions()method- Menu items referenced through toolbar properties (menuItemMentions, menuItemStarredMessages, menuItemPinnedMessages, menuItemPruneMessages)
This improves test maintainability and aligns with the coding guidelines' preference for semantic locators.
Also applies to: 1030-1032, 1039-1041, 1048-1050, 1057-1059, 1066-1068, 1075-1077, 1084-1086, 1093-1095
apps/meteor/client/views/room/HeaderV2/Omnichannel/QuickActions/QuickActionOptions.tsx (1)
3-3: LGTM! Type-safe icon prop addition.The addition of the
iconprop toQuickActionOptionsis well-implemented:
- Uses the type-safe
IconNametype from@rocket.chat/icons- Properly forwards the icon to
HeaderToolbarAction- Maintains existing functionality while extending the component's capabilities
Note: This is a breaking change to the public API as
iconis now a required prop.Also applies to: 15-16, 18-18, 31-31
apps/meteor/client/views/room/HeaderV2/RoomToolbox/RoomToolbox.tsx (2)
26-37: Handler prop migration to onClick looks correctCallback wiring and pressed state are preserved. If tooltip is a translation key, consider wrapping with t() for consistency.
56-57: Menu title OK; ensure tests don’t rely on removed QA attrsSwitching to title={t('Options')} is fine. Confirm e2e/page objects no longer depend on prior data attributes.
apps/meteor/tests/e2e/federation/tests/messaging/dm.spec.ts (1)
1300-1301: Migration to RoomToolbar API in DMs looks goodSelectors and actions map cleanly to toolbar methods and menu items.
Please confirm the RoomToolbar page-object uses semantic locators (getByRole/getByLabel) and includes web-first waits to avoid flakiness. As per coding guidelines.
Also applies to: 1308-1309, 1315-1318, 1324-1327, 1333-1336, 1342-1345, 1351-1354, 1360-1363
apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.ts (1)
498-501: Adoption of Room/Quick Actions toolbars in OC tests is soundopenContactInfo and closeChat({ comment }) improve clarity and reduce steps.
Confirm the POM waits for the contact info panel/chat close completion (e.g., success toast or status) to avoid race conditions. As per coding guidelines.
Also applies to: 631-636, 804-806
packages/ui-client/src/components/HeaderV2/Header.stories.tsx (2)
2-2: LGTM! IconButton import supports the WithBurger story.The added IconButton import is used in the WithBurger story (line 118) to render a burger menu button, which is a valid use case for this icon-based button component.
102-110: LGTM! Consistent API updates across HeaderState and HeaderToolbarAction.The changes standardize the component API by:
- Adding a
titleprop to HeaderState for accessibility- Replacing the action callback in HeaderToolbarAction with explicit
titlepropsThis improves accessibility and makes the component usage more explicit and consistent.
apps/meteor/client/hooks/roomActions/useThreadRoomAction.tsx (1)
48-56: LGTM! renderToolboxItem signature updated to align with HeaderToolbarAction refactor.The changes correctly adapt to the new HeaderToolbarAction API:
- Removed
indexparameter (no longer needed)- Changed
action={action}toonClick={action}(standard React event handler prop)This aligns with the refactored HeaderToolbarAction component that now accepts standard IconButton props.
apps/meteor/tests/e2e/omnichannel/omnichannel-canned-responses-sidebar.spec.ts (2)
37-37: LGTM! Migration to toolbar-based API improves test maintainability.The change from
content.closeChat()toquickActionsRoomToolbar.closeChat()follows the Page Object Model pattern and provides better abstraction of UI interactions.
63-63: LGTM! Consistent use of roomToolbar API.The migration from
content.btnCannedResponses.click()toroomToolbar.openCannedResponses()provides a cleaner, more semantic API for test interactions.Also applies to: 82-82
packages/ui-client/src/components/HeaderV2/HeaderToolbar/HeaderToolbarAction.tsx (1)
4-13: LGTM! Simplified and strongly typed component improves maintainability.The refactor provides several benefits:
- Explicit
HeaderToolbarActionPropstype with proper typing- Clean delegation to
IconButtoncomponent- Smart
tooltipfallback totitlefor better accessibility- Removed inline logic in favor of standard React patterns
The previous functionality (data attributes, custom onClick handling) has been removed in favor of letting consumers pass these through
...propsdirectly, which is cleaner and more flexible.apps/meteor/tests/e2e/federation/tests/messaging/private.spec.ts (2)
1021-1021: LGTM! Migration to roomToolbar API follows Page Object Model pattern.The changes from
tabs.btnFileListtoroomToolbar.btnFilesprovide better semantic naming and follow the established pattern for room interactions.As per coding guidelines for e2e tests.
Also applies to: 1029-1029
1036-1101: LGTM! Consistent toolbar API usage across all visibility checks.All kebab menu interactions have been properly migrated to use
roomToolbar.openMoreOptions()followed byroomToolbar.menuItem*assertions. This provides a cleaner abstraction and better test maintainability.apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.ts (2)
190-196: LGTM! Consolidated on-hold interaction improves test readability.The change from manual UI steps to
quickActionsRoomToolbar.placeChatOnHold()encapsulates the complex interaction pattern into a single, semantic method call, making the test more maintainable.
202-202: LGTM! Consistent use of quickActionsRoomToolbar API.The visibility check for
btnOnHoldand thecloseChat()call properly use the new toolbar API, maintaining consistency across the test file.Also applies to: 208-210
apps/meteor/tests/e2e/page-objects/fragments/omnichannel-on-hold-modal.ts (2)
10-17: LGTM! Clean modal interaction methods.The
btnPlaceChatOnHoldlocator andconfirm()method properly encapsulate the modal interaction pattern, with appropriate waiting for modal dismissal.
6-8: Fix Modal constructor usage
Pass thePageinstance tosuper()(not aLocator) and usethis.getModalByName('Place chat On-Hold')to scope the dialog.⛔ Skipped due to learnings
Learnt from: CR PR: RocketChat/Rocket.Chat#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 consistentlyLearnt from: CR PR: RocketChat/Rocket.Chat#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/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-info.spec.ts (2)
6-6: LGTM! Proper rename from HomeChannel to HomeOmnichannel.The import, type annotation, and constructor call have been consistently updated to use the renamed
HomeOmnichannelclass, which better reflects the specialized nature of this page object for Omnichannel scenarios.Also applies to: 14-14, 24-24
50-50: LGTM! Migration to toolbar-based contact info access.The change from
content.btnContactInformation.click()toroomToolbar.openContactInfo()provides better semantic clarity and follows the Page Object Model pattern.
57586dd to
4c0fcd9
Compare
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
CORE-1500
Summary by CodeRabbit
Accessibility
New Features
Bug Fixes & Refactor
Tests