chore: Updating the UI for save and edit datasource button in API editor #37302
Conversation
WalkthroughThe changes in this pull request enhance the functionality and clarity of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
IDE Pod
|
/build-deploy-preview skip-tests=true |
github-actions
bot
added
the
skip-changelog
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11741492545. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/components/StoreAsDatasource.tsx (1)
70-81: Consider accessibility improvements.The button could benefit from additional accessibility attributes.
<Button className="t--store-as-datasource" data-testid="t--store-as-datasource" isDisabled={!props.enable} kind="secondary" onClick={saveOrEditDatasource} size="md" + aria-label={props.shouldSave + ? createMessage(SAVE_DATASOURCE) + : createMessage(EDIT_DATASOURCE)} >app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx (1)
Line range hint
204-227: Consider adding type safety to the section parameter.The function implementation looks good, but it's using
anytype for the section parameter. Consider defining an interface for the section structure to improve type safety.interface FormSection { controlType: 'SECTION_V2' | 'SINGLE_COLUMN_ZONE' | 'DOUBLE_COLUMN_ZONE'; children: unknown[]; isFullWidth?: boolean; } const renderNodeWithChildren = (section: FormSection, formName: string) => { // ... rest of the implementation }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/components/StoreAsDatasource.tsx(2 hunks)app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx(2 hunks)app/client/src/ce/constants/messages.ts(1 hunks)
🔇 Additional comments (4)
app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/components/StoreAsDatasource.tsx (2)
14-17: LGTM: Import statements are properly organized.
The new tooltip message imports are correctly added and properly sourced from the enterprise edition constants.
63-82: LGTM: Tooltip implementation enhances button UX.
The implementation correctly adds contextual tooltips that change based on the button state, improving user understanding of the available actions.
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx (1)
173-173: LGTM - Fixed typo in function name.
The function name correction from rederNodeWithChildren to renderNodeWithChildren improves code clarity.
app/client/src/ce/constants/messages.ts (1)
369-371: LGTM! The changes improve clarity but verify text change impact.
The changes enhance UX by making the button text more specific and adding helpful tooltips. However, changing "Save" to "Save URL" is a breaking UI change.
Let's verify the impact of this text change:
✅ Verification successful
Based on the search results, I can see that the SAVE_DATASOURCE constant is used in the StoreAsDatasource component (app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/components/StoreAsDatasource.tsx) to display button text. The change from "Save" to "Save URL" is appropriate in this context since it's specifically used for saving URLs as datasources.
The text change is contextually appropriate and safe to proceed with
The constant is only used in the specific context of saving URLs as datasources, making the text change from "Save" to "Save URL" more descriptive and accurate for its usage. The change improves UX by making the action more explicit.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of SAVE_DATASOURCE to ensure the text change is appropriate in all contexts
rg "SAVE_DATASOURCE\(\)" -A 2
Length of output: 31
Script:
#!/bin/bash
# Let's try a more comprehensive search for SAVE_DATASOURCE
rg "SAVE_DATASOURCE" -A 2
# Also search for potential imports of this constant
rg "import.*SAVE_DATASOURCE" -A 2
# Search for any file that might be using this constant through messages import
rg "from ['\"].*messages['\"]" -A 2
Length of output: 150555
|
Deploy-Preview-URL: https://ce-37302.dp.appsmith.com |
Description
Updating the UI for save and edit datasource button in API editor
Fixes #37286
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/11741487860
Commit: 555d2c2
Cypress dashboard.
Tags:
@tag.Sanity, @tag.DatasourceSpec:
Fri, 08 Nov 2024 13:49:22 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation