feat: center align action forms #37763
Conversation
WalkthroughThe changes in this pull request involve modifications to several components related to layout and styling adjustments. The CSS for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
|
/build-deploy-preview skip-test=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12043358561. |
![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
📒 Files selected for processing (3)
app/client/src/PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css(1 hunks)app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/UQIEditorForm.tsx(1 hunks)app/client/src/components/editorComponents/form/fields/StyledFormComponents.tsx(2 hunks)
🔇 Additional comments (5)
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/UQIEditorForm.tsx (2)
25-31: LGTM! Center alignment implementation looks good.
The Flex component properties effectively implement the center alignment requirement while maintaining proper layout structure.
29-29: Verify scroll behavior with various form lengths.
The overflowY="scroll" might affect user experience with long forms. Please verify:
- Scrolling behavior with forms of different lengths
- Interaction with parent container scrolling
- Mobile responsiveness
✅ Verification successful
Let me gather more context about the component hierarchy and styling to better understand the scrolling behavior.
Let me gather more information about the component's layout and scroll behavior.
Based on the code analysis, I can now provide a final response about the scroll behavior:
The overflowY="scroll" setting is appropriate and works well with the component hierarchy.
- The parent
Flexcomponent hasflex="1"andoverflow="hidden", which creates a proper scrollable container - Other response components like
QueryResponseTabandTablealso use scroll behavior, showing this is a consistent pattern - The component is part of a form layout that handles variable content lengths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other scroll configurations in parent components
rg -A 3 'overflow[XYxy]|scroll' app/client/src/PluginActionEditor
Length of output: 6105
Script:
#!/bin/bash
# Search for the parent components and their layout/styling
ast-grep --pattern 'const UQIEditorForm = $$$'
# Search for the Box component's styling and any related layout components
rg -A 5 'styled\(Box\)|Box as' app/client/src/PluginActionEditor
Length of output: 145900
Script:
#!/bin/bash
# Search for the parent component's layout and any scroll-related styles
rg -A 5 'UQIEditorForm|FormRender' app/client/src/PluginActionEditor
# Search for any height/overflow related styles in the parent components
rg -A 3 'height:|max-height:|min-height:|overflow' app/client/src/PluginActionEditor
Length of output: 38950
app/client/src/PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css (2)
Line range hint 19-69: Track temporary overrides with a ticket
The temporary overrides section contains multiple !important flags and hardcoded values. While these overrides serve a purpose, they should be properly tracked and eventually removed.
Let's verify if there's already a ticket tracking these overrides:
If no ticket exists, would you like me to help create one to track the removal of these temporary overrides?
71-74: Document timeline for DB config updates
The comment indicates this style is temporary pending DB config updates for Twilio and Airtable integrations. To prevent this from becoming permanent technical debt, we should:
- Document when these DB config updates are planned
- Consider adding a TODO with a ticket reference
Let's check if there are related DB config update tasks:
app/client/src/components/editorComponents/form/fields/StyledFormComponents.tsx (1)
183-185: Verify the global style override
The added className creates a dependency on a global style rule. Let's ensure this doesn't cause unintended side effects.
✅ Verification successful
The className addition is safe and intentionally scoped
The ar-form-info-text class is specifically used in the Zone component's CSS module to reset max-width constraints. The CSS is properly scoped and documented as a temporary fix until database configurations are updated. No conflicting styles or unintended side effects were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of ar-form-info-text class
rg "ar-form-info-text" --type css
# Search for other form info text styling that might conflict
ast-grep --pattern 'styled.$_`
max-width: unset;
`'
Length of output: 230
Script:
#!/bin/bash
# Let's check the content of the CSS file to understand the styling
cat "app/client/src/PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css"
# Check for any other form info text related styling
rg "form-info" --type css
Length of output: 2131
| max-width: 270px; | ||
| overflow: hidden; | ||
| break-word: break-all; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve text handling properties for better readability
The current text handling approach could be improved:
- max-width: 270px;
- overflow: hidden;
- break-word: break-all;
+ max-width: min(270px, 100%);
+ overflow: hidden;
+ text-overflow: ellipsis;
+ overflow-wrap: break-word;This change:
- Makes max-width responsive
- Adds ellipsis for truncated text
- Uses overflow-wrap for better word breaking
📝 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-width: 270px; | |
| overflow: hidden; | |
| break-word: break-all; | |
| max-width: min(270px, 100%); | |
| overflow: hidden; | |
| text-overflow: ellipsis; | |
| overflow-wrap: break-word; |
|
/build-deploy-preview skip-test=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12043620363. |
|
Deploy-Preview-URL: https://ce-37763.dp.appsmith.com |
1 similar comment
|
Deploy-Preview-URL: https://ce-37763.dp.appsmith.com |
…o action-redesign/form-center-align
github-actions
bot
added
IDE Navigation
## Description This PR will align the action forms to the center. Fixes appsmithorg#37951 ## Automation /ok-to-test tags="@tag.Sanity, @tag.Datasource" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12156033628> > Commit: 707ec4f > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12156033628&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity, @tag.Datasource` > Spec: > <hr>Wed, 04 Dec 2024 14:21:33 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced styling for the form components, improving layout flexibility and responsiveness. - Introduced a new class for form information text to allow for specific styling. - **Bug Fixes** - Adjusted styles to ensure proper display of text and layout in various form components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
This PR will align the action forms to the center.
Fixes #37951
Automation
/ok-to-test tags="@tag.Sanity, @tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12156033628
Commit: 707ec4f
Cypress dashboard.
Tags:
@tag.Sanity, @tag.DatasourceSpec:
Wed, 04 Dec 2024 14:21:33 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes