-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: move building blocks to bottom of widget explorer #35270
Conversation
WalkthroughThe recent code changes aim to streamline the onboarding experience and enhance clarity within the codebase. The onboarding component has been simplified to a static presentation, removing unnecessary dynamic elements that depended on application state. Additionally, import statements have been reorganized across various files to improve code readability without impacting functionality. These adjustments contribute to a more intuitive user experience and enhanced component usability. Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx (1)
Line range hint
85-102
:
Remove redundant test case.The test case is identical to the previous one and is redundant. Consider removing it to avoid confusion.
- it("3. does not render onboarding component when in preview mode", () => { - mockUseCurrentEditorStatePerTestCase(EditorEntityTab.UI); - const previewModeStore = { - ...storeToUseWithDragDropBuildingBlocksEnabled, - ui: { - ...storeToUseWithDragDropBuildingBlocksEnabled.ui, - gitSync: { - protectedBranches: false, - }, - editor: { - isPreviewMode: true, - }, - }, - }; - render(BaseComponentRender(previewModeStore)); - - const buildingBlockOnboardingElement = screen.queryByText( - createMessage(EMPTY_CANVAS_HINTS.DRAG_DROP_BUILDING_BLOCK_HINT.TITLE), - ); - const onboardingElement = screen.queryByText( - createMessage(EMPTY_CANVAS_HINTS.DRAG_DROP_WIDGET_HINT), - ); - expect(buildingBlockOnboardingElement).not.toBeInTheDocument(); - expect(onboardingElement).toBeInTheDocument(); - });
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_Sidebar.ts (1 hunks)
- app/client/src/constants/WidgetConstants.tsx (2 hunks)
- app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx (1 hunks)
- app/client/src/layoutSystems/common/dropTarget/OnBoarding/index.tsx (1 hunks)
Files skipped from review due to trivial changes (3)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_Sidebar.ts
- app/client/src/constants/WidgetConstants.tsx
- app/client/src/layoutSystems/common/dropTarget/OnBoarding/index.tsx
Additional comments not posted (2)
app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx (2)
59-81
: Great job! Ensure consistency in test descriptions.The updated test case correctly checks that the onboarding component does not render in preview mode. However, ensure that the test description is consistent with other similar test cases for clarity.
Line range hint
140-160
:
Well done! The test case is clear and consistent.The test case correctly ensures that the onboarding component does not render in preview mode for the AirGap edition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx (3 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10151214900. |
Deploy-Preview-URL: https://ce-35270.dp.appsmith.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/Editor/widgetSidebar/tests/UIEntitySidebar.test.tsx (1 hunks)
Additional comments not posted (5)
app/client/src/pages/Editor/widgetSidebar/tests/UIEntitySidebar.test.tsx (5)
124-128
: Good job updating the test case to reflect the new requirements!The change from
queryByText
togetByText
is appropriate as it ensures the test will fail if the "Suggested" section is not present when the feature flag is enabled.
Line range hint
29-35
:
Well done on ensuring the search input and "Suggested" section are rendered correctly!The test case is clear and verifies the essential elements are present in the UI.
Line range hint
37-47
:
Great job on testing the component's robustness with an empty cards array!This test case ensures that the component handles edge cases gracefully, which is crucial for maintaining a robust application.
Line range hint
49-57
:
Excellent work on verifying the display of grouped widget cards!This test case ensures that the component behaves correctly when the feature flag is disabled, which is crucial for maintaining expected functionality.
Line range hint
59-70
:
Nice job on testing the search functionality!This test case ensures that the component filters the widget cards correctly based on user input, which is essential for a good user experience.
Description
Problem
While we don't have the configuration interface for blocks, we should make them available as we have other regular widgets, but we shouldn't promote them for new users, since they are not able to easily modify them.
Remove the building blocks empty state that is creating confusion.
Solution
Fixes #34964
Automation
/ok-to-test tags="@tag.IDE, @tag.Widget, @tag.Templates"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10187244230
Commit: e0aac0e
Cypress dashboard.
Tags:
@tag.IDE, @tag.Widget, @tag.Templates
Spec:
Wed, 31 Jul 2024 21:39:07 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Chores