feat: Expandable Message Composer#36920
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 |
|
WalkthroughAdds an expandable message composer behind a feature-preview flag, introducing MessageComposerInputExpandable, wiring the feature flag into the client registry and i18n, updating exports, stories, and tests, and conditionally rendering the expandable input in the room message box. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant MB as MessageBox
participant FP as FeaturePreview
participant MCX as MessageComposerInputExpandable
participant MCI as MessageComposerInput
U->>MB: Open room composer
MB->>FP: Is "expandableMessageComposer" enabled?
alt enabled
FP-->>MB: enabled
MB->>MCX: Render expandable input (props forwarded)
else disabled
FP-->>MB: disabled
MB->>MCI: Render standard input (props forwarded)
end
sequenceDiagram
autonumber
participant U as User
participant MCX as MessageComposerInputExpandable
participant TA as Textarea
U->>MCX: Type text
MCX->>TA: Update value
alt dimensions.blockSize > 100
MCX-->>U: Show "Expand" button (i18n)
U->>MCX: Click Expand
MCX->>TA: Apply height=500, maxHeight=50vh
MCX-->>U: Button becomes "Collapse"
U->>MCX: Click Collapse or clear text
MCX->>TA: Remove expanded styles (collapse)
MCX-->>U: Button becomes "Expand"
else
MCX-->>U: No expand button shown
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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 #36920 +/- ##
===========================================
+ Coverage 64.80% 67.42% +2.61%
===========================================
Files 2988 3287 +299
Lines 106831 111781 +4950
Branches 19290 20560 +1270
===========================================
+ Hits 69233 75367 +6134
+ Misses 35372 33736 -1636
- Partials 2226 2678 +452
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
MartinSchoeler
force-pushed
the
feat/expand-composer
branch
from
98b15f8 to
84d5c42
Compare
MartinSchoeler
force-pushed
the
feat/expand-composer
branch
from
3b4ad69 to
63f202f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (16)
packages/ui-composer/src/MessageComposerInputExpandable/index.ts (1)
1-1: Also re-export the props type from the folder index.Consumers won’t be able to import the component’s props type from the package root because this index only re-exports the default. Re-export the type to keep the public API complete.
Apply:
export { default as MessageComposerInputExpandable } from './MessageComposerInputExpandable'; +export type { ExpandComposerButtonProps } from './MessageComposerInputExpandable';packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.tsx (2)
19-24: Auto-collapse should also react to external clears (controlled value).Collapsing only on input events misses cases where the parent clears the value (e.g., after send). Watch
props.valueto collapse when it becomes empty.Apply:
const { t } = useTranslation(); + // Collapse when parent clears the input (controlled component) + useEffect(() => { + if (props.value === '' || props.value == null) { + setExpanded(false); + } + }, [props.value]); + const onChange = (event: ChangeEvent<HTMLTextAreaElement>) => { - props.onChange?.(event); - if (event.target.value.length === 0) { + props.onChange?.(event); + if (event.currentTarget.value.length === 0) { setExpanded(false); } };
49-51: Minor: merge conditional props for less churn.Combine the conditional spread into one to reduce prop object allocations (non-blocking).
Apply:
- {...(!!expanded && { height: 500 })} - {...(!!expanded && { maxHeight: '50vh' })} + {...(expanded ? { height: 500, maxHeight: '50vh' } : {})}packages/i18n/src/locales/en.i18n.json (1)
2027-2028: Align casing with existing feature names and clarify behavior in descriptionMost feature names nearby use sentence case (e.g., “Adjustable font size”). Also, clarifying the “max height” trigger aligns with the PR objective.
Apply this diff:
- "Expandable_message_composer": "Expandable Message Composer", - "Expandable_message_composer_description": "Adds a button to expand the message composer vertically", + "Expandable_message_composer": "Expandable message composer", + "Expandable_message_composer_description": "Shows an expand button when the composer reaches its maximum height",packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.spec.tsx (8)
10-13: Avoid broad story snapshots; keep a lightweight smoke test insteadSnapshotting full DOM for every story increases flakiness and review noise.
Replace with a minimal render assertion:
-test.each(testCases)(`renders %s without crashing`, async (_storyname, Story) => { - const tree = render(<Story />); - expect(tree.baseElement).toMatchSnapshot(); -}); +test.each(testCases)(`renders %s`, (_storyname, Story) => { + const { container } = render(<Story />); + expect(container.firstChild).toBeTruthy(); +});
23-37: Make button queries resilient and i18n-safeRelying on getByRole('button') and asserting aria-label === 'Expand' is brittle with additional buttons and localization.
Use accessible name filtering and assertions:
-const expandButton = screen.getByRole('button'); +const expandButton = screen.getByRole('button', { name: /expand/i }); expect(expandButton).toBeInTheDocument(); -expect(expandButton).toHaveAttribute('aria-label', 'Expand'); +expect(expandButton).toHaveAccessibleName(/expand/i);If toHaveAccessibleName isn’t available, keep the name filter and drop the attribute assertion.
39-52: Tighten the negative query to the intended controlFuture UI changes could add other buttons; scope the query by accessible name.
-const expandButton = screen.queryByRole('button'); +const expandButton = screen.queryByRole('button', { name: /expand/i }); expect(expandButton).not.toBeInTheDocument();
65-78: Avoid hard-coding expanded height; expose/read a constant or propAsserting '500px' couples tests to implementation details.
- Export a constant (e.g., DEFAULT_EXPANDED_HEIGHT = 500) from the component and import it in tests, or
- Allow an expandedHeight prop for testability and pass it explicitly in tests.
Example:
-<MessageComposerInputExpandable +<MessageComposerInputExpandable + expandedHeight={500} dimensions={{ inlineSize: 400, blockSize: 120 }} placeholder='Type a message...' />Then assert using that value.
71-76: Use userEvent and await DOM updates to reduce flakinessState updates may be async; fireEvent with immediate assertions can be flaky.
- Switch to @testing-library/user-event.
- Await changes with waitFor.
Minimal example:
-import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; - fireEvent.click(expandButton); + await userEvent.click(expandButton); - expect(textarea).toHaveStyle({ height: '500px' }); + await waitFor(() => expect(textarea).toHaveStyle({ height: '500px' }));Apply similarly for collapse and auto‑collapse.
Also applies to: 98-104, 132-135
103-105: Also assert the accessible name after collapseStrengthen the assertion by checking the control’s name toggles back.
expect(textarea).not.toHaveStyle({ maxHeight: '500px' }); -expect(expandButton).toHaveAttribute('aria-label', 'Expand'); +expect(expandButton).toHaveAccessibleName(/expand/i);
23-37: Test keyboard activation for accessibilityAdd tests for Space/Enter to toggle expand/collapse on the button.
Example:
await userEvent.keyboard('{Tab}'); // focus button await userEvent.keyboard('{Enter}'); expect(textarea).toHaveStyle({ height: `${DEFAULT_EXPANDED_HEIGHT}px` });Also applies to: 54-63, 80-89, 107-116
23-37: De-duplicate the “100px threshold” assumptionBoth tests rely on a 100px threshold that’s likely hard-coded in the component. Prefer importing a shared constant or setting via prop.
- Export THRESHOLD_BLOCK_SIZE from the component and import it here; or
- Accept a minBlockSizeToShowExpand prop in tests and stories.
Also applies to: 39-52
packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.stories.tsx (3)
87-101: Avoid controlled value without onChange in StorybookProviding value without onChange produces a read‑only control and dev warnings. Use defaultValue for static content.
- value='This is some sample text to demonstrate the expandable input with content.' + defaultValue='This is some sample text to demonstrate the expandable input with content.'Alternatively, add readOnly if you must keep value.
103-116: Don’t duplicate the 100px threshold in storiesKeep the threshold in one place to prevent drift.
- Import a THRESHOLD_BLOCK_SIZE constant from the component and reference it in the comment/args; or
- Expose a prop (e.g., minBlockSizeToShowExpand) for demos/tests and pass 100 explicitly.
17-21: Prefer CSF3 StoryObj for simpler storiesUsing StoryObj reduces boilerplate and plays nicer with tooling and test reuse.
Refactor to:
import type { Meta, StoryObj } from '@storybook/react'; const meta: Meta<typeof MessageComposerInputExpandable> = { title: 'Components/MessageComposerInputExpandable', component: MessageComposerInputExpandable }; export default meta; export type Story = StoryObj<typeof MessageComposerInputExpandable>;Then define stories with args instead of inline functions where possible.
apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx (1)
427-455: Extract shared input props and merge styles correctly
- Factor out repeated props into a
commonInputPropsobject and spread it into both<MessageComposerInputExpandable>and<MessageComposerInput>.- In
packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.tsx(around lines 45–51), merge the incomingstyleprop after the component’s computed height/maxHeight so parent overrides take effect.
📜 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)
apps/meteor/public/images/featurePreview/expandable-composer.pngis excluded by!**/*.pngpackages/ui-composer/src/MessageComposerInputExpandable/__snapshots__/MessageComposerInputExpandable.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx(3 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/ui-client/src/hooks/useFeaturePreviewList.ts(2 hunks)packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.spec.tsx(1 hunks)packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.stories.tsx(1 hunks)packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.tsx(1 hunks)packages/ui-composer/src/MessageComposerInputExpandable/index.ts(1 hunks)packages/ui-composer/src/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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:
packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.spec.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
packages/ui-client/src/hooks/useFeaturePreviewList.ts (2)
8-9: Union type updates look good.Adding 'secondarySidebar' and 'expandableMessageComposer' to the union aligns the type with defaults below.
77-85: Verified i18n keys and image asset exist. BothExpandable_message_composerandExpandable_message_composer_descriptionare present in packages/i18n/src/locales/en.i18n.json andimages/featurePreview/expandable-composer.pngexists.packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.tsx (2)
15-16: forwardRef generic stays correct after props fix.No change needed here beyond the props type update above.
38-42: Verify Fuselage icon names in MessageComposerInputExpandable.tsx
Translations ‘Collapse’/‘Expand’ are present; confirm ‘arrow-collapse’/‘arrow-expand’ are valid Fuselage icons (e.g., swap to ‘chevron-up’/‘chevron-down’ if not).packages/ui-composer/src/index.ts (1)
3-3: Barrel re-export is fine; ensure props type is exposed upstream.With
export * from './MessageComposerInputExpandable', re-exporting the props type from the folder index (see comment in that file) will make it available from the package root.apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx (1)
4-4: Exports confirmed: FeaturePreview, FeaturePreviewOn, FeaturePreviewOff, and useSafeRefCallback are exported from @rocket.chat/ui-client; MessageComposerInputExpandable is exported from @rocket.chat/ui-composer.packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.spec.tsx (2)
15-20: Matcher already registered in jest setup
The jest-axe matcher is globally extended in jest-presets/src/client/jest-setup.ts (import { toHaveNoViolations } from 'jest-axe'; expect.extend(toHaveNoViolations);), so no additional import is needed.Likely an incorrect or invalid review comment.
1-3: Verify composeStories import for Storybook v8
Storybook 8 exportscomposeStoriesfrom@storybook/test(renderer-agnostic). If you’ve upgraded to v8, replace:-import { composeStories } from '@storybook/react'; +import { composeStories } from '@storybook/test';Ensure
@storybook/testis installed and adjust imports accordingly.
packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.tsx
Outdated
Show resolved
Hide resolved
packages/ui-composer/src/MessageComposerInputExpandable/MessageComposerInputExpandable.tsx
Outdated
Show resolved
Hide resolved
dougfabris
force-pushed
the
feat/expand-composer
branch
from
3db6473 to
3211839
Compare
dougfabris
force-pushed
the
feat/expand-composer
branch
from
3211839 to
47224cd
Compare
scuciatto
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Co-authored-by: Douglas Fabris <27704687+dougfabris@users.noreply.github.com>
3 participants
Proposed changes (including videos or screenshots)
This PR should be a proposal for this feature.
Screen.Recording.2025-09-11.at.14.55.02.mov
Why add this?
Given the way our current composer height limitations, sometimes larger messages are difficult to write and edit, due the need to keep scrolling up and down. While having the composer always grow to a large height is not ideal, and not desired in most situations, this PR proposes to add a "expand" button that only appears once you've reached the height limit of the composer.
Also we have some planned GSoC Projects that would heavily benefit from this feature
#35975
and
#36526
Issue(s)
COMM-30
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Documentation
Tests