chore: Update Entity Tree implementation for better composition#39488
chore: Update Entity Tree implementation for better composition#39488
Conversation
WalkthroughThe pull request introduces a customizable item rendering mechanism for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (8)
✨ Finishing Touches
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
|
hetunandu
requested review from
alex-golovanov and
ankitakinger
and removed request for
a team
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.tsx(3 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.types.ts(1 hunks)app/client/src/IDE/Components/EditableName/useValidateEntityName.ts(1 hunks)app/client/src/pages/AppIDE/components/UIEntityListTree/UIEntityListTree.tsx(1 hunks)app/client/src/pages/AppIDE/components/UIEntityListTree/WidgetTreeItem.tsx(1 hunks)app/client/src/pages/AppIDE/components/UIEntityListTree/hooks/useSwitchToWidget.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- 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
- GitHub Check: chromatic-deployment
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
🔇 Additional comments (19)
app/client/src/pages/AppIDE/components/UIEntityListTree/hooks/useSwitchToWidget.ts (2)
9-9: Clean import addition for type safety.The addition of the WidgetType import improves type safety by providing explicit typing for the widget parameter.
17-20: Improved parameter typing enhances API clarity.The parameter type has been updated from a generic
CanvasStructureto a more specific object type with exactly the properties needed by the function. This makes the API requirements more explicit and removes potential dependencies on unused properties.This change aligns well with the composability improvements mentioned in the PR objectives, as it allows for more flexible component integration by clearly defining the minimal interface required.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.tsx (4)
13-13: Good addition of ItemComponent parameter for improved composability.This change allows the EntityListTree to accept a custom component for rendering items, making it more flexible and reusable.
53-53: Prefix added to data-testid attribute.The data-testid attribute has been updated with a "t--" prefix, which is likely for test selector consistency.
66-66: Improved component composition with ItemComponent.The direct rendering of a specific component has been replaced with the provided ItemComponent prop, enabling better component composition patterns.
70-70: Properly propagating ItemComponent to nested tree instances.Good practice to pass the ItemComponent to nested EntityListTree instances, ensuring consistent rendering throughout the tree.
app/client/src/pages/AppIDE/components/UIEntityListTree/UIEntityListTree.tsx (3)
1-8: Well-executed simplification of imports.The component now has focused imports, removing unnecessary dependencies that have been moved to the WidgetTreeItem component.
16-20: Simplified tree item configuration.The tree item configuration has been streamlined to only include essential properties needed for rendering, with the rest moved to the WidgetTreeItem component.
22-28: Clean implementation of component composition pattern.The component now leverages the new ItemComponent prop from EntityListTree, using WidgetTreeItem as the renderer. This is a good application of component composition.
app/client/src/pages/AppIDE/components/UIEntityListTree/WidgetTreeItem.tsx (5)
1-15: Well-structured imports for the new component.The imports are organized logically, bringing in the necessary dependencies for rendering, state management, and user interactions.
16-37: Good implementation of widget-specific functionality.The component properly retrieves widget data and sets up the necessary state and callbacks for managing widget names and interactions.
38-48: Proper name validation implementation.Good use of the validateName hook with the appropriate parameters, including the widget name and ID.
50-68: Well-organized memoization of nameEditorConfig.The nameEditorConfig is properly memoized with the correct dependencies to prevent unnecessary re-renders.
69-106: Comprehensive item rendering setup.The component includes all necessary callbacks and UI elements for a fully functional widget tree item, including click handlers, icons, and context menus.
app/client/src/IDE/Components/EditableName/useValidateEntityName.ts (4)
12-15: Good type definition improvements.Making entityName required and adding optional entityId enhances the type safety and functionality of the validation hook.
22-26: Improved prop destructuring with defaults.The destructuring now properly handles the entityId with a sensible default value.
28-31: Enhanced selector usage with entityId.The selector now receives the entityId parameter, allowing for more accurate validation against used entity names.
33-42: Simplified validation logic.The validation callback has been streamlined by removing the redundant oldName parameter and directly using entityName for comparison.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.types.ts (1)
12-12: New ItemComponent prop enables better composition.Adding
ItemComponentas a required prop enables better component composition and customization of item rendering. This follows React best practices by delegating rendering responsibility to the parent component rather than hard-coding it.This change aligns well with the PR's objective of "better composition" and improves the component's flexibility. The typing ensures type safety when implementing custom renderers.
...ckages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.types.ts
Show resolved
Hide resolved
…h into fix/app-ide-ui-tree
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.stories.tsx (1)
133-133: Simplify boolean expressionThe
Booleancall is redundant since negation already coerces values to boolean.- setExpanded((prev) => ({ ...prev, [id]: !Boolean(prev[id]) })); + setExpanded((prev) => ({ ...prev, [id]: !prev[id] }));🧰 Tools
🪛 Biome (1.9.4)
[error] 133-133: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.stories.tsx(4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.stories.tsx
[error] 133-133: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
- GitHub Check: chromatic-deployment
🔇 Additional comments (3)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityListTree/EntityListTree.stories.tsx (3)
31-38: Good addition of centralized naming mapThis approach separates data from presentation by mapping IDs to display names, making the code more maintainable.
90-117: Well-structured component extractionThe
EntityItemComponenteffectively encapsulates the item rendering logic and state management. This demonstrates the composability pattern being implemented in this PR.
146-150: Good implementation of the composability patternThe updated
EntityListTreeusage properly demonstrates how to use the newItemComponentprop, which is the main objective of this PR.
github-actions
bot
added
IDE Pod
2 participants
Description
Make EntityListTree component accept a ItemRender component instead of rendering the EntityItem component itself to improve composibility
Fixes #39491
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/13612177606
Commit: 5d915d1
Cypress dashboard.
Tags:
@tag.IDESpec:
Sun, 02 Mar 2025 05:19:40 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Refactor