chore: add max height for textarea#39542
Conversation
WalkthroughThis pull request introduces enhancements to the textarea input elements by adding a Changes
Sequence Diagram(s)sequenceDiagram
participant S as Storybook
participant T as TextArea Component
participant B as Browser
S->>T: Render TextArea with maxRows prop (e.g., 8)
T->>T: Set CSS variable (--max-height) based on maxRows
T->>B: Renders component with applied CSS rule for textarea
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🪛 Biome (1.9.4)app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx[error] 115-115: Avoid redundant It is not necessary to use (lint/complexity/noExtraBooleanCast) ⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (5)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css(1 hunks)app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx(2 hunks)app/client/packages/design-system/widgets/src/components/TextArea/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/TextArea/stories/TextArea.stories.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx
[error] 117-117: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: chromatic-deployment
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
🔇 Additional comments (5)
app/client/packages/design-system/widgets/src/components/TextArea/stories/TextArea.stories.tsx (1)
75-80: Good addition of the MaxHeight story to demonstrate the new prop.This story effectively showcases the
maxHeightfeature, allowing users to see how the TextArea behaves with a max height constraint. Consider adding a description to clarify what this story demonstrates.app/client/packages/design-system/widgets/src/components/TextArea/src/types.ts (1)
14-14: Appropriate type definition for maxHeight prop.The optional numeric type is suitable for this property and matches the implementation in the component.
app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (2)
54-54: CSS implementation correctly adds max-height constraint.This CSS property works well with the custom variable to apply the maximum height to textarea elements.
17-19: Verify if overflow property should be removed.There's a comment on line 17 suggesting to "Delete overflow: hidden; when the max height is added", but the property still exists on line 18. Please verify if this comment is outdated or if the overflow property should be removed now that max-height has been implemented.
app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx (1)
33-33: Good addition of maxHeight prop extraction.The maxHeight prop is properly extracted from the props object.
| "--input-height": Boolean(textFieldHeight) | ||
| ? `${textFieldHeight}px` | ||
| : "auto", | ||
| "--max-height": Boolean(maxHeight) ? `${maxHeight}px` : "none", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant Boolean() call.
The Boolean() call is unnecessary since the ternary operator will already coerce the value to a boolean.
- "--max-height": Boolean(maxHeight) ? `${maxHeight}px` : "none",
+ "--max-height": maxHeight ? `${maxHeight}px` : "none",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "--max-height": Boolean(maxHeight) ? `${maxHeight}px` : "none", | |
| "--max-height": maxHeight ? `${maxHeight}px` : "none", |
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css(1 hunks)app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx(2 hunks)app/client/packages/design-system/widgets/src/components/TextArea/src/types.ts(1 hunks)app/client/packages/design-system/widgets/src/components/TextArea/stories/TextArea.stories.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx
[error] 117-117: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: chromatic-deployment
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
🔇 Additional comments (4)
app/client/packages/design-system/widgets/src/components/TextArea/stories/TextArea.stories.tsx (1)
76-80: Appropriate story added for maxHeight demonstration.The story correctly showcases the
maxHeightproperty with a value of 100px, which follows the same pattern as other stories.app/client/packages/design-system/widgets/src/components/TextArea/src/types.ts (1)
14-14: Type definition correctly added.The
maxHeightproperty is properly typed as an optional number parameter, which aligns with its usage.app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (1)
54-54: CSS property appropriately implemented.The max-height CSS property effectively leverages the custom variable, allowing dynamic control through props.
app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx (1)
33-33: Prop correctly destructured from component props.The
maxHeightproperty is appropriately extracted from props for use in the component.
| "--input-height": Boolean(textFieldHeight) | ||
| ? `${textFieldHeight}px` | ||
| : "auto", | ||
| "--max-height": Boolean(maxHeight) ? `${maxHeight}px` : "none", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant Boolean call.
The Boolean conversion is unnecessary as the value will already be coerced to a boolean in this context.
- "--max-height": Boolean(maxHeight) ? `${maxHeight}px` : "none",
+ "--max-height": maxHeight ? `${maxHeight}px` : "none",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "--max-height": Boolean(maxHeight) ? `${maxHeight}px` : "none", | |
| "--max-height": maxHeight ? `${maxHeight}px` : "none", |
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
| align-items: flex-start; | ||
| resize: none; | ||
| font-family: inherit; | ||
| max-height: var(--max-height); |
There was a problem hiding this comment.
What if we use maxRows instead of the max-height? So let's say we have maxRows={5}, then we can calculate max-height like this: typography.body.lineHeight*maxRows.
cc @ichik
There was a problem hiding this comment.
Yes, it would make a lot of sense to use rows instead of raw pixels here.
/ok-to-test tags="@tag.Anvil" https://github.com/user-attachments/assets/b189bdb2-2391-4cd8-8867-7c233aba0059 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Updated textarea inputs with a maximum height setting to improve layout consistency. - The TextArea component now accepts a new property for specifying its maximum number of rows. - **Documentation** - Added a Storybook example demonstrating the new max height functionality for the TextArea component. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/13652776635> > Commit: fc4fd3f > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13652776635&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Tue, 04 Mar 2025 11:57:15 UTC <!-- end of auto-generated comment: Cypress test results -->
3 participants
/ok-to-test tags="@tag.Anvil"
CleanShot.2025-03-04.at.13.00.56.mp4
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13652776635
Commit: fc4fd3f
Cypress dashboard.
Tags:
@tag.AnvilSpec:
Tue, 04 Mar 2025 11:57:15 UTC