refactor: standardize CreateDiscussion modal using GenericModal #38334

NAME-ASHWANIYADAV · 2026-01-26T08:54:36Z

Description

This PR refactors the CreateDiscussion component to utilize the standardized
GenericModal component from @rocket.chat/ui-client, replacing the manual
composition of Modal, ModalHeader, ModalContent, and ModalFooter.

This change addresses existing technical debt identified by a TODO comment in
CreateDiscussion.tsx and aligns the component with the codebase’s modern UI
patterns.

Additionally, this PR enhances GenericModal by introducing a
confirmLoading prop, enabling loading states on the confirmation button during
async operations such as creating a discussion.

Related issues

Closes Refactor: Replace Modal usage with GenericModal in CreateDiscussion component #38333

Changes proposed

Refactor CreateDiscussion.tsx to replace direct usage of Modal
sub-components with GenericModal.
Enhance GenericModal.tsx by adding a confirmLoading prop to
GenericModalProps and forwarding it to the primary action button.
Cleanup: Remove unused imports (Modal, ModalHeader, ModalTitle,
ModalClose, ModalContent, ModalFooter).
Type safety: Remove the explicit ReactElement return type to improve type
inference and resolve linting issues.

Verification steps

Navigate to any channel or room.
Open the Discussions tab from the Contextual Bar (or via the + menu).
Click Create Discussion.
Verify UI: The modal appears correctly with the title Create Discussion.
Verify functionality: Fill in the details and click Create.
Observe the loading spinner on the Create button
(new confirmLoading functionality).
Verify the discussion is created successfully and you are redirected to it.
Verify cancellation: Click Cancel or close the modal and ensure it closes
as expected.

Summary by CodeRabbit

New Features
- Modal confirm buttons show a loading state during actions.
Refactor
- Discussion creation dialog updated with a new modal container and streamlined form layout.
- Parent-channel selection now uses a default field when applicable, otherwise an autocomplete selector.
- Extended input props support in parent room field component for improved flexibility.
Bug Fixes
- Form validations and enable/disable hints preserved and corrected for message/encryption interactions.
Tests
- Added tests to verify modal confirm loading behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

changeset-bot · 2026-01-26T08:54:41Z

⚠️ No Changeset found

Latest commit: 2000012

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

dionisio-bot · 2026-01-26T08:54:42Z

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

This PR has conflicts, please resolve them before merging
This PR is not mergeable
This PR is targeting the wrong base branch. It should target 8.2.0, but it targets 8.1.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

coderabbitai · 2026-01-26T08:55:18Z

Walkthrough

Replaces the deprecated Modal in CreateDiscussion with GenericModal, adds a confirmLoading prop to GenericModal, and expands DefaultParentRoomField to accept and forward additional TextInput props; adjusts form wiring and room-selection logic accordingly.

Changes

Cohort / File(s)	Summary
CreateDiscussion (modal refactor) `apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx`	Replaced manual `Modal` composition with `GenericModal`; moved header/body/footer into `GenericModal` wrapperFunction; rewired form fields (name, topic, members, firstMessage, encrypted) and room selection to use `DefaultParentRoomField` when `defaultParentRoom` exists or `RoomAutoComplete` (via `Controller`) otherwise; removed explicit `ReactElement` return type.
GenericModal (API & tests) `packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx`, `packages/ui-client/src/components/Modal/GenericModal/GenericModal.spec.tsx`	Added public prop `confirmLoading?: boolean`; pass `loading={confirmLoading}` to primary/Ok button and adjust onClick/submit behavior for wrapperFunction vs non-wrapper flows; added test asserting confirm button shows loading/disabled state.
DefaultParentRoomField (props forwarding) `apps/meteor/client/components/CreateDiscussion/DefaultParentRoomField.tsx`	Changed props to `DefaultParentRoomFieldProps` (includes `defaultParentRoom` plus forwarded `TextInput` props excluding `defaultValue`/`disabled`); spread props into `TextInput`; extended room-name resolution to include `name` and `prid`.

Sequence Diagram(s)

(Skipped — changes are primarily a refactor and do not introduce a new multi-component control flow requiring visualization.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

chore: new secondary-danger generic modal variant #37223 — Modifies GenericModal (adds variant/button props); related at the GenericModal API/behavior level.

Suggested reviewers

MartinSchoeler
dougfabris

🐰 Hooray, the modal hops anew,
GenericModal stitched through and through.
Buttons shimmer with patient light,
Room names pass props snug and tight.
A rabbit cheers this tidy view.

🚥 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 pull request title accurately describes the main change: refactoring CreateDiscussion to use GenericModal instead of manual Modal composition.
Linked Issues check	✅ Passed	The pull request successfully addresses all objectives from issue `#38333`: replaces Modal with GenericModal in CreateDiscussion.tsx, aligns with standardized modal patterns, and adds confirmLoading support for async operations.
Out of Scope Changes check	✅ Passed	Changes to GenericModal (confirmLoading prop) and DefaultParentRoomField (prop extension) are scope-appropriate enhancements supporting the CreateDiscussion refactor objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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 and usage tips.}

