Skip to content

chore: add ADS slider#38971

Merged
KelvinOm merged 3 commits intoreleasefrom
chore/ads-slider
Feb 4, 2025
Merged

chore: add ADS slider#38971
KelvinOm merged 3 commits intoreleasefrom
chore/ads-slider

Conversation

@KelvinOm
Copy link
Collaborator

@KelvinOm KelvinOm commented Feb 3, 2025

Description

Add ADS slider.
Also contains CE code for user RAG controls PR.

Automation

/ok-to-test tags="@tag.Sanity"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13119445734
Commit: becea81
Cypress dashboard.
Tags: @tag.Sanity
Spec:


Mon, 03 Feb 2025 18:20:56 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Introduced a new slider component with enhanced interactivity and accessibility.
    • Expanded form controls to include Rag Integrations for streamlined file and AI query setups.
  • Documentation

    • Added interactive examples and detailed guidance to help users leverage the new slider functionality.
  • Refactor

    • Consolidated Rag controls by replacing previous document settings with Rag Integrations.
    • Adjusted datasource update behavior for a smoother experience with AI-driven configurations.
  • Bug Fixes

    • Updated the handling of control types in the DatasourceFormRenderer to ensure correct rendering of components.

@KelvinOm KelvinOm requested a review from ayushpahwa as a code owner February 3, 2025 15:58
@KelvinOm KelvinOm added the ok-to-test Required label for CI label Feb 3, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2025

Walkthrough

This pull request introduces a new slider component to the design system by adding its dependency, constants, styles, documentation, stories, and main component implementation. It also refines the control flow in the data source update saga to include an Anvil integration check. Additionally, the RAG functionality is updated system-wide by deprecating the legacy RagDocuments and replacing it with RagIntegrations, along with necessary registry, import, and configuration changes in forms and plugins.

Changes

