chore: Remove style from editor and setting config#38756
Conversation
WalkthroughThe pull request involves removing default width specifications for action settings configurations across multiple plugin JSON files on the server side. The changes primarily focus on eliminating the Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🔥 Files not summarized due to errors (1)
💤 Files with no reviewable changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (9)
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 |
github-actions
bot
added
IDE Pod
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12861869418. |
Failed server tests
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12862309739. |
|
Deploy-Preview-URL: https://ce-38756.dp.appsmith.com |
![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
🧹 Nitpick comments (4)
app/client/src/components/formControls/FieldArrayControl.tsx (1)
31-39: Consider removing fixed width for dropdown controls.While the recent changes improve layout flexibility, the dropdown controls still use a fixed width of
12vw. Consider making this responsive as well to maintain consistency with the other style changes.- & > .t--form-control-DROP_DOWN, - & > .t--form-control-DROP_DOWN > div > div, - & > .t--form-control-DROP_DOWN > div > div > div > div { - width: 12vw; - }app/client/src/components/formControls/DynamicInputTextControl.tsx (1)
Line range hint
79-91: Clean up unused prop and TODO comment.Since custom styles have been removed, we should:
- Remove the unused
customStylesprop- Address the TODO comment about eslint disable
- // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - customStyles?: any;app/client/src/components/formControls/DropDownControl.tsx (1)
88-99: Remove commented-out width style.The width style is commented out but still present in the styles object. Either remove it completely or document why it's being kept.
const styles = { - // width: "280px", ...("customStyles" in this.props && typeof this.props.customStyles === "object" ? this.props.customStyles : {}), };app/client/src/PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css (1)
24-34: Consider documenting the reset styles.The reset styles for
.ads-v2-selectand.ar-form-info-textlack context. Add comments explaining why these resets are necessary.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/client/src/PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css(1 hunks)app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx(2 hunks)app/client/src/components/formControls/DropDownControl.tsx(1 hunks)app/client/src/components/formControls/DynamicInputTextControl.tsx(2 hunks)app/client/src/components/formControls/DynamicTextFieldControl.tsx(1 hunks)app/client/src/components/formControls/FieldArrayControl.tsx(1 hunks)app/client/src/components/formControls/InputTextControl.tsx(1 hunks)app/server/appsmith-plugins/anthropicPlugin/src/main/resources/editor/chat.json(3 hunks)app/server/appsmith-plugins/anthropicPlugin/src/main/resources/editor/vision.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- app/client/src/components/formControls/DynamicTextFieldControl.tsx
- app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/server/appsmith-plugins/anthropicPlugin/src/main/resources/editor/vision.json
- app/server/appsmith-plugins/anthropicPlugin/src/main/resources/editor/chat.json
⏰ 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: server-unit-tests / server-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (5)
app/client/src/components/formControls/FieldArrayControl.tsx (2)
31-31: LGTM! Improved responsive layout.The change from
min-contentto100%width enhances the component's responsiveness.
36-39: LGTM! Better space distribution.Adding
flex: 1ensures child elements share available space evenly, which is a good responsive design practice.app/client/src/components/formControls/DynamicInputTextControl.tsx (1)
Line range hint
29-42: LGTM! Good use of design system variables.The styling changes improve consistency by using design system variables and enhance user feedback with hover states.
app/client/src/components/formControls/InputTextControl.tsx (1)
194-197: LGTM! Verify usage of control type identifier.The implementation of
getControlTypeis clean and follows best practices.Let's verify how this control type identifier is used in the new styling system:
✅ Verification successful
Implementation follows standard control type pattern
The
getControlTypemethod implementation is consistent with the application's form control patterns and is safely isolated from the style removal changes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of INPUT_TEXT in style-related files rg -l "INPUT_TEXT" | rg -i "style|theme|css" # Search for any removed style references to this control type git log -p | rg -B2 -A2 "\.t--form-control-INPUT_TEXT"Length of output: 118
Script:
#!/bin/bash # Check content of the style config file cat app/client/src/widgets/TableWidgetV2/widget/propertyConfig/styleConfig.ts # Search for all INPUT_TEXT occurrences rg "INPUT_TEXT" -A 2 -B 2 # Look for interface/type definitions related to control types ast-grep --pattern 'interface $_ { $$$ getControlType$$$ $$$ }'Length of output: 66206
app/client/src/components/formControls/DropDownControl.tsx (1)
88-99: LGTM! Clean implementation of the dropdown wrapper.The replacement of the styled component with a simple div maintains functionality while reducing complexity.
.../PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12863060199. |
|
Deploy-Preview-URL: https://ce-38756.dp.appsmith.com |
...lient/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx
Show resolved
Hide resolved
.../PluginActionEditor/components/PluginActionForm/components/ActionForm/Zone/styles.module.css
Show resolved
Hide resolved
There was a problem hiding this comment.
Update the below file to remove width for settings:
https://github.com/appsmithorg/appsmith/blob/release/app/client/src/constants/AppsmithActionConstants/formConfig/QuerySettingsConfig.ts
ankitakinger
left a comment
ankitakinger
left a comment
There was a problem hiding this comment.
Also, queries are no longer showing numbering like JS object.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12866830147. |
|
Deploy-Preview-URL: https://ce-38756.dp.appsmith.com |
|
Following files need |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12881509024. |
|
Deploy-Preview-URL: https://ce-38756.dp.appsmith.com |
ankitakinger
left a comment
ankitakinger
left a comment
There was a problem hiding this comment.
Looks good to me even after DP testing.
| className={`t--${props?.name} uqi-dynamic-input-text`} | ||
| style={customStyle} | ||
| > | ||
| <div className={`t--${props?.name} uqi-dynamic-input-text`}> |
There was a problem hiding this comment.
nit: name is a required prop of string type, so there is no need to check if it's present.
|
We cannot merge this in. Seems there are some UI issues in side by side mode as compared to release. @hetunandu |
ankitakinger
left a comment
ankitakinger
left a comment
There was a problem hiding this comment.
Need to look at UI fixes for side by side mode
3 participants
Description
Removes any pending Style overrides in settings and editor config that was used by the old Query editor flows
Fixes #36897
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12879737201
Commit: fbfef1a
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Tue, 21 Jan 2025 04:56:53 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Style
Refactor