fix: replace raw Modal with GenericModal in Omnichannel hold/close mo… by Aryan107rn · Pull Request #41440 · RocketChat/Rocket.Chat

Aryan107rn · 2026-07-17T11:56:50Z

What does this PR do?

Resolves long-standing TODO comments in two Omnichannel modal components by replacing
the manual Modal + sub-component assembly with the GenericModal abstraction from
@rocket.chat/ui-client — which is the established pattern throughout the codebase.

Files changed

apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx
- Replaced 10 raw Modal sub-component imports + manual JSX with a single <GenericModal variant='warning'> call
- Removed TODO: Replace Modal with GenericModal comment
apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx
- Replaced the form-wrapped <Modal> branch with <GenericModal wrapperFunction={...}>
- Removed 9 now-unused fuselage imports (Modal, Button, all Modal* sub-components)
- Removed TODO: Replace Modal with GenericModal comment

Type of change

Bug fix / Refactor (nothing chnage in behaviour of app)

Checklist

My changes do not introduce a visual or functional effect on app.
I added a changeset via .changeset/omnichannel-modal-to-generic-modal.md

Summary by CodeRabbit

UI Improvements
- Updated chat hold and close dialogs with a more consistent modal design.
- Added clearer warning styling, icons, titles, and confirmation actions.
- Preserved existing form fields, validation, transcript options, and error messaging.
- Improved modal behavior so cancel and close actions work consistently.

…dals

dionisio-bot · 2026-07-17T11:56:54Z

Looks like this PR is not ready to merge, because of the following issues:

This PR is missing the 'stat: QA assured' label
This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

changeset-bot · 2026-07-17T11:56:56Z

🦋 Changeset detected

Latest commit: 8ed02fb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

CLAassistant · 2026-07-17T11:56:56Z

All committers have signed the CLA.

coderabbitai · 2026-07-17T11:58:31Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1401be24-e0f3-4c36-8b78-2479dce4434b

📥 Commits

Reviewing files that changed from the base of the PR and between 65a366e and 9d57353.

📒 Files selected for processing (3)

.changeset/omnichannel-modal-to-generic-modal.md
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx
apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

GitHub Check: cubic · AI code reviewer

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx

🧠 Learnings (3)

📚 Learning: 2026-03-16T21:50:37.589Z

Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.

Applied to files:

.changeset/omnichannel-modal-to-generic-modal.md

📚 Learning: 2026-03-27T14:52:56.865Z

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.

Applied to files:

apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx

📚 Learning: 2026-05-06T12:21:44.083Z

Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.

Applied to files:

apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx
apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx

🔇 Additional comments (3)

apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx (1)

1-1: LGTM!

Also applies to: 10-24

apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx (1)

5-12: LGTM!

Also applies to: 146-230

.changeset/omnichannel-modal-to-generic-modal.md (1)

1-6: LGTM!

Walkthrough

Both omnichannel modal components now use GenericModal instead of Fuselage modal components. Existing form fields and validation remain in CloseChatModal, while PlaceChatOnHoldModal updates its presentation and callback wiring. A patch changeset records the migration.

Changes

Omnichannel modal migration

Layer / File(s)	Summary
PlaceChatOnHoldModal migration `apps/meteor/app/livechat-enterprise/client/components/modals/PlaceChatOnHoldModal.tsx`	Replaces the Fuselage modal with a warning `GenericModal`, updates localized labels and icon configuration, and routes confirmation and cancellation callbacks through the new API.
CloseChatModal migration `apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx`, `.changeset/omnichannel-modal-to-generic-modal.md`	Replaces the modal shell with `GenericModal`, preserves the form and validation behavior, and records a patch release changeset.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: type: chore

Suggested reviewers: tassoevan

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: replacing raw Modals with GenericModal in the Omnichannel hold/close modals.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…c-modal

cubic-dev-ai

All reported issues were addressed

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

…m branch

…c-modal

fix: replace raw Modal with GenericModal in Omnichannel hold/close mo…

9d57353

…dals

Aryan107rn requested a review from a team as a code owner July 17, 2026 11:56

dionisio-bot Bot added the community label Jul 17, 2026

coderabbitai Bot added the type: chore label Jul 17, 2026

Merge branch 'develop' into fix/omnichannel-replace-modal-with-generi…

480387d

…c-modal

cubic-dev-ai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread apps/meteor/client/views/omnichannel/modals/CloseChatModal.tsx

Aryan107rn added 3 commits July 17, 2026 18:14

fix: add missing variant='warning' to CloseChatModal GenericModal for…

d82f2f3

…m branch

fix: add missing variant='warning' to CloseChatModal GenericModal for…

8c336b5

…m branch

Merge branch 'develop' into fix/omnichannel-replace-modal-with-generi…

8ed02fb

…c-modal

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: replace raw Modal with GenericModal in Omnichannel hold/close mo…#41440

fix: replace raw Modal with GenericModal in Omnichannel hold/close mo…#41440
Aryan107rn wants to merge 5 commits into
RocketChat:developfrom
Aryan107rn:fix/omnichannel-replace-modal-with-generic-modal

Aryan107rn commented Jul 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

dionisio-bot Bot commented Jul 17, 2026

Uh oh!

changeset-bot Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Jul 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Aryan107rn commented Jul 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Files changed

Type of change

Checklist

Summary by CodeRabbit

Uh oh!

dionisio-bot Bot commented Jul 17, 2026

Uh oh!

changeset-bot Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

CLAassistant commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Aryan107rn commented Jul 17, 2026 •

edited by coderabbitai Bot

Loading

changeset-bot Bot commented Jul 17, 2026 •

edited

Loading

CLAassistant commented Jul 17, 2026 •

edited

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading