chore: Add datasource link control#39841
Conversation
github-actions
bot
added
the
skip-changelog
WalkthroughThis PR introduces updates to several form control components. Inline styles from Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as DatasourceLinkControl
participant P as useParentEntityInfo
participant H as History
U->>D: Click button
D->>P: Retrieve parentEntityId
P-->>D: Return parentEntityId
D->>D: Construct URL using datasourcesEditorIdURL
D->>H: history.push(URL)
H-->>U: Navigate to new page
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (1)
62-62: Consider using a safer approach for width stylingThe
UNSAFE_widthprop suggests this is using a non-standard or potentially deprecated approach for setting the button width. While it works, consider using styled-components or standard CSS classes for more maintainable styling.- UNSAFE_width="110px" + // Use styled extension or CSS class instead + UNSAFE_width="110px"app/client/src/components/formControls/DatasourceLinkControl.tsx (4)
14-20: Prop 'href' is defined but never used.The interface defines a
hrefproperty, but it's not being used in the component implementation. If it's not needed, consider removing it to keep the interface clean and avoid confusion.export interface DatasourceLinkControlProps extends ControlProps { - href: string; text: string; size?: ButtonProps["size"]; kind?: ButtonProps["kind"]; icon?: ButtonProps["startIcon"]; }
39-40: Add type validation for datasourceId.The
datasourceIdis cast as string without checking if it exists. Add proper type validation to prevent potential runtime errors.- datasourceId: props.datasourceId as string, + datasourceId: typeof props.datasourceId === 'string' ? props.datasourceId : '',Also, consider adding this property to the
DatasourceLinkControlPropsinterface for better type safety.
48-48: Consider making button width configurable.The hardcoded width of "110px" may not be suitable for all use cases, especially with different text lengths or in different languages.
- UNSAFE_width="110px" + UNSAFE_width={props.width || "110px"}And update the interface:
export interface DatasourceLinkControlProps extends ControlProps { // other properties width?: string; }
31-57: Add error handling for missing or invalid datasourceId.The component doesn't handle cases where
datasourceIdmight be missing or invalid. Consider adding appropriate error handling to improve robustness.function DatasourceLink(props: DatasourceLinkControlProps) { const { icon, kind = "secondary", size = "sm", text } = props; const ideType = getIDETypeByUrl(location.pathname); const { parentEntityId } = useParentEntityInfo(ideType); + // Early return if datasourceId is missing + if (!props.datasourceId) { + return ( + <Button + UNSAFE_width="110px" + kind={kind} + size={size} + startIcon={icon} + disabled={true} + > + {text} + </Button> + ); + } const onPress = useCallback(() => { const url = datasourcesEditorIdURL({ baseParentEntityId: parentEntityId, datasourceId: props.datasourceId as string, params: { ...omit(getQueryParams(), "viewMode"), viewMode: false }, }); history.push(url); }, [parentEntityId, props.datasourceId]); // Rest of the component... }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx(2 hunks)app/client/src/components/formControls/DatasourceLinkControl.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx(1 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx(2 hunks)app/client/src/utils/formControl/formControlTypes.ts(1 hunks)
🔇 Additional comments (3)
app/client/src/utils/formControl/formControlTypes.ts (1)
27-27: Added new control type for datasource linkingThe addition of
DATASOURCE_LINKconstant follows the established pattern in this file. This will enable the use of a new form control type for datasource navigation.app/client/src/utils/formControl/FormControlRegistry.tsx (2)
50-53: Good import structure for the new componentImport structure follows TypeScript best practices by co-locating component and type imports.
247-256: Correctly implemented registration for new control typeThe registration for the
DATASOURCE_LINKcontrol follows the established pattern in the registry. The implementation provides proper typing for the control props.
...lient/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx
Outdated
Show resolved
Hide resolved
...lient/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx
Show resolved
Hide resolved
app/client/src/components/formControls/DatasourceLinkControl.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx (1)
62-62: Consider using a design system token instead of hardcoded widthThe
UNSAFE_width="110px"property sets a fixed width for the button, but using hardcoded values can lead to inconsistencies in the UI. Consider using a design token for width if available in your design system.- UNSAFE_width="110px" + UNSAFE_width="var(--ads-v2-button-width-medium)" // or appropriate design tokenapp/client/src/components/formControls/DatasourceLinkControl.tsx (4)
14-20: Interface contains an unused property.The
hrefproperty defined inDatasourceLinkControlPropsis never used in the component implementation. Navigation is handled via theonPresscallback usingdatasourceIdanddatasourcesEditorIdURL.export interface DatasourceLinkControlProps extends ControlProps { - href: string; text: string; size?: ButtonProps["size"]; kind?: ButtonProps["kind"]; icon?: ButtonProps["startIcon"]; }
39-40: Add type checking for datasourceId.The
datasourceIdis cast as string without verifying its existence. Consider adding validation to prevent runtime errors.- datasourceId: props.datasourceId as string, + datasourceId: props.datasourceId ? String(props.datasourceId) : "", params: { ...omit(getQueryParams(), "viewMode"), viewMode: false },
48-48: Fixed width may cause UI issues with different text lengths.Using a fixed width of "110px" might truncate longer button text or cause inconsistent spacing with shorter text. Consider using auto width or a more flexible approach.
- UNSAFE_width="110px" + UNSAFE_width="auto"
36-44: Add error handling for navigation.The
onPresscallback doesn't handle cases whereparentEntityIdordatasourceIdmight be undefined or invalid. Consider adding error handling to prevent unexpected navigation behavior.const onPress = useCallback(() => { + if (!parentEntityId || !props.datasourceId) { + // Consider showing a notification or handling the error + return; + } const url = datasourcesEditorIdURL({ baseParentEntityId: parentEntityId, datasourceId: props.datasourceId as string, params: { ...omit(getQueryParams(), "viewMode"), viewMode: false }, }); history.push(url); }, [parentEntityId, props.datasourceId]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx(2 hunks)app/client/src/components/formControls/DatasourceLinkControl.tsx(1 hunks)app/client/src/components/formControls/FunctionCallingConfigControl/components/FunctionCallingConfigForm.tsx(1 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx(2 hunks)app/client/src/utils/formControl/formControlTypes.ts(1 hunks)
🔇 Additional comments (5)
app/client/src/utils/formControl/formControlTypes.ts (1)
27-27: LGTM: Added new DATASOURCE_LINK constantThe addition of this constant follows the established pattern in the file and will be used to register the new control type.
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/FormRender.tsx (2)
186-186: LGTM: Added custom styling support to FieldWrapperProper application of custom styles from the section configuration to the FieldWrapper component.
225-229: LGTM: Added custom styling support to Zone componentGood approach using the spread operator to apply all custom styles from section.zoneCustomStyle to the Zone component.
app/client/src/utils/formControl/FormControlRegistry.tsx (2)
50-53: LGTM: Clean import of DatasourceLinkControlImport statement correctly brings in both the component and its type definition.
247-256: LGTM: Properly registered new DATASOURCE_LINK controlThe registration follows the established pattern used for other controls in the registry.
app/client/src/components/formControls/DatasourceLinkControl.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/client/src/components/formControls/DatasourceLinkControl.tsx (3)
14-20: Interface property defined but unusedThe
hrefproperty is defined in the interface but not used in the implementation. Consider removing it or implementing its usage.export interface DatasourceLinkControlProps extends ControlProps { - href: string; text: string; size?: ButtonProps["size"]; kind?: ButtonProps["kind"]; icon?: ButtonProps["startIcon"]; }
39-39: Add null check for datasourceIdThe component assumes
datasourceIdwill be available and casts it directly to string. Consider adding validation to handle cases where it might be undefined.- datasourceId: props.datasourceId as string, + datasourceId: props.datasourceId ? String(props.datasourceId) : "",
31-57: Consider improving component reusabilityThe component tightly couples the navigation logic with the button UI. Consider extracting the navigation logic to a custom hook to improve reusability and testability.
+ // In a separate hook file + export function useDatasourceNavigation(datasourceId: string | undefined) { + const ideType = getIDETypeByUrl(location.pathname); + const { parentEntityId } = useParentEntityInfo(ideType); + + return useCallback(() => { + if (!datasourceId) return; + + const url = datasourcesEditorIdURL({ + baseParentEntityId: parentEntityId, + datasourceId: String(datasourceId), + params: { ...omit(getQueryParams(), "viewMode"), viewMode: false }, + }); + + history.push(url); + }, [parentEntityId, datasourceId]); + } // In this component function DatasourceLink(props: DatasourceLinkControlProps) { const { icon, kind = "secondary", size = "sm", text } = props; - const ideType = getIDETypeByUrl(location.pathname); - const { parentEntityId } = useParentEntityInfo(ideType); - - const onPress = useCallback(() => { - const url = datasourcesEditorIdURL({ - baseParentEntityId: parentEntityId, - datasourceId: props.datasourceId as string, - params: { ...omit(getQueryParams(), "viewMode"), viewMode: false }, - }); - - history.push(url); - }, [parentEntityId, props.datasourceId]); + const onPress = useDatasourceNavigation(props.datasourceId); return ( // ... ); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/components/formControls/DatasourceLinkControl.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (1)
app/client/src/components/formControls/DatasourceLinkControl.tsx (1)
22-29: Well-structured control componentGood implementation of the control component that follows the BaseControl pattern and properly encapsulates the rendering logic.
| export interface DatasourceLinkControlProps extends ControlProps { | ||
| href: string; | ||
| text: string; | ||
| size?: ButtonProps["size"]; | ||
| kind?: ButtonProps["kind"]; | ||
| icon?: ButtonProps["startIcon"]; | ||
| } |
There was a problem hiding this comment.
Missing property in interface definition
The datasourceId property is used in the component implementation but not defined in the DatasourceLinkControlProps interface.
export interface DatasourceLinkControlProps extends ControlProps {
href: string;
text: string;
size?: ButtonProps["size"];
kind?: ButtonProps["kind"];
icon?: ButtonProps["startIcon"];
+ datasourceId?: string;
}Also applies to: 39-39
|
|
||
| return ( | ||
| <Button | ||
| UNSAFE_width="110px" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid using UNSAFE_ prefixed props
The UNSAFE_width prop suggests it might be deprecated or not recommended for general use. Consider using a more standard approach for setting width.
🌐 Web query:
Is UNSAFE_width a deprecated property in Appsmith Button component?
💡 Result:
Based on the provided documentation and resources, there is no evidence that UNSAFE_width is a documented or supported property for Appsmith's Button component[1][10]. The term "UNSAFE_width" doesn't appear in any official Appsmith widget documentation or recent GitHub issues regarding button components.
For width control in Button widgets, Appsmith provides these official methods:
- Layout properties: Standard width configuration through property pane settings[1][12]
- Dynamic styling: Programmatic control using JavaScript via
setColor()method[1] - Theme alignment: Style buttons using theme properties (
appsmith.theme)[4] - CSS overrides: Custom widget styling through CSS-in-JS patterns[6][11]
The "UNSAFE_" prefix typically indicates internal/experimental React APIs, but Appsmith's component API doesn't use this pattern for its widget properties[1][2]. For width modifications, use the standard width property or CSS-based approaches through Custom widgets[6][11].
For reference, here are the supported width-related patterns for buttons:
| Approach | Implementation | Use Case |
|---|---|---|
| Property Pane | Direct width adjustment in widget settings | Basic layout control |
| JS Methods | setColor() combined with icon positioning[7] |
Dynamic styling |
| Custom CSS | Using Custom widget's CSS variables[6] | Brand-specific designs |
| Theme Sync | appsmith.theme properties for consistency[4] |
App-wide styling |
Citations:
- 1: https://docs.appsmith.com/reference/widgets/button
- 2: https://github.com/appsmithorg/appsmith/blob/release/contributions/AppsmithWidgetDevelopmentGuide.md
- 3: https://docs.appsmith.com/reference/widgets/menu
- 4: https://docs.appsmith.com/reference/appsmith-framework/context-object
- 5: [Feature]: deprecation warning on widgets #12291
- 6: https://docs.appsmith.com/reference/widgets/custom
- 7: https://docs.appsmith.com/reference/widgets/button-group/buttons
- 8: https://docs.appsmith.com/reference/widgets/button-group
- 9: https://www.appsmith.com/blog/appsmith-news-august
- 10: https://docs.appsmith.com/reference/widgets
- 11: https://community.appsmith.com/content/guide/iframe-experiments-extending-appsmith-custom-iframe-code
- 12: https://docs.appsmith.com/reference/widgets/form
- 13: https://community.appsmith.com/content/blog/recommended-appsmith-naming-conventions
- 14: [Bug]-[400]:Button and menu button widget properties and layout changes in table widget #10278
- 15: [Feature]-[800]:Deprecate form button widget #3613
Action: Remove UNSAFE_width Prop for Consistency
The use of the UNSAFE_width prop in DatasourceLinkControl.tsx (line 48) isn’t supported by official Appsmith documentation. Since it doesn’t appear to be a documented or reliable property, please remove it and use a standard width configuration (e.g., via CSS or the widget’s property pane).
- Remove
UNSAFE_width="110px"on line 48. - Adopt one of the documented methods for width control as recommended in the Appsmith documentation.
/ok-to-test tags="@tag.Sanity" This PR adds an datasource link control that is required to on query page of AI datasource. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new form control that allows users to navigate directly to datasource-related views. - Enhanced forms with dynamic styling capabilities, enabling sections to apply custom visual configurations. - **Style** - Adjusted the width of a key configuration button to improve layout consistency. - **Bug Fixes** - Resolved issues related to the application of inline styles in form components. <!-- 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/13986086739> > Commit: 456b285 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13986086739&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Fri, 21 Mar 2025 07:20:48 UTC <!-- end of auto-generated comment: Cypress test results -->
2 participants
/ok-to-test tags="@tag.Sanity"
This PR adds an datasource link control that is required to on query page of AI datasource.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
Bug Fixes
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13986086739
Commit: 456b285
Cypress dashboard.
Tags:
@tag.SanitySpec:
Fri, 21 Mar 2025 07:20:48 UTC