chore: Move smaller Entity Explorer components into ADS #38321
Conversation
WalkthroughThis pull request introduces several new components and utilities in the design system and editor pane, focusing on enhancing the user interface for empty states and explorer containers. The changes include creating reusable components like Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
hetunandu
requested review from
ankitakinger,
alex-golovanov and
albinAppsmith
github-actions
bot
added
the
skip-changelog
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12466731548. |
![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
🧹 Nitpick comments (9)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.types.ts (1)
1-13: Consider Optional Fields for a More Flexible API
The current design is solid. If future requirements arise, consider making fields like “icon” optional and defaulting to a generic icon.app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.tsx (1)
7-13: Encourage translatable string usage
For future internationalization, consider using a translation hook or utility for the displayed text.app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.stories.tsx (1)
1-1: Console warnings disabled
Disabling console warnings is fine for Storybook demos, but remember to keep them enabled in production.app/client/src/pages/Editor/IDE/EditorPane/index.tsx (1)
27-27: Clarity in width attribute
Switching from brace syntax to a direct string for width is a minor but clean improvement.app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.stories.tsx (1)
35-44: Consider using safe logging or a Storybook addon.
Currently, console logs are acceptable but can be replaced with an action logger in Storybook for better traceability.- onClick: () => console.log("Add clicked"), + onClick: () => console.log("Add clicked"), // or actions('Add clicked') from '@storybook/addon-actions'app/client/src/pages/Editor/IDE/EditorPane/JS/BlankState.tsx (1)
21-29: Using useMemo for button props is a neat optimization if re-render performance is a concern.
However, if there's no heavy logic involved, you can safely remove it to minimize complexity, unless you expect frequent re-renders.- const buttonProps = useMemo( - () => ({ - className: "t--add-item", - testId: "t--add-item", - text: createMessage(EDITOR_PANE_TEXTS.js_add_button), - onClick: canCreateActions ? openAddJS : undefined, - }), - [canCreateActions, openAddJS], - ); + const buttonProps = { + className: "t--add-item", + testId: "t--add-item", + text: createMessage(EDITOR_PANE_TEXTS.js_add_button), + onClick: canCreateActions ? openAddJS : undefined, + };app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx (1)
33-33: Use optional chaining for concise checks.
Static analysis suggests using an optional chain. Adjusting code to rely on optional chaining can make it more elegant.- {button && button.onClick ? ( + {button?.onClick ? ( <Button ... > {button.text} </Button> ) : null}🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx (1)
29-33: Minor note on border prop usage.
Using string values ("NONE", "STANDARD") helps maintain code clarity, but consider extracting them into a shared Typescript Enum for better discoverability and type safety in the future.app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.tsx (1)
97-97: Minor whitespace observation.
Ensure consistent spacing around function blocks for readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.mdx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.stories.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.types.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/index.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.constants.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.stories.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.types.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/index.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.mdx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.stories.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.types.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/index.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/index.ts(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx(3 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/Add.tsx(2 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/BlankState.tsx(3 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx(2 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/Add.tsx(2 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/BlankState.tsx(2 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx(2 hunks)app/client/src/pages/Editor/IDE/EditorPane/UI/List.tsx(4 hunks)app/client/src/pages/Editor/IDE/EditorPane/components/EmptySearchResult.tsx(0 hunks)app/client/src/pages/Editor/IDE/EditorPane/components/EmptyState.tsx(0 hunks)app/client/src/pages/Editor/IDE/EditorPane/index.tsx(3 hunks)app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.tsx(4 hunks)
💤 Files with no reviewable changes (2)
- app/client/src/pages/Editor/IDE/EditorPane/components/EmptyState.tsx
- app/client/src/pages/Editor/IDE/EditorPane/components/EmptySearchResult.tsx
✅ Files skipped from review due to trivial changes (5)
- app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.constants.ts
- app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/index.ts
- app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/index.ts
- app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.mdx
- app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/pages/Editor/IDE/EditorPane/UI/List.tsx
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (65)
app/client/src/pages/Editor/IDE/EditorPane/Query/BlankState.tsx (4)
1-1: Nice usage of useMemo for performance optimization
By selectively importing and using React hooks, we reduce unnecessary re-renders.
9-9: Correct import path alignment
Referencing EmptyState from @appsmith/ads correctly centralizes and standardizes design components.
21-29: Good approach for encapsulating button properties
Using useMemo to define buttonProps helps avoid recreating object literals on every render, enhancing performance and maintainability.
33-33: Appropriate usage of the button prop
Passing the computed buttonProps object to the EmptyState component keeps the code clean and consistent.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.types.ts (1)
1-3: Straightforward Interface Definition
This interface is minimal and sufficiently clear for usage.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/index.ts (1)
5-7: All Exports are Appropriately Located
Export statements are concise and organized. No issues found.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.types.ts (1)
1-10: Well-Structured Props
The interface is clearly defined, ensuring type safety for border style usage. Looks good.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.tsx (3)
2-2: Imported component usage is clear and consistent
The import from "../../.." for the Text component is good practice within a shared design system.
5-6: Minimal and effective functional component
The component signature is succinct and properly types its props.
17-17: Export statement
Exporting the component at the end is a straightforward approach that promotes clarity.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.tsx (2)
2-2: Centralized constants
Importing ExplorerContainerBorder ensures consistent styling across the application. Nicely done.
5-18: Well-structured container component
The use of Flex for layout and the typed props for border, width, and height make the component highly reusable.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/NoSearchResults/NoSearchResults.stories.tsx (3)
5-5: Component import is consistent
The import statement follows a clear, predictable structure for design system components.
14-18: Simple and effective Storybook template
This is straightforward, allowing testers to validate different text content easily.
20-24: Basic story with sensible default
Providing a default text prop value helps others quickly grasp the component's purpose.
app/client/src/pages/Editor/IDE/EditorPane/index.tsx (2)
2-2: Border constants usage
Using ExplorerContainerBorder here increases maintainability and consistency across the IDE.
17-18: Semantic condition for border
Defining the standard vs. none border in a conditional expression is clear, avoiding magic strings.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.stories.tsx (2)
1-1: Good use of ESLint disable for console logs in Storybook.
This is acceptable for a Storybook environment where console output helps demonstrate interactions.
14-26: General structure and usage look solid.
The props interface is clearly used, and the minimal code in the template ensures readability. Good job!
app/client/packages/design-system/ads/src/Templates/EntityExplorer/ExplorerContainer/ExplorerContainer.stories.tsx (4)
1-1: ESLint rule disable for console logs is acceptable for stories.
No issues here.
14-19: Good approach to managing borderRight with a select control.
This ensures an intuitive demonstration in Storybook for different layouts.
23-37: Template is concise and reusable.
The destructuring approach for props keeps it clear. Nothing further to address here.
39-54: Smart usage of children for better composition.
No immediate refactoring suggestions. Nicely done.
app/client/src/pages/Editor/IDE/EditorPane/JS/BlankState.tsx (1)
33-33: Good usage of the new button prop in EmptyState.
This is consistent with the new approach across the codebase; no additional suggestions here.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx (1)
5-46: Overall structure is well-organized.
The component properly handles optional props and gracefully renders a button when available.
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx (2)
2-2: Good adoption of the new container.
Switching from a standard layout component to the specialized ExplorerContainer is a neat step towards consistency.
60-60: ExplorerContainer usage looks good.
No issues noted in the container’s closing tag or children structure.
app/client/src/pages/Editor/IDE/EditorPane/Query/Add.tsx (2)
2-7: Import statements for new UI components.
These imports look correct for the new design system items. No conflicts or mistakes noted here.
72-76: Enhanced user feedback for empty results.
Replacing the old component with NoSearchResults is a good step to unify the design pattern. The dynamic text prop usage correctly supplies relevant messages.
app/client/src/pages/Editor/IDE/EditorPane/JS/Add.tsx (2)
5-5: Import statements correctly updated.
Switching to NoSearchResults aligns with the new design approach.
100-104: Meaningful error message for empty search results.
The new component helps provide better feedback when a user can’t find a JS object. The code is clear and matches the usage pattern seen elsewhere.
app/client/src/pages/Editor/IDE/EditorPane/UI/List.tsx (6)
15-15: Updated import for EmptyState.
Adopting the new design system component for blank states is consistent with the PR’s objective.
29-29: Destructuring props.
Directly destructuring setFocusSearchInput is a stylistic improvement for readability.
45-47: Function call referencing up-to-date prop.
setFocusSearchInput is invoked before redirecting, ensuring the search input is focused after navigation.
52-58: Clear effect to reset the focus flag.
The inline named function resetFocusOnSearch is more readable and explicit. Good call.
59-67: Memoization for button props.
Extracting blankStateButtonProps ensures stable references and avoids unnecessary re-renders.
79-79: Consistent styling for EmptyState.
Providing a custom description and icon with the cohesive design system button makes the empty state more engaging.
app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx (2)
2-2: Good addition of NoSearchResults component.
Imports look consistent with the new ADS library usage.
97-101: Clear usage of NoSearchResults for handling empty states.
The updated component seamlessly replaces the old one, matching the existing design patterns.
app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (2)
3-3: Consistent import of NoSearchResults.
This keeps the UI aligned with other modules.
113-117: Appropriate conditional rendering for search results.
The newly employed NoSearchResults component is well integrated here.
app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.tsx (6)
1-1: Smart usage of hooks.
Adding useMemo and useCallback enhances performance on re-renders.
30-30: Migration to the EmptyState from ADS.
Good step towards consistent design across the application.
89-89: Well-defined callback.
This is straightforward, improving clarity on how datasources are handled.
96-96: Economic reliance on basePageId in dependency array.
Good practice to avoid re-creating the callback unnecessarily.
98-106: Structured props for the EmptyState button.
Use of useMemo ensures stable references and prevents excessive re-renders.
126-126: Seamless integration of the button props into EmptyState.
Enhances user engagement by guiding them to add new datasources.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.mdx (18)
1-1: Proper Storybook imports.
This ensures consistent component documentation.
3-3: Adherence to single source of truth for stories.
Promotes centralized and reusable story definitions.
5-5: Meta usage is correct.
It binds the documentation to the relevant stories.
7-7: Clear top-level heading.
A concise title helps maintain the documentation’s structure.
9-9: Straightforward description.
Users gain immediate clarity on Empty State usage.
11-11: Logical organization with sub-headings.
Easy to navigate for developers.
13-13: Relevant mention of the icon element.
Helps define critical UI elements succinctly.
15-15: Short descriptor of message usage.
Keeps interactions minimal yet informative.
17-17: Optional button note.
Clear mention that the button is not mandatory.
19-19: Subheading for default usage.
Effectively separates different component states.
21-21: Docs referencing default usage example.
Good demonstration for new adopters.
23-23: Canvas usage for Basic story.
Ensures the actual component is previewed.
25-25: Heading for variations with button.
Organized approach to highlight advanced usage.
27-27: Explanation of button props.
Integrates seamlessly with component usage details.
29-29: Canvas rendering for WithButton.
Exemplifies usage in real scenarios.
31-31: Subsection exploring button without onClick.
Addresses possible usage scenarios thoroughly.
33-33: Optional nature of onClick is clarified.
Misdirected usage is avoided by clarifying default hide.
35-35: Canvas demonstration for WithButtonWithoutOnClick.
Effective for crossover or partial usage.
|
Deploy-Preview-URL: https://ce-38321.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12477172807. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.types.ts (1)
1-1: Consider using more specific importsThe relative import path
../../..is not very maintainable. Consider using a more specific path or setting up path aliases.app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx (2)
5-46: Component structure looks good with a suggestion for improvementThe component is well-structured and follows design system patterns. However, the button rendering condition can be simplified using optional chaining.
Consider this improvement:
- {button && button.onClick ? ( + {button?.onClick ? (🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
34-42: Consider adding aria-label for accessibilityThe button implementation looks good, but consider adding an aria-label for better accessibility.
<Button className={button.className} data-testid={button.testId} kind={button.kind || "secondary"} onClick={button.onClick} size="sm" + aria-label={`${description} action button`} >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.types.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx
[error] 33-33: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.types.ts (1)
3-13: LGTM! Well-structured interface definition
The interface is well-defined with proper types and optional properties. Good job on restricting the ButtonKind to specific variants.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EmptyState/EmptyState.tsx (1)
7-25: Excellent use of design system tokens
Good job using design system variables for spacing, colors, and border radius. The icon container's styling is consistent with the design system.
|
Deploy-Preview-URL: https://ce-38321.dp.appsmith.com |
Description
Moves the following Entity Explorer components into ADS/Templates/EntityExplorer
Fixes #38281
Fixes #37609
Automation
/ok-to-test tags="@tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12477432425
Commit: 41d4904
Cypress dashboard.
Tags:
@tag.IDESpec:
Tue, 24 Dec 2024 06:00:54 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
EmptyStatecomponent as a placeholder for scenarios with no data.NoSearchResultscomponent for improved user feedback when searches yield no results.ExplorerContainerfor enhanced layout management in the Explorer section.Bug Fixes
Documentation
EmptyStateandNoSearchResultscomponents, providing usage guidelines.Chores
EmptySearchResultandEmptyState) to streamline the codebase.