-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: fix text area ui bug #38282
chore: fix text area ui bug #38282
Conversation
WalkthroughThis pull request introduces refinements to input component styling and textarea height calculation. The changes focus on enhancing the layout and visual properties of input elements, specifically modifying CSS styles for input groups and adjusting textarea height computation. The modifications aim to improve the rendering and responsiveness of input components across different states and sizes. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (1)
186-198
: Consider using CSS custom properties for sizesThe size calculations look good, but consider extracting magic numbers into CSS custom properties for better maintainability.
Example:
:root { --input-base-block-size: var(--body-line-height); --input-small-padding: var(--inner-spacing-2); --input-large-padding: var(--inner-spacing-3); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css
(4 hunks)app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx
(1 hunks)
🔇 Additional comments (4)
app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx (1)
79-84
: Verify height calculation changes across browsers
The removal of the 1px addition and simplified height calculation might affect textarea rendering in different browsers.
Let's verify if there are any related issues or discussions:
✅ Verification successful
Let me check the implementation details of these files to better understand the height calculation logic.
Height calculation changes are safe and correct
The removal of +1px is actually an improvement as the component now correctly uses the exact scrollHeight
minus padding. The implementation:
- Uses proper height calculation considering padding and borders
- Has Firefox-specific handling to avoid scroll position issues
- Includes resize observer to handle width changes
- Maintains proper overflow and alignment styles
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for related height calculation issues
rg -i "textarea.*height|scrollbar.*issue" --type ts
Length of output: 354
Script:
#!/bin/bash
# Get the content of both files
echo "=== TextArea.tsx ==="
cat app/client/packages/design-system/widgets/src/components/TextArea/src/TextArea.tsx
echo -e "\n=== AutoResizeTextArea.tsx ==="
cat app/client/src/components/editorComponents/AutoResizeTextArea.tsx
Length of output: 8457
app/client/packages/design-system/widgets/src/components/Input/src/styles.module.css (3)
27-30
: Review box-sizing impact on layout calculations
The change to content-box
from the default border-box
might affect size calculations and layout. Ensure this doesn't cause unexpected behavior with fixed-width containers.
32-37
: LGTM: Improved RTL support
The use of logical properties (padding-inline-start/end
) properly handles both LTR and RTL layouts.
62-68
: LGTM: Consistent spacing for prefix/suffix elements
The margin adjustments ensure uniform spacing around input add-ons.
This PR fixes UI bugs in text area that got introduced during refactoring of inputs. /ok-to-test tags="@tag.Anvil" <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced layout and visual feedback for input components with refined styles for focus, hover, and invalid states. - Improved handling of read-only inputs and conditional styling based on data attributes. - **Bug Fixes** - Adjusted height calculation logic for the TextArea component to ensure accurate sizing across browsers. <!-- 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/12429446430> > Commit: 28022c4 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12429446430&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Fri, 20 Dec 2024 10:28:51 UTC <!-- end of auto-generated comment: Cypress test results -->
This PR fixes UI bugs in text area that got introduced during refactoring of inputs.
/ok-to-test tags="@tag.Anvil"
Summary by CodeRabbit
New Features
Bug Fixes
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12429446430
Commit: 28022c4
Cypress dashboard.
Tags:
@tag.Anvil
Spec:
Fri, 20 Dec 2024 10:28:51 UTC