cubic-dev-ai

1 issue found across 6 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name=".changeset/add-emoji-keyword-search.md">

<violation number="1" location=".changeset/add-emoji-keyword-search.md:5">
P1: Unrelated changeset: declares emoji keyword search feature and minor meteor bump not present in this PR</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

.changeset/add-emoji-keyword-search.md

cubic-dev-ai

1 issue found across 4 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx">

<violation number="1" location="packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx:157">
P2: Submit path with `wrapperFunction` is treated as dismiss because `dismissedRef` is never cleared, leading to `onDismiss` firing after submit</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx

apps/meteor/app/emoji/client/helpers.ts

… and add confirmLoading prop

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx`:
- Around line 112-123: FieldLabel's htmlFor is using parentRoomId but when
defaultParentRoom is rendered the DefaultParentRoomField isn't receiving that id
so the label-input association is broken; update the Controller rendering of
DefaultParentRoomField (inside the parentRoom Controller) to forward the
parentRoomId (or an aria-labelledby that references FieldLabel) as a prop and
ensure DefaultParentRoomField applies that prop to its internal input element so
the FieldLabel htmlFor matches the actual input id.

apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx

…put association

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx`:
- Around line 217-223: The aria-describedby currently references
`${firstMessageId}-hint` even when `encrypted` is true and that hint element
isn't rendered; update the TextAreaInput in CreateDiscussion so aria-describedby
only includes the hint IDs that are actually rendered (i.e., include
`${firstMessageId}-hint` when not encrypted and include
`${firstMessageId}-encrypted-hint` when encrypted), apply the same conditional
logic to the other TextAreaInput usage noted (lines 227-231), and keep this
change within the CreateDiscussion component around the TextAreaInput props to
avoid broken ARIA references.
- Around line 161-167: The aria-describedby on the TextInput references
`${discussionNameId}-hint` but no hint element is rendered; either remove
`${discussionNameId}-hint` from the aria-describedby string or add a
visible/assistive hint element with that id next to the TextInput so the
reference is valid. Locate the TextInput usage (discussionNameId, field,
errors.name) and either (a) remove `${discussionNameId}-hint` from
aria-describedby leaving `${discussionNameId}-error` only, or (b) render a hint
element (e.g., a small help text element with id `${discussionNameId}-hint`)
adjacent to the input so assistive tech can resolve the reference.

apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx

…element

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx

apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx

apps/meteor/client/components/CreateDiscussion/DefaultParentRoomField.tsx

@dougfabris

- Update to warning variant with null icon - Remove unused modalId variable - Fix broken ARIA references for accessibility Addresses feedback from @dougfabris in PR RocketChat#38334

NAME-ASHWANIYADAV · 2026-01-27T15:07:39Z

@dougfabris All review comments addressed:

✅ Updated to warning variant with icon={null}
✅ Removed unused modalId
✅ Fixed accessibility ARIA references

Ready for re-review!

dougfabris · 2026-01-27T15:17:31Z

@NAME-ASHWANIYADAV Please make sure to check the lint issues and there some requests I did still pending

cubic-dev-ai

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx">

<violation number="1" location="packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx:157">
P2: Wrapper function confirm no longer clears dismissedRef, so modal cleanup may fire onDismiss/onCancel after a successful submit.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx

NAME-ASHWANIYADAV · 2026-01-27T17:18:18Z

sorry my this pr takes a lot of your effort i will make sure to improve myself and contribute more efficient by getting more familiar with the code base
Thank you !! @dougfabris

codecov · 2026-01-27T17:18:55Z

Codecov Report

❌ Patch coverage is 72.22222% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.78%. Comparing base (c6ec7a9) to head (2000012).
⚠️ Report is 7 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #38334   +/-   ##
========================================
  Coverage    70.77%   70.78%           
========================================
  Files         3159     3159           
  Lines       109401   109402    +1     
  Branches     19675    19672    -3     
========================================
+ Hits         77428    77436    +8     
+ Misses       29944    29934   -10     
- Partials      2029     2032    +3