File(s) Change Summary
app/client/.../design-system/ads/package.json
app/client/.../design-system/ads/src/Slider/*
Added new dependency (@react-aria/slider) and introduced slider-related files: constants, documentation (Slider.mdx), Storybook stories, styled-components, component implementation, types, and index exports.
app/client/.../ce/sagas/DatasourcesSagas.ts Updated updateDatasourceSaga to check if Anvil is enabled and to handle redirection based on the plugin type (APPSMITH_AI).
app/client/.../ee/components/formControls/Rag/*
app/client/.../pages/Editor/DataSourceEditor/DatasourceFormRenderer.tsx
app/client/.../utils/formControl/*
Replaced RagDocuments with RagIntegrations: created a new RagIntegrations component and interface, updated exports, adjusted import paths and conditional checks, and modified form control registry and type constants.
app/server/.../appsmithAiPlugin/src/main/resources/form.json Updated form configuration: added new properties for Rag Integrations and modified the Files field with a hidden property based on a feature flag.

Sequence Diagram(s)

sequenceDiagram
    participant Action as Redux Action
    participant Saga as updateDatasourceSaga
    participant Selector as AnvilSelector
    participant Plugin as Plugin Check
    participant Redirect as Redirect Logic

    Action->>Saga: Dispatch updateDatasourceSaga
    Saga->>Selector: Check if Anvil is enabled
    Selector-->>Saga: Return true/false
    Saga->>Plugin: Verify plugin type (APPSMITH_AI)
    Plugin-->>Saga: Return plugin status
    alt Anvil enabled & plugin type is APPSMITH_AI
      Saga->>Redirect: Skip redirection
    else
      Saga->>Redirect: Execute redirection
    end
Loading
sequenceDiagram
    participant User as End User
    participant Slider as Slider Component
    participant Aria as React Aria Hook
    participant State as Slider State

    User->>Slider: Interact (drag/click)
    Slider->>Aria: Process interaction
    Aria-->>Slider: Return updated value
    Slider->>State: Update slider state
    State-->>Slider: State updated
    Slider->>User: Render new slider position
Loading

Suggested labels

Enhancement, UI Improvement, Task, Widgets & Accelerators Pod, Design System Product

Suggested reviewers

  • ApekshaBhosale
  • ankitakinger

Poem

In lines of code where changes bloom,
A slider glides like a silver tune.
New integrations spark a brighter day,
With Anvil checks keeping redirections at bay.
Code and components dance in digital delight 🚀.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Feb 3, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx (1)

1-1: Consider using type alias instead of empty interface.

The empty interface can be replaced with a type alias for better TypeScript practices.

-interface RagIntegrationsProps {}
+type RagIntegrationsProps = Record<string, never>;
🧰 Tools
🪛 Biome (1.9.4)

[error] 1-1: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

app/client/packages/design-system/ads/src/Slider/Slider.tsx (1)

66-76: Rendering the label and output.

Using @ts-expect-error is understandable if there’s a type mismatch with the Text component. However, consider removing or refactoring the @ts-expect-error by adjusting the Text component’s type definition if feasible.

app/client/packages/design-system/ads/src/Slider/Slider.stories.tsx (1)

17-25: Consider adding more edge cases to the story.

While the basic story is well implemented, consider adding cases for:

  • Disabled state
  • Custom step sizes
  • Min/max boundary values
app/client/packages/design-system/ads/src/Slider/Slider.styles.tsx (2)

4-29: Consider adding transition animations.

The StyledSlider component looks good, but smooth transitions would enhance the user experience.

 export const StyledSlider = styled.div<{
   disabled?: boolean;
 }>`
   position: relative;
   display: flex;
   flex-direction: column;
   align-items: center;
   touch-action: none;
   width: 100%;
   padding: 0 calc(var(--ads-v2-spaces-5) / 2) calc(var(--ads-v2-spaces-5) / 2);
+  transition: opacity 0.2s ease-in-out;

38-55: Add hover state transition for thumb.

The thumb hover states would benefit from smooth transitions.

 export const Thumb = styled.div`
   position: absolute;
   transform: translateX(-50%);
   width: var(--ads-v2-spaces-5);
   height: var(--ads-v2-spaces-5);
   border-radius: 50%;
   box-sizing: border-box;
   background-color: var(--ads-v2-color-bg-brand-secondary);
   cursor: pointer;
   top: 0;
+  transition: background-color 0.2s ease-in-out;
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e59806 and 2452078.

⛔ Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (16)
  • app/client/packages/design-system/ads/package.json (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.constants.ts (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.mdx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.stories.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.styles.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.types.ts (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/index.ts (1 hunks)
  • app/client/src/ce/sagas/DatasourcesSagas.ts (3 hunks)
  • app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx (1 hunks)
  • app/client/src/ee/components/formControls/Rag/index.ts (1 hunks)
  • app/client/src/ee/components/formControls/RagDocuments/index.ts (0 hunks)
  • app/client/src/pages/Editor/DataSourceEditor/DatasourceFormRenderer.tsx (2 hunks)
  • app/client/src/utils/formControl/FormControlRegistry.tsx (2 hunks)
  • app/client/src/utils/formControl/formControlTypes.ts (1 hunks)
  • app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json (1 hunks)
💤 Files with no reviewable changes (1)
  • app/client/src/ee/components/formControls/RagDocuments/index.ts
✅ Files skipped from review due to trivial changes (3)
  • app/client/src/ee/components/formControls/Rag/index.ts
  • app/client/packages/design-system/ads/src/Slider/index.ts
  • app/client/packages/design-system/ads/src/Slider/Slider.mdx
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx

[error] 1-1: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

⏰ Context from checks skipped due to timeout of 90000ms (12)
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
  • GitHub Check: server-unit-tests / server-unit-tests
  • GitHub Check: client-build / client-build
  • GitHub Check: client-prettier / prettier-check
  • GitHub Check: server-spotless / spotless-check
  • GitHub Check: chromatic-deployment
  • GitHub Check: chromatic-deployment
  • GitHub Check: storybook-tests
  • GitHub Check: perform-test / client-build / client-build
  • GitHub Check: perform-test / rts-build / build
  • GitHub Check: perform-test / server-build / server-unit-tests
🔇 Additional comments (21)
app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx (1)

5-7: Implementation needed for RagIntegrations component.

The component currently returns null, which suggests incomplete implementation. Please implement the component functionality or add a TODO comment explaining what needs to be implemented.

Would you like me to help create a basic implementation based on the RagDocuments component it's replacing?

app/client/src/utils/formControl/formControlTypes.ts (1)

22-22: LGTM!

The addition of RAG_INTEGRATIONS control type is consistent with the component changes.

app/client/src/utils/formControl/FormControlRegistry.tsx (1)

40-40: LGTM!

The registration of RAG_INTEGRATIONS control type follows the established pattern and correctly passes through all control props.

Also applies to: 194-201

app/client/src/ce/sagas/DatasourcesSagas.ts (3)

91-91: LGTM!

Import statement for the Anvil feature flag selector is correctly placed.


546-548: LGTM!

The selector is correctly used to check if Anvil is enabled in the current application.


679-685: Verify the plugin type check.

The condition has been updated to prevent redirection to view mode when Anvil is enabled and the plugin is APPSMITH_AI.

Run the following script to verify the plugin type usage:

✅ Verification successful

Plugin type check verified successfully.

  • The enum definition for APPSMITH_AI in app/client/src/entities/Plugin/index.ts confirms that it's a valid plugin type.
  • The conditional check in app/client/src/ce/sagas/DatasourcesSagas.ts correctly prevents redirection when Anvil is enabled and the plugin type is APPSMITH_AI.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that APPSMITH_AI is a valid plugin type.

# Test: Search for APPSMITH_AI plugin type definition
ast-grep --pattern $'enum PluginPackageName {
  $$$
  APPSMITH_AI = $_, 
  $$$
}'

Length of output: 1230

app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json (3)

48-53: LGTM!

The Files field visibility is correctly controlled by the release_anvil_enabled feature flag.


55-60: LGTM!

The Rag Integrations key property is correctly defined with appropriate hidden configuration.


61-155: Verify the chunk and overlap sizes for different integrations.

The configuration includes various integrations with their respective chunk and overlap sizes. Please ensure these values are optimal for each integration type.

  1. For LOCAL_FILES:
  • PDF: 1000/300 with OCR and page boundary
  • Other formats (TXT, MD, RTF, DOCX): 1000/300
  1. For other integrations (NOTION, ZENDESK, etc.):
  • All use 1500/450

The configuration looks consistent, but let's verify these values are tested and optimal:

✅ Verification successful

Chunk and Overlap Size Configurations Verified
The JSON configuration in app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json shows that the LOCAL_FILES integration uses chunk/overlap values of 1000/300, with PDF specifically configured with OCR and page boundaries, while integrations like NOTION use 1500/450. These values match the intended settings outlined.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any existing chunk size configurations in the codebase

# Test: Look for chunk size configurations
rg -A 2 'chunkSize|overlapSize' --type ts --type tsx

Length of output: 82


Script:

#!/bin/bash
# Re-run the verification by searching within the JSON file for integration configurations

# Verify the LOCAL_FILES integration entry and its chunk/overlap settings
echo "LOCAL_FILES integration configuration:"
rg -C 5 '"id": "LOCAL_FILES"' app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json

# Verify the PDF file type settings within LOCAL_FILES (should show OCR and page boundary settings)
echo -e "\nPDF file type configuration within LOCAL_FILES:"
rg -C 5 '"extension": "PDF"' app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json

# Verify one of the other integrations (e.g., NOTION) settings
echo -e "\nNOTION integration configuration:"
rg -C 5 '"id": "NOTION"' app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json

Length of output: 1595

app/client/packages/design-system/ads/src/Slider/Slider.tsx (8)

1-19: Imports and type definitions look consistent.

All imported libraries and types for the slider functionality appear valid and correctly referenced. No issues spotted here.


21-25: Properly setting the default origin.

Falling back to props.minValue or 0 is a good safeguard for the fill start. Consider documenting any edge cases (e.g., if minValue is greater than maxValue) in future.


26-38: Single-value slider mapped to multi-value props.

The approach to convert single-value props into AriaSliderProps<number[]> is neat. This is an effective pattern to handle the library's multi-thumb interface with a single thumb.


39-46: Slider state initialization is clear.

useSliderState integration with the numberFormatter nicely addresses localization. The destructuring of groupProps, labelProps, etc., looks aligned with best practices.


47-55: Thumb configuration is consistent.

The references for trackRef and inputRef are set up correctly. The use of thumbProps with the index 0 is straightforward for a single thumb slider.


57-64: Custom value label logic is flexible.

Allowing callers to supply a custom getValueLabel function provides good extensibility.


78-99: Track and rail rendering logic is well executed.

Calculating the fill range from Math.min(value, origin) to Math.max(value, origin) nicely handles scenarios where the origin might be higher or lower than the current value. This design is robust for offset-based sliders.


66-101: Overall structure is clear and maintainable.

The compositional approach, using separate layout components (TrackContainer, Thumb, Rail, etc.), promotes maintainability and readability. Good job keeping the slider fully accessible.

app/client/packages/design-system/ads/src/Slider/Slider.constants.ts (1)

1-4: Consistent naming for class constants.

Defining SliderClassName and SliderFocusVisibleClassName this way ensures a coherent naming convention, making the slider’s styling more maintainable.

app/client/packages/design-system/ads/src/Slider/Slider.types.ts (1)

3-11: Interface design is concise.

SliderProps cleanly extends AriaSliderProps<number> while omitting orientation. The extra fields (origin, formatOptions, getValueLabel) are straightforward and well-documented, promoting clarity.

app/client/packages/design-system/ads/src/Slider/Slider.stories.tsx (1)

11-15: LGTM! Clean template implementation.

The template function correctly manages state and props passing.

app/client/packages/design-system/ads/package.json (1)

29-29: Verify React Aria version compatibility.

The added @react-aria/slider dependency uses a caret range. Ensure compatibility with other React Aria packages in the project.

✅ Verification successful

React Aria Versions Compatible

All the React Aria packages, including the newly added @react-aria/slider (^3.7.15), are aligned with major version 3. This consistency across the dependencies suggests compatibility without any immediate concerns.

  • All React Aria packages (checkbox, focus, link, radio, slider, switch, textfield) use version 3.
  • Caret ranges allow minor updates while preserving API compatibility.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for version mismatches in React Aria dependencies
echo "Checking React Aria package versions..."
jq '.dependencies | with_entries(select(.key | startswith("@react-aria")))' package.json

Length of output: 250


Script:

#!/bin/bash
FILE="app/client/packages/design-system/ads/package.json"
if [ -f "$FILE" ]; then
    echo "Checking React Aria package versions in $FILE..."
    jq '.dependencies | with_entries(select(.key | startswith("@react-aria")))' "$FILE"
else
    echo "Error: $FILE not found."
fi

Length of output: 691

import { Table } from "@appsmith/ads-old";
import type { FeatureFlags } from "ee/entities/FeatureFlag";
import { RagDocuments } from "ee/components/formControls/RagDocuments";
import { RagDocuments } from "ee/components/formControls/Rag";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix component inconsistency in conditional rendering.

The code checks for RAG_INTEGRATIONS control type but renders the RagDocuments component. This seems inconsistent with the transition to the new RagIntegrations component.

Apply this diff to fix the inconsistency:

-import { RagDocuments } from "ee/components/formControls/Rag";
+import { RagIntegrations } from "ee/components/formControls/Rag";

 if (controlType === "RAG_INTEGRATIONS") {
   return (
-    <RagDocuments
+    <RagIntegrations
       datasourceId={datasource.id}
       isDeletedAvailable={false}
       isImportDataAvailable={false}
       key={reactKey}
       workspaceId={datasource.workspaceId}
     />
   );
 }

Also applies to: 167-177

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
app/client/packages/design-system/ads/src/Slider/Slider.styles.tsx (3)

4-29: Consider enhancing disabled state handling.

The disabled state implementation looks good but could be more comprehensive for better accessibility.

Consider adding these improvements:

 export const StyledSlider = styled.div<{
   disabled?: boolean;
 }>`
   position: relative;
   display: flex;
   flex-direction: column;
   align-items: center;
   touch-action: none;
   width: 100%;
   padding: 0 calc(var(--ads-v2-spaces-5) / 2) calc(var(--ads-v2-spaces-5) / 2);

   ${({ disabled }) =>
     disabled &&
     `
       opacity: 0.6;
       cursor: not-allowed !important;
+      pointer-events: none;
+      user-select: none;
+      * {
+        pointer-events: none;
+      }
     `}

38-55: Add transitions for smoother state changes.

The Thumb component's interactive states would benefit from smooth transitions.

Add transition for background-color:

 export const Thumb = styled.div`
   position: absolute;
   transform: translateX(-50%);
   width: var(--ads-v2-spaces-5);
   height: var(--ads-v2-spaces-5);
   border-radius: 50%;
   box-sizing: border-box;
   background-color: var(--ads-v2-color-bg-brand-secondary);
   cursor: pointer;
   top: 0;
+  transition: background-color 200ms ease-in-out;

   &:hover {
     background-color: var(--ads-v2-color-bg-brand-secondary-emphasis);
   }

57-73: Document RTL support in styles.

The margin-inline-start usage indicates RTL support. Consider adding a comment for clarity.

Add documentation:

 export const Rail = styled.div`
   position: absolute;
   background-color: var(--ads-v2-color-bg-emphasis);
   height: var(--ads-v2-spaces-1);
   transform: translateY(-50%);
   width: calc(100% + var(--ads-v2-spaces-5));
+  /* Uses margin-inline-start for RTL support */
   margin-inline-start: calc(var(--ads-v2-spaces-5) / 2 * -1);
 `;
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2452078 and becea81.

⛔ Files ignored due to path filters (1)
  • app/client/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (16)
  • app/client/packages/design-system/ads/package.json (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.constants.ts (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.mdx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.stories.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.styles.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/Slider.types.ts (1 hunks)
  • app/client/packages/design-system/ads/src/Slider/index.ts (1 hunks)
  • app/client/src/ce/sagas/DatasourcesSagas.ts (3 hunks)
  • app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx (1 hunks)
  • app/client/src/ee/components/formControls/Rag/index.ts (1 hunks)
  • app/client/src/ee/components/formControls/RagDocuments/index.ts (0 hunks)
  • app/client/src/pages/Editor/DataSourceEditor/DatasourceFormRenderer.tsx (2 hunks)
  • app/client/src/utils/formControl/FormControlRegistry.tsx (2 hunks)
  • app/client/src/utils/formControl/formControlTypes.ts (1 hunks)
  • app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json (1 hunks)
💤 Files with no reviewable changes (1)
  • app/client/src/ee/components/formControls/RagDocuments/index.ts
🚧 Files skipped from review as they are similar to previous changes (12)
  • app/client/packages/design-system/ads/src/Slider/index.ts
  • app/client/packages/design-system/ads/package.json
  • app/client/packages/design-system/ads/src/Slider/Slider.mdx
  • app/client/packages/design-system/ads/src/Slider/Slider.constants.ts
  • app/client/src/ee/components/formControls/Rag/index.ts
  • app/client/packages/design-system/ads/src/Slider/Slider.types.ts
  • app/client/src/pages/Editor/DataSourceEditor/DatasourceFormRenderer.tsx
  • app/client/src/ce/sagas/DatasourcesSagas.ts
  • app/client/packages/design-system/ads/src/Slider/Slider.tsx
  • app/client/src/utils/formControl/formControlTypes.ts
  • app/client/packages/design-system/ads/src/Slider/Slider.stories.tsx
  • app/client/src/utils/formControl/FormControlRegistry.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx

[error] 1-1: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

⏰ Context from checks skipped due to timeout of 90000ms (13)
  • GitHub Check: perform-test / client-build / client-build
  • GitHub Check: perform-test / rts-build / build
  • GitHub Check: perform-test / server-build / server-unit-tests
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-unit-tests / client-unit-tests
  • GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
  • GitHub Check: client-build / client-build
  • GitHub Check: server-unit-tests / server-unit-tests
  • GitHub Check: client-prettier / prettier-check
  • GitHub Check: server-spotless / spotless-check
  • GitHub Check: chromatic-deployment
  • GitHub Check: storybook-tests
  • GitHub Check: chromatic-deployment
🔇 Additional comments (6)
app/client/src/ee/components/formControls/Rag/RagIntegrations.tsx (1)

3-7: Implement the component's UI elements.

The current implementation is a placeholder. Since this is a form control component, it should render appropriate UI elements.

Would you like me to help implement the basic form control structure? This could include:

  • Form field wrapper
  • Label rendering
  • Input elements
  • Error handling
app/server/appsmith-plugins/appsmithAiPlugin/src/main/resources/form.json (3)

48-53: Dynamic Hidden Configuration for Files Picker

The "hidden" property is now defined as an object with a feature flag condition. This improves flexibility by letting the UI dynamically hide the Files field based on the release_anvil_enabled flag. Please verify that the condition (i.e. hiding when the flag is true) aligns with the intended UX for file uploads.


55-60: Addition of Rag Integrations Label Field

A new property is added to label the Rag Integrations control. Notably, its "hidden" attribute is statically set to true. Confirm if this field should remain permanently hidden or if it should also follow a feature flag condition. Consistency in hiding behavior across related fields might improve clarity.


61-156: Comprehensive Configuration for Rag Integrations

The new Rag Integrations component is introduced with a detailed configuration. The integration object for "LOCAL_FILES" includes specific settings such as the allowed file types, chunkSize, and overlapSize. In addition, various external integration options (e.g., NOTION, ZENDESK, etc.) are clearly defined. The dynamic "hidden" property on this block leverages the release_anvil_enabled feature flag in a way that appears to complement the Files control.

A couple of points to consider:

  • Maintainability: If chunkSize and overlapSize values remain constant across integrations, centralizing these values or documenting them in a shared configuration could improve maintainability.
  • UX Consistency: Double-check that the visibility behavior (i.e. "value": false) on the Rag Integrations block meets the intended form logic when the feature flag is active.
app/client/packages/design-system/ads/src/Slider/Slider.styles.tsx (2)

31-36: LGTM!

The SliderLabel component correctly implements the layout with proper spacing alignment.


75-85: LGTM!

The TrackContainer and Track components are correctly implemented with proper positioning.

@@ -0,0 +1,7 @@
interface RagIntegrationsProps {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Define interface properties for form control functionality.

The empty interface should be extended with necessary form control properties. Consider extending from a base form control interface if available.

Example structure:

interface RagIntegrationsProps {
  name: string;
  label?: string;
  // Add other form control properties
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 1-1: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

@KelvinOm KelvinOm merged commit 42988bb into release Feb 4, 2025
53 checks passed
@KelvinOm KelvinOm deleted the chore/ads-slider branch February 4, 2025 06:42
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Feb 7, 2025
## Description
Add ADS slider.
[Also contains CE code for user RAG controls
PR](appsmithorg/appsmith-ee#6025).

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 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/13119445734>
> Commit: becea81
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13119445734&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Mon, 03 Feb 2025 18:20:56 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**
- Introduced a new slider component with enhanced interactivity and
accessibility.
- Expanded form controls to include Rag Integrations for streamlined
file and AI query setups.

- **Documentation**
- Added interactive examples and detailed guidance to help users
leverage the new slider functionality.

- **Refactor**
- Consolidated Rag controls by replacing previous document settings with
Rag Integrations.
- Adjusted datasource update behavior for a smoother experience with
AI-driven configurations.
  
- **Bug Fixes**
- Updated the handling of control types in the DatasourceFormRenderer to
ensure correct rendering of components.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Feb 7, 2025
## Description
Add ADS slider.
[Also contains CE code for user RAG controls
PR](appsmithorg/appsmith-ee#6025).

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 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/13119445734>
> Commit: becea81
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13119445734&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Mon, 03 Feb 2025 18:20:56 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**
- Introduced a new slider component with enhanced interactivity and
accessibility.
- Expanded form controls to include Rag Integrations for streamlined
file and AI query setups.

- **Documentation**
- Added interactive examples and detailed guidance to help users
leverage the new slider functionality.

- **Refactor**
- Consolidated Rag controls by replacing previous document settings with
Rag Integrations.
- Adjusted datasource update behavior for a smoother experience with
AI-driven configurations.
  
- **Bug Fixes**
- Updated the handling of control types in the DatasourceFormRenderer to
ensure correct rendering of components.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants