fix: Remove collapsed widget tags #34979
Conversation
WalkthroughThe updates enhance the Widgets Sidebar's functionality and user experience through simplified state management in the Changes
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 (
|
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9968483100. |
![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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx (1 hunks)
Additional comments not posted (1)
app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx (1)
162-162: Ensure proper integration of theisSearchingcondition.Dear student, let's take a moment to appreciate the addition of the
isSearchingcondition to the logic that determines if a section should be initially open. This is a crucial addition for enhancing user experience during searches. However, it's important to ensure that this new condition integrates seamlessly with the existing conditionsisAnvilandhasWidgets.Please verify that the logical combination of these conditions does not unintentionally affect other functionalities or the visibility of widget tags under different circumstances. It might be beneficial to add a few more test cases to cover scenarios where multiple conditions are true simultaneously.
|
Deploy-Preview-URL: https://ce-34979.dp.appsmith.com |
|
@hetunandu The experience still feels broken, the dropdown does not open the first time any widget is searched. It only opens after the input is cleared and searched for a second time. Here's a video to explain - https://www.loom.com/share/6d240f9fce1743af810c711a2f9168c6?sid=3dec1cb1-232b-4c77-a6df-676f60c339f7 |
hetunandu
changed the title
|
Updated the PR to reverse the change as per conversation |
There was a problem hiding this comment.
Actionable comments posted: 0
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/pages/Editor/widgetSidebar/UIEntitySidebar.tsx (3 hunks)
- app/client/src/pages/Editor/widgetSidebar/UIEntityTagGroup.tsx (3 hunks)
- app/client/src/pages/Editor/widgetSidebar/tests/UIEntitySidebar.test.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/pages/Editor/widgetSidebar/tests/UIEntitySidebar.test.tsx
Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_Sidebar.ts (1)
Pattern
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Additional comments not posted (3)
app/client/src/pages/Editor/widgetSidebar/UIEntityTagGroup.tsx (2)
Line range hint
13-17:
Verify the impact of removingisInitiallyOpenfromProps.The removal of the
isInitiallyOpenproperty simplifies the interface. However, please ensure that this change does not break any existing functionality where this prop might have been used.Verification successful
Let's correct our approach and re-run the search to ensure we capture any instances of
isInitiallyOpenin the codebase.
The removal of
isInitiallyOpenfrom thePropsinterface has been verified and does not impact the existing codebase.
- No instances of
isInitiallyOpenwere found in the codebase, confirming that this property is not being used elsewhere.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing `isInitiallyOpen` from `Props`. # Test: Search for the usage of `isInitiallyOpen` in the codebase. Expect: No instances of `isInitiallyOpen` being used. rg --type tsx --type ts 'isInitiallyOpen'Length of output: 69
Script:
#!/bin/bash # Description: Verify the impact of removing `isInitiallyOpen` from `Props`. # Test: Search for the usage of `isInitiallyOpen` in the codebase. Expect: No instances of `isInitiallyOpen` being used. rg 'isInitiallyOpen' --glob '*.ts' --glob '*.tsx'Length of output: 49
Line range hint
19-64:
Verify the component behavior after removingisInitiallyOpen.The removal of the
isInitiallyOpenstate initialization simplifies the component's internal state management. Please ensure that the component behaves as expected without this prop, particularly in scenarios where the initial open state was previously controlled byisInitiallyOpen.Verification successful
Let's correct the file type recognition issue and re-run the search to verify the usage of
UIEntityTagGroupin the codebase.
Let's search for the
isInitiallyOpenprop in the relevant files to confirm its removal and ensure it is not being used elsewhere in the codebase.
Verification complete: The
isInitiallyOpenprop has been successfully removed and is not used elsewhere in the codebase.The removal of the
isInitiallyOpenstate initialization simplifies the component's internal state management. The component should behave as expected without this prop, as confirmed by the absence ofisInitiallyOpenin the relevant files.
UIEntityTagGroup.tsxUIEntitySidebar.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the behavior of `UIEntityTagGroup` after removing `isInitiallyOpen`. # Test: Search for the `UIEntityTagGroup` usage in the codebase. Expect: No instances where the initial open state is controlled by `isInitiallyOpen`. rg --type tsx --type ts 'UIEntityTagGroup'Length of output: 70
Script:
#!/bin/bash # Description: Verify the behavior of `UIEntityTagGroup` after removing `isInitiallyOpen`. # Test: Search for the `UIEntityTagGroup` usage in the codebase. Expect: No instances where the initial open state is controlled by `isInitiallyOpen`. rg --glob '**/*.tsx' --glob '**/*.ts' 'UIEntityTagGroup'Length of output: 467
Script:
#!/bin/bash # Description: Verify the removal of `isInitiallyOpen` prop in the relevant files. # Test: Search for `isInitiallyOpen` in the relevant files. rg 'isInitiallyOpen' app/client/src/pages/Editor/widgetSidebar/UIEntityTagGroup.tsx app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsxLength of output: 145
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_Sidebar.ts (1)
Line range hint
1-136:
Verify the test behavior after removing thebeforehook.The removal of the
beforehook changes the initial state of the tests. Please ensure that the tests still function correctly without this setup, particularly in scenarios where the initial state was previously controlled by thebeforehook.
hetunandu
added
ok-to-test
hetunandu
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_Sidebar.ts (1 hunks)
- app/client/src/pages/Editor/widgetSidebar/UIEntityTagGroup.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_Sidebar.ts
- app/client/src/pages/Editor/widgetSidebar/UIEntityTagGroup.tsx
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10059635441. |
Description
Reverses the collapsible widget tags introduced in #34644
Automation
/ok-to-test tags="@tag.IDE, @tag.Anvil, @tag.Widget"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10053206890
Commit: c63a676
Cypress dashboard.
Tags:
@tag.IDE, @tag.Anvil, @tag.WidgetSpec:
Tue, 23 Jul 2024 06:29:58 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
UIEntityTagGroupcomponent to manage its visibility behavior statically instead of dynamically.Tests
UIEntitySidebarcomponent to reflect changes in behavior regarding the "Building Blocks" section.