Flag	Coverage Δ
e2e	`60.36% <68.75%> (+0.03%)`	⬆️
e2e-api	`47.94% <ø> (+0.07%)`	⬆️
unit	`71.94% <100.00%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dougfabris · 2026-01-27T18:16:49Z

Not worries @NAME-ASHWANIYADAV this was a very nice work.
Thank you so much for the help!

dougfabris

Thanks for the work here!

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx">

<violation number="1" location="packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx:156">
P2: Confirm button now always renders; for non-form modals this creates duplicate “Ok” buttons or a non-functional confirm button when `wrapperFunction` is absent.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx

dougfabris · 2026-01-27T18:44:13Z

why are you doing this? I approved your PR and you added more not related stuff?

NAME-ASHWANIYADAV · 2026-01-27T18:48:50Z

Sorry about that @dougfabris— my mistake 🙏

I acted on a bot comment without realizing the PR was already approved.
You’re right, I shouldn’t have pushed additional changes at that stage.

If you prefer, I can:
• revert the last commit, or
• leave it as-is if you think the fix is acceptable.

Please let me know how you’d like me to proceed.

dougfabris

Don't worry, everything is ok now!

…modal

kodiakhq · 2026-01-28T15:46:03Z

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

NAME-ASHWANIYADAV requested a review from a team as a code owner January 26, 2026 08:54

ggazzo added the Invalid PR Title label Jan 26, 2026

NAME-ASHWANIYADAV changed the title ~~Replace Modal with GenericModal in CreateDiscussion~~ refactor: standardize CreateDiscussion modal using GenericModal Jan 26, 2026

ggazzo removed the Invalid PR Title label Jan 26, 2026

cubic-dev-ai bot reviewed Jan 26, 2026

View reviewed changes

.changeset/add-emoji-keyword-search.md Outdated Show resolved Hide resolved

cubic-dev-ai bot reviewed Jan 26, 2026

View reviewed changes

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx Show resolved Hide resolved

dougfabris requested changes Jan 26, 2026

View reviewed changes

apps/meteor/app/emoji/client/helpers.ts Outdated Show resolved Hide resolved

refactor(client): replace Modal with GenericModal in CreateDiscussion…

298ce8e

… and add confirmLoading prop

NAME-ASHWANIYADAV force-pushed the refactor/38333-create-discussion-generic-modal branch from e1c615e to 298ce8e Compare January 27, 2026 06:09

coderabbitai bot reviewed Jan 27, 2026

View reviewed changes

apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx Show resolved Hide resolved

fix(a11y): pass id prop to DefaultParentRoomField for proper label-in…

fef2dfe

…put association

NAME-ASHWANIYADAV requested a review from dougfabris January 27, 2026 06:24

coderabbitai bot reviewed Jan 27, 2026

View reviewed changes

apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx Show resolved Hide resolved

apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx Show resolved Hide resolved

fix(a11y): conditionally set aria-describedby based on rendered hint …

2b80a0e

…element

dougfabris requested changes Jan 27, 2026

View reviewed changes

fix: address PR review feedback for CreateDiscussion refactor

c0f6fde

- Update to warning variant with null icon - Remove unused modalId variable - Fix broken ARIA references for accessibility Addresses feedback from @dougfabris in PR RocketChat#38334

NAME-ASHWANIYADAV requested a review from dougfabris January 27, 2026 15:07

fix: address review comments and clean up unused code

f9d1247

cubic-dev-ai bot reviewed Jan 27, 2026

View reviewed changes

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx Show resolved Hide resolved

chore: add alert role in FieldError

53799b9

dougfabris added this to the 8.2.0 milestone Jan 27, 2026

chore: remove changeset

1f33626

dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label Jan 27, 2026

dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jan 27, 2026

dougfabris previously approved these changes Jan 27, 2026

View reviewed changes

NAME-ASHWANIYADAV dismissed dougfabris’s stale review via 3eba2ca January 27, 2026 18:23

cubic-dev-ai bot reviewed Jan 27, 2026

View reviewed changes

packages/ui-client/src/components/Modal/GenericModal/GenericModal.tsx Outdated Show resolved Hide resolved

dougfabris force-pushed the refactor/38333-create-discussion-generic-modal branch from 904fdc8 to 1f33626 Compare January 27, 2026 19:25

dougfabris approved these changes Jan 27, 2026

View reviewed changes

Merge branch 'develop' into refactor/38333-create-discussion-generic-…

2000012

…modal

kodiakhq bot removed the stat: ready to merge PR tested and approved waiting for merge label Jan 28, 2026

dionisio-bot bot added the stat: conflict label Jan 28, 2026

NAME-ASHWANIYADAV requested a review from dougfabris January 28, 2026 16:07

refactor: standardize CreateDiscussion modal using GenericModal #38334

Are you sure you want to change the base?

refactor: standardize CreateDiscussion modal using GenericModal #38334

Conversation

NAME-ASHWANIYADAV commented Jan 26, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Changes proposed

Verification steps

Summary by CodeRabbit

Uh oh!

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

Uh oh!

⚠️ No Changeset found

Uh oh!

dionisio-bot bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NAME-ASHWANIYADAV commented Jan 27, 2026

Uh oh!

dougfabris commented Jan 27, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NAME-ASHWANIYADAV commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dougfabris commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dougfabris left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dougfabris commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NAME-ASHWANIYADAV commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

NAME-ASHWANIYADAV commented Jan 26, 2026 •

edited by coderabbitai bot

Loading

changeset-bot bot commented Jan 26, 2026 •

edited

Loading

dionisio-bot bot commented Jan 26, 2026 •

edited

Loading

coderabbitai bot commented Jan 26, 2026 •

edited

Loading

NAME-ASHWANIYADAV commented Jan 27, 2026 •

edited

Loading

codecov bot commented Jan 27, 2026 •

edited

Loading

dougfabris commented Jan 27, 2026 •

edited

Loading

dougfabris commented Jan 27, 2026 •

edited

Loading

NAME-ASHWANIYADAV commented Jan 27, 2026 •

edited

Loading