Open
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
apps/meteor/tests/e2e/**/*.{ts,spec.ts}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2026-02-24T19:36:55.089ZApplied to files:
📚 Learning: 2025-12-16T17:29:45.163ZApplied to files:
📚 Learning: 2026-03-20T13:51:23.302ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-09-23T19:22:59.217ZApplied to files:
📚 Learning: 2026-02-24T19:39:42.247ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
🔇 Additional comments (2)
WalkthroughReorganized e2e page-object flextabs into a new Changes
Sequence Diagram(s)sequenceDiagram
participant Test as E2E Test
participant Home as HomeChannel/HomeTeam
participant RoomToolbar as RoomToolbar
participant MenuMore as MenuMore
participant Dialog as UI Dialog
Test->>Home: open room/team UI
Home->>RoomToolbar: get toolbar
Test->>RoomToolbar: openMoreOptions()
RoomToolbar->>MenuMore: click btnMoreOptions
MenuMore->>Dialog: show menu
Test->>MenuMore: getMenuItem(itemName)
MenuMore->>Dialog: find menu item by label
Test->>Dialog: click menu item (e.g., Export / Enable E2EE / Notifications)
Dialog->>Dialog: perform action / open FlexTab dialog
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/10 00:04 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 10 Feb 2026 00:04:04 GMT |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38533 +/- ##
===========================================
- Coverage 70.57% 70.53% -0.04%
===========================================
Files 3270 3270
Lines 116766 116788 +22
Branches 21091 21128 +37
===========================================
- Hits 82409 82381 -28
- Misses 32304 32355 +51
+ Partials 2053 2052 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dougfabris
force-pushed
the
test/reorg-flextabs
branch
from
20966ee to
6eee4f8
Compare
dougfabris
added
the
stat: QA assured
dougfabris
force-pushed
the
test/reorg-flextabs
branch
from
e308937 to
6cf7948
Compare
… EditTeamFlexTab class
dougfabris
force-pushed
the
test/reorg-flextabs
branch
from
6cf7948 to
7cc04bb
Compare
dougfabris
removed
the
stat: QA assured
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
apps/meteor/client/views/teams/contextualBar/channels/AddExistingModal/AddExistingModal.tsx (1)
32-32: Consider removing or addressing the TODO comment.The TODO comment violates the coding guideline to avoid code comments in the implementation. Either address this by using
GenericModalnow, or track it in an issue and remove the comment.As per coding guidelines: "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/teams/contextualBar/channels/AddExistingModal/AddExistingModal.tsx` at line 32, Remove or resolve the TODO comment in AddExistingModal: either refactor the component to use GenericModal instead of Modal (update imports and replace Modal usage within the AddExistingModal component) or create a tracked issue and remove the inline TODO; if choosing the issue route, add the issue ID in a short comment and delete the TODO. Ensure the change references the AddExistingModal component and the Modal/GenericModal symbols so reviewers can verify the replacement or the created issue.apps/meteor/tests/e2e/page-objects/fragments/flextabs/flextab.ts (1)
6-8: Consider removing the JSDoc comment.While this documents a design decision, it could be seen as violating the guideline to avoid code comments. An alternative is to track this refactoring task in an issue.
As per coding guidelines: "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/flextabs/flextab.ts` around lines 6 - 8, Remove the JSDoc comment above the public property "root" in flextab.ts (the comment that explains why root is public) and replace that in-repo explanation with a tracked issue instead; simply delete the comment block so the code no longer contains the implementation-level justification, and create/link an issue to track the refactor of making root protected in the future.apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-remove-modal.ts (1)
5-8: Constructor can be simplified.The constructor only passes
rootto the parent class without additional logic, making it redundant in TypeScript.♻️ Proposed simplification
export class ConfirmRemoveModal extends Modal { - constructor(root: Locator) { - super(root); - } - get btnRemove() {Note: This requires the parent
Modalclass to have a public constructor that acceptsLocator.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-remove-modal.ts` around lines 5 - 8, The ConfirmRemoveModal constructor is redundant because it only forwards `root` to the parent; remove the constructor from the `ConfirmRemoveModal` class so it inherits `Modal`'s constructor directly. Ensure the parent `Modal` class has a public constructor signature that accepts a `Locator` (update `Modal` to export a public constructor if needed) so instances of `ConfirmRemoveModal` continue to be constructible with a `Locator`.apps/meteor/tests/e2e/page-objects/fragments/flextabs/admin-edit-room-flextab.ts (1)
10-47: Prefer semantic role/label locators in this flextab page object.Line 10-Line 47 currently rely on CSS/text-engine selectors (
locator('input[...]'),label >> text=...); migrating togetByRole/getByLabelwould make these locators more resilient to markup refactors.Based on learnings: "In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/flextabs/admin-edit-room-flextab.ts` around lines 10 - 47, The current getters (roomNameInput, privateLabel, privateInput, roomOwnerInput, archivedLabel, archivedInput, favoriteLabel, favoriteInput, defaultLabel, defaultInput) use CSS/text selectors; replace them with semantic Playwright locators like getByLabel/getByRole/getByText/getByTitle or ARIA-based selectors (e.g., use getByLabel for inputs by their visible label, getByRole('checkbox'/'textbox'/'switch') with name for inputs, and getByText for labels) to make selectors resilient—update each getter to use the corresponding semantic locator instead of locator('input[...]') or 'label >> text=...'; ensure you do not switch to data-qa attributes.apps/meteor/tests/e2e/page-objects/home-team.ts (1)
14-20: Consider caching tabs instances.The
tabsgetter creates newTeamInfoFlexTabandEditTeamFlexTabinstances on every access. While this follows the existing pattern in the codebase and works correctly, caching these instances (similar to other properties likeheaderToolbar) would be slightly more efficient.♻️ Optional: Cache tabs instances
export class HomeTeam extends HomeChannel { readonly headerToolbar: TeamToolbar; + private readonly _room: TeamInfoFlexTab; + private readonly _editRoom: EditTeamFlexTab; constructor(page: Page) { super(page); this.headerToolbar = new TeamToolbar(page); + this._room = new TeamInfoFlexTab(page); + this._editRoom = new EditTeamFlexTab(page); } override get tabs() { return { ...super.tabs, - room: new TeamInfoFlexTab(this.page), - editRoom: new EditTeamFlexTab(this.page), + room: this._room, + editRoom: this._editRoom, }; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/home-team.ts` around lines 14 - 20, The tabs getter currently instantiates new TeamInfoFlexTab and EditTeamFlexTab objects on every access (override get tabs), which is inefficient; change it to cache those instances on the class (similar to headerToolbar) by creating private properties (e.g., _tabs or _teamInfoTab/_editTeamTab) and return the cached objects from the tabs getter, instantiating them only if the cache is empty; ensure you still spread super.tabs when building the returned object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/tests/e2e/avatar-settings.ts`:
- Line 94: The test is calling the async DOM method getAttribute('src') without
awaiting and asserting its return directly; replace that with a Playwright
web-first assertion by awaiting expect on the image locator
(poHomeChannel.userCard.imgUserCard) and using toHaveAttribute('src', avatarUrl)
so the assertion is retried and stable; update the assertion from
expect(poHomeChannel.userCard.imgUserCard.getAttribute('src')).toBe(avatarUrl)
to an awaited web-first form (await
expect(poHomeChannel.userCard.imgUserCard).toHaveAttribute('src', avatarUrl)).
In `@apps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.ts`:
- Around line 123-125: The showAllUsers() helper is not idempotent because it
always queries the trigger by its current label ('Online') and fails once 'All'
is selected; update showAllUsers() to short-circuit when 'All' is already
selected or to target the filter control independent of its displayed value:
first inspect the listbox/current selection (e.g., read the selected option text
or an aria-selected attribute on the listbox) and return immediately if it
equals 'All', otherwise open the filter control (locate the toggle by a stable
selector instead of name, or locate the listbox container and click its toggle)
and then call listbox.selectOption('All'); ensure you update references to
root.getByRole('button', { name: 'Online' }) and listbox.selectOption('All')
accordingly so the helper works whether the current value is 'Online' or 'All'.
In
`@apps/meteor/tests/e2e/page-objects/fragments/flextabs/notification-preferences-flextab.ts`:
- Around line 15-20: The XPath locators in the methods are bypassing this.root
because they start with '//' (absolute); change them to relative XPath by using
'.//' so they stay scoped to the notification-preferences dialog—update the
locator in the method that returns
this.root.locator(`//div[`@id`="${device}Alert"]`) and in async
selectDropdownById(text: string) which uses
this.root.locator(`//div[`@id`="${text}"]`) to use './/div[`@id`="..."]' instead.
In `@apps/meteor/tests/e2e/page-objects/fragments/flextabs/threads-flextab.ts`:
- Around line 6-8: The constructor for ThreadsFlexTab currently binds to a
generic dialog via super(page.getByRole('dialog')), which can attach to the
wrong dialog; update the locator to scope by name by calling
page.getByRole('dialog', { name: 'Threads' }) (i.e., change the super(...) call
in the ThreadsFlexTab constructor) so it follows the established pattern used by
FilesFlexTab, SearchMessagesFlexTab, and UserInfoFlexTab and prevents flakiness
when multiple dialogs are present.
- Around line 14-16: Update the lastThreadMessage getter to avoid data-qa-type
selectors: within the existing lastThreadMessage accessor (which uses
listThreadMessages), replace the chain that locates '[data-qa-type="message"]'
with listThreadMessages.getByRole('listitem').last() and then access the message
body using .locator('.rcx-message-body') so it follows the semantic locator
pattern used in home-content.ts.
In `@apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-delete-modal.ts`:
- Around line 21-23: The dialog locator in the ConfirmDeleteRoomModal
constructor (super(page.getByRole('dialog', { name: 'Delete' }))) is performing
substring matching and can collide with other dialogs; update the locator to use
exact matching by adding exact: true to the options object so the constructor
targets only the dialog with the exact name "Delete".
In `@apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-remove-modal.ts`:
- Line 1: Change the Locator import in this fragment to use the project's common
Playwright package: replace the current import of Locator from 'playwright-core'
with an import from '@playwright/test' so the file's top-level import (the
Locator type used in this module) matches other modal page-object files.
In `@apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts`:
- Around line 127-138: The openTeamMembers method clicks menuItemTeamMembers
without opening the overflow menu first; update openTeamMembers in TeamToolbar
to open the menu (e.g., await this.menu.open()) before awaiting
this.menuItemTeamMembers.click() so the menu item is visible and clickable —
ensure you use the existing menu accessor and keep the method async/await flow
intact.
In `@apps/meteor/tests/e2e/prune-messages.spec.ts`:
- Around line 64-65: The test is asserting checkboxes via label element locators
(pruneMessages.labelFilesOnly and pruneMessages.labelDoNotPrunePinned), but
Playwright's .toBeChecked() only works on input elements; change those locators
to semantic label-to-input resolvers using getByLabel with the exact label text
(e.g., getByLabel('Only remove the attached files, keep messages') and
getByLabel('Do not prune pinned messages')) so the assertions target the actual
checkbox inputs and not the label elements, then update the assertions to use
those new locators.
---
Nitpick comments:
In
`@apps/meteor/client/views/teams/contextualBar/channels/AddExistingModal/AddExistingModal.tsx`:
- Line 32: Remove or resolve the TODO comment in AddExistingModal: either
refactor the component to use GenericModal instead of Modal (update imports and
replace Modal usage within the AddExistingModal component) or create a tracked
issue and remove the inline TODO; if choosing the issue route, add the issue ID
in a short comment and delete the TODO. Ensure the change references the
AddExistingModal component and the Modal/GenericModal symbols so reviewers can
verify the replacement or the created issue.
In
`@apps/meteor/tests/e2e/page-objects/fragments/flextabs/admin-edit-room-flextab.ts`:
- Around line 10-47: The current getters (roomNameInput, privateLabel,
privateInput, roomOwnerInput, archivedLabel, archivedInput, favoriteLabel,
favoriteInput, defaultLabel, defaultInput) use CSS/text selectors; replace them
with semantic Playwright locators like getByLabel/getByRole/getByText/getByTitle
or ARIA-based selectors (e.g., use getByLabel for inputs by their visible label,
getByRole('checkbox'/'textbox'/'switch') with name for inputs, and getByText for
labels) to make selectors resilient—update each getter to use the corresponding
semantic locator instead of locator('input[...]') or 'label >> text=...'; ensure
you do not switch to data-qa attributes.
In `@apps/meteor/tests/e2e/page-objects/fragments/flextabs/flextab.ts`:
- Around line 6-8: Remove the JSDoc comment above the public property "root" in
flextab.ts (the comment that explains why root is public) and replace that
in-repo explanation with a tracked issue instead; simply delete the comment
block so the code no longer contains the implementation-level justification, and
create/link an issue to track the refactor of making root protected in the
future.
In `@apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-remove-modal.ts`:
- Around line 5-8: The ConfirmRemoveModal constructor is redundant because it
only forwards `root` to the parent; remove the constructor from the
`ConfirmRemoveModal` class so it inherits `Modal`'s constructor directly. Ensure
the parent `Modal` class has a public constructor signature that accepts a
`Locator` (update `Modal` to export a public constructor if needed) so instances
of `ConfirmRemoveModal` continue to be constructible with a `Locator`.
In `@apps/meteor/tests/e2e/page-objects/home-team.ts`:
- Around line 14-20: The tabs getter currently instantiates new TeamInfoFlexTab
and EditTeamFlexTab objects on every access (override get tabs), which is
inefficient; change it to cache those instances on the class (similar to
headerToolbar) by creating private properties (e.g., _tabs or
_teamInfoTab/_editTeamTab) and return the cached objects from the tabs getter,
instantiating them only if the cache is empty; ensure you still spread
super.tabs when building the returned object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3bd3b19b-d644-496b-a04b-d182066ee96f
⛔ Files ignored due to path filters (1)
apps/meteor/client/components/UserInfo/__snapshots__/UserInfo.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (64)
apps/meteor/client/components/UserInfo/UserInfo.tsxapps/meteor/client/views/teams/contextualBar/channels/AddExistingModal/AddExistingModal.tsxapps/meteor/tests/e2e/account-profile.spec.tsapps/meteor/tests/e2e/avatar-settings.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/feature-preview.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/page-objects/admin-device-management.tsapps/meteor/tests/e2e/page-objects/admin-emojis.tsapps/meteor/tests/e2e/page-objects/admin-rooms.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/admin-edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/admin-flextab-emoji.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/channels-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/device-info-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/edit-contact-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/edit-user-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/export-messages-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/files-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/index.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/notification-preferences-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/prune-messages-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/room-info-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/search-messages-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/threads-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/flextabs/user-info-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-channels.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-room.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/modals/confirm-delete-modal.tsapps/meteor/tests/e2e/page-objects/fragments/modals/confirm-remove-modal.tsapps/meteor/tests/e2e/page-objects/fragments/modals/index.tsapps/meteor/tests/e2e/page-objects/fragments/room-info-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/user-card.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/page-objects/home-omnichannel.tsapps/meteor/tests/e2e/page-objects/home-team.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-agents.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-contact-center/omnichannel-contact-center-chats.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-contact-center/omnichannel-contact-center-contacts.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-custom-fields.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-priorities.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-tags.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-triggers.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-units.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/retention-policy.spec.tsapps/meteor/tests/e2e/team-management.spec.tspackages/ui-client/src/components/InfoPanel/InfoPanelField.tsx
💤 Files with no reviewable changes (8)
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab-channels.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab-room.ts
- apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.ts
- apps/meteor/tests/e2e/page-objects/fragments/room-info-flextab.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.ts
apps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.ts
Show resolved
Hide resolved
apps/meteor/tests/e2e/page-objects/fragments/flextabs/notification-preferences-flextab.ts
Show resolved
Hide resolved
apps/meteor/tests/e2e/page-objects/fragments/flextabs/threads-flextab.ts
Show resolved
Hide resolved
apps/meteor/tests/e2e/page-objects/fragments/flextabs/threads-flextab.ts
Show resolved
Hide resolved
apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-delete-modal.ts
Show resolved
Hide resolved
apps/meteor/tests/e2e/page-objects/fragments/modals/confirm-remove-modal.ts
Outdated
Show resolved
Hide resolved
aleksandernsilva
requested changes
Apr 6, 2026
Contributor
aleksandernsilva
left a comment
aleksandernsilva
left a comment
There was a problem hiding this comment.
Just a few nitpicks and questions.
| get btnCreate(): Locator { | ||
| return this.page.locator('role=dialog >> role=group >> role=button[name="Create"]'); | ||
| super(page); | ||
| this.tabs.room = new RoomInfoFlexTab(page.getByRole('dialog', { name: 'Discussion Info' }), page); |
Contributor
There was a problem hiding this comment.
Creating an override tabs getter seems like a better way of doing this. You are already following the pattern in the HomeTeam class.
| this.contacts = new OmnichannelContactCenterContacts(page); | ||
| this.chats = new OmnichannelContactCenterChats(page); | ||
| this.roomInfo = new OmnichannelRoomInfoFlexTab(page); | ||
| this.roomInfo = new OmnichannelRoomInfoFlexTab(page.getByRole('dialog', { name: 'Room Information' }), page); |
Contributor
There was a problem hiding this comment.
Could we movepage.getByRole('dialog', { name: 'Room Information' }) to the OmnichannelRoomInfoFlexTab class constructor, like I see in similar flextab classes?
| return this.menu.root.getByRole('menuitem', { name: 'Delete' }); | ||
| } | ||
|
|
||
| get userName(): Locator { |
Contributor
There was a problem hiding this comment.
Suggested change
| get userName(): Locator { | |
| get username(): Locator { |
| this.threadComposer = new ThreadComposer(page); | ||
| } | ||
|
|
||
| get tabs() { |
Contributor
There was a problem hiding this comment.
is the private _tabs + getter future-proofing? Wouldn't readonly tabs achieve the same goal?
|
|
||
| export class ThreadsFlexTab extends FlexTab { | ||
| constructor(page: Page) { | ||
| super(page.getByRole('dialog')); |
| this.confirmRemoveModal = new ConfirmRemoveModal(page.getByRole('dialog', { name: 'Are you sure?' })); | ||
| this.confirmDeleteModal = new ConfirmDeleteRoomModal(page); | ||
| this.addExistingChannelModal = new AddExistingChannelModal(page); | ||
| this.listbox = new Listbox(page); |
Contributor
There was a problem hiding this comment.
Does it make sense to restrict the listbox scope to the dialog at least? since the listbox doesn't have label for now
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
CORE-1989
Summary by CodeRabbit
Accessibility
Refactor