-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: Action redesign: Updating the config for SMTP plugin to use sections and zones format #36001
Conversation
WalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DynamicInputTextControl
participant SMTPPlugin
User->>DynamicInputTextControl: Interacts with input field
DynamicInputTextControl->>User: Displays enhanced input field
User->>SMTPPlugin: Configures email parameters
SMTPPlugin->>User: Provides multi-section layout
User->>SMTPPlugin: Enters CC/BCC and other details
SMTPPlugin->>User: Confirms email configuration
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
Early access features: disabledWe are currently testing the following features in early access:
Note:
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10629704553. |
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- app/client/src/components/formControls/DynamicInputTextControl.tsx (1 hunks)
- app/client/src/pages/Editor/ActionForm/Zone/styles.module.css (1 hunks)
- app/server/appsmith-plugins/smtpPlugin/src/main/resources/editor/root.json (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/components/formControls/DynamicInputTextControl.tsx
Additional comments not posted (3)
app/client/src/pages/Editor/ActionForm/Zone/styles.module.css (1)
51-54
: Review the impact of CSS changes on layout.The changes to the
.uqi-dynamic-input-text
class remove constraints on width and min-height, which could potentially affect the layout in various parts of the application. While the flexibility is appreciated, it's crucial to ensure that these changes do not introduce any layout issues elsewhere.Please verify the impact of these changes on the layout by checking the elements using this class across different pages and screen sizes.
app/server/appsmith-plugins/smtpPlugin/src/main/resources/editor/root.json (2)
4-69
: Review the new section and zone configurations.The introduction of
SECTION_V2
and variousDOUBLE_COLUMN_ZONE
controls represents a significant redesign aimed at improving the organization and functionality of the SMTP plugin's configuration. It's important to ensure that all new fields and controls are correctly configured and that the placeholders and evaluation substitution types are set up properly to enhance user interaction.Please verify that all new configurations are correctly implemented and function as expected during runtime. This includes checking the dynamic behavior of placeholders and evaluation substitutions.
70-174
: Ensure comprehensive testing of new email parameter controls.The addition of new fields for CC, BCC, and conditional visibility settings for the "Reply to email" field based on the "Set reply to email" switch is a welcome improvement. However, it's crucial to ensure that these new controls are thoroughly tested, especially the conditional visibility and the dynamic input functionalities.
Please conduct comprehensive testing to ensure that all new controls work as expected and that the conditional logic behaves correctly under various scenarios.
Deploy-Preview-URL: https://ce-36001.dp.appsmith.com |
@@ -47,6 +47,11 @@ | |||
} | |||
} | |||
} | |||
/* DynamicInputTextControl min height and width removed */ | |||
& :global(.uqi-dynamic-input-text) { |
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.
Do you really need a global selector here?
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.
Yeah thats how it has been done for other fields... hence I have added it the similar way. This global lies under the selector .zone
…o action-redesign/smtp
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10635374975. |
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/Editor/ActionForm/Zone/styles.module.css (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/pages/Editor/ActionForm/Zone/styles.module.css
Deploy-Preview-URL: https://ce-36001.dp.appsmith.com |
Failed server tests
|
…o action-redesign/smtp
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10659699107. |
Deploy-Preview-URL: https://ce-36001.dp.appsmith.com |
Adding @AmanAgarwal041 for review |
Description
Action redesign: Updating the config for SMTP plugin to use sections and zones format
Fixes #35505
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10659691143
Commit: 3917fdb
Cypress dashboard.
Tags:
@tag.All
Spec:
Mon, 02 Sep 2024 04:23:47 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes