chore: Move Sidebar to IDE/Components #34487
Conversation
WalkthroughThe changes involve enhancing the sidebar functionality in an IDE-like environment. Key modifications include introducing a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
User->>IDESidebar: Click Button
IDESidebar->>SidebarButton: Update State
SidebarButton->>IDESidebar: Reflect Condition
User->>IDESidebar: Navigate based on Condition
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 Configration File (
|
github-actions
bot
added
the
skip-changelog
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
hetunandu
added
ok-to-test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- app/client/src/IDE/Components/BottomView.tsx (1 hunks)
- app/client/src/IDE/Components/Sidebar/SidebarButton.test.tsx (1 hunks)
- app/client/src/IDE/Components/Sidebar/SidebarButton.tsx (3 hunks)
- app/client/src/IDE/Components/Sidebar/index.tsx (1 hunks)
- app/client/src/IDE/index.ts (1 hunks)
- app/client/src/api/LibraryAPI.tsx (1 hunks)
- app/client/src/ce/entities/IDE/constants.ts (3 hunks)
- app/client/src/pages/Editor/IDE/Sidebar/index.tsx (2 hunks)
- app/client/src/pages/Editor/JSEditor/constants.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/pages/Editor/JSEditor/constants.ts
Additional context used
Biome
app/client/src/api/LibraryAPI.tsx
[error] 5-33: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
Additional comments not posted (12)
app/client/src/IDE/Components/Sidebar/SidebarButton.test.tsx (2)
8-13: Ensure comprehensive prop setup for testing.The
sidebarButtonPropssetup appears to be comprehensive for the test scenario, including all necessary properties. This setup ensures that the component can be tested under controlled conditions.
16-26: Validate rendering logic under specific conditions.The test checks if the warning icon renders correctly when the
conditionprop is set toWarn. This is a good practice as it ensures that the component behaves as expected under different conditions.app/client/src/IDE/Components/Sidebar/index.tsx (2)
35-78: Good use of React hooks and callback optimization.The
SidebarComponentusesuseCallbackeffectively to optimize the rendering performance. The structured and clean implementation of the component with conditional rendering and hooks demonstrates good React practices.
18-25: Consider relocatingISidebarButtoninterface.As per the past review comment, consider moving the
ISidebarButtoninterface to a constants file to improve organization and reusability.- export interface ISidebarButton { - state: EditorState; - icon: string; - title?: string; - urlSuffix: string; - condition?: Condition; - tooltip?: string; - } + // This interface should be moved to a more central location like `constants.ts`.app/client/src/pages/Editor/IDE/Sidebar/index.tsx (2)
22-46: Dynamic update of button states based on datasource availability.The use of
useEffectto dynamically update thetopButtonsbased on the availability of datasources is a robust design choice. It ensures that the UI reflects the current state accurately and provides feedback to the user.
68-73: Proper integration ofIDESidebar.The
Sidebarcomponent integratesIDESidebareffectively, passing all necessary props and handling state changes appropriately. This modular approach enhances maintainability and reusability.app/client/src/IDE/Components/Sidebar/SidebarButton.tsx (2)
5-19: Well-definedConditionenum and configuration.The introduction of the
Conditionenum and the correspondingConditionConfigenhances the component's flexibility and readability by mapping conditions to visual representations.
85-88: Conditional rendering of icons based oncondition.The implementation of conditional rendering for the
ConditionIconbased on theconditionprop is a good practice. It ensures that the UI elements are only rendered when necessary, improving performance.app/client/src/IDE/index.ts (1)
47-49: Ensure correct export paths and types for Sidebar-related entities.The new exports for
IDESidebar,ISidebarButton, andConditionare introduced. It's crucial to ensure that these components are correctly implemented and exported from their respective files. The comments provide a good overview of theIDESidebar.app/client/src/ce/entities/IDE/constants.ts (2)
Line range hint
61-86: Review initialization ofTopButtonsandBottomButtonsarrays.The arrays
TopButtonsandBottomButtonshave been updated to include thetooltipandurlSuffixproperties. Ensure that these properties are being utilized correctly in the Sidebar component.
24-25: Validate the introduction ofISidebarButtontype.The
ISidebarButtontype has been imported and is being used in the definition ofTopButtonsandBottomButtons. It's important to ensure that this type is correctly defined in the referenced location.app/client/src/IDE/Components/BottomView.tsx (1)
6-7: Review updated import paths and usage of view behaviors.The import paths for
CodeEditorWithGutterStylesand view behaviors have been updated. Verify that these changes are correctly reflected in the usage within the component.Verification successful
Verified correct import and usage of
CodeEditorWithGutterStylesand view behaviors.The imports for
CodeEditorWithGutterStyles,ViewDisplayMode, andViewHideBehaviourare correctly reflected and used within theBottomViewcomponent.
Import Statements:
import { CodeEditorWithGutterStyles } from "pages/Editor/JSEditor/styledComponents"; import { ViewDisplayMode, ViewHideBehaviour } from "IDE/Interfaces/View";Usage:
const Container = styled.div<{ displayMode: ViewDisplayMode }>` case ViewDisplayMode.OVERLAY: ${CodeEditorWithGutterStyles}; [ViewHideBehaviour.COLLAPSE]: `${VIEW_MIN_HEIGHT}px`, [ViewHideBehaviour.CLOSE]: "0px", behaviour: ViewHideBehaviour; displayMode?: ViewDisplayMode; hideBehaviour: ViewHideBehaviour; return props.hideBehaviour === ViewHideBehaviour.CLOSE if (props.hideBehaviour === ViewHideBehaviour.COLLAPSE) { displayMode={props.displayMode || ViewDisplayMode.BLOCK}The code changes are consistent and correctly implemented.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct import and usage of `CodeEditorWithGutterStyles` and view behaviors. # Test: Search for the usage of `CodeEditorWithGutterStyles` and view behaviors. rg --type typescript $'CodeEditorWithGutterStyles|ViewHideBehaviour|ViewDisplayMode' app/client/src/IDE/Components/BottomView.tsxLength of output: 165
Script:
#!/bin/bash # Description: Verify the correct import and usage of `CodeEditorWithGutterStyles` and view behaviors. # Test: Search for the usage of `CodeEditorWithGutterStyles` and view behaviors. rg 'CodeEditorWithGutterStyles|ViewHideBehaviour|ViewDisplayMode' app/client/src/IDE/Components/BottomView.tsxLength of output: 785
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
app/client/src/IDE/Components/Sidebar/SidebarButton.tsx (1)
Line range hint
1-83: SidebarButton Component Needs Completion of Condition ConfigurationsThe
SidebarButtoncomponent is well-implemented with conditional rendering of icons based on theconditionprop. However, the TODO comments indicate that further configurations for different conditions are needed. Completing these would ensure consistency and completeness of visual feedback.Would you like me to help complete this configuration?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- app/client/src/IDE/Components/Sidebar/SidebarButton.test.tsx (1 hunks)
- app/client/src/IDE/Components/Sidebar/SidebarButton.tsx (3 hunks)
- app/client/src/IDE/Components/Sidebar/index.tsx (1 hunks)
- app/client/src/IDE/Interfaces/Condition.ts (1 hunks)
- app/client/src/IDE/Interfaces/ISidebarButton.ts (1 hunks)
- app/client/src/IDE/index.ts (1 hunks)
- app/client/src/pages/Editor/IDE/Sidebar.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/IDE/Interfaces/Condition.ts
Additional comments not posted (6)
app/client/src/IDE/Interfaces/ISidebarButton.ts (1)
1-12: Interface Definition for Sidebar Buttons: Looks GoodThe
ISidebarButtoninterface is well-defined with appropriate properties that support different states and conditions of sidebar buttons. This should facilitate the implementation of dynamic and responsive sidebar buttons in the IDE.app/client/src/IDE/Components/Sidebar/SidebarButton.test.tsx (1)
1-25: Comprehensive Test for SidebarButton ComponentThe test setup for
SidebarButtonis appropriately configured to check the rendering behavior under specific conditions (empty datasource list). The use of props to simulate this scenario and the assertion on the number of rendered SVGs are correctly implemented.app/client/src/IDE/Components/Sidebar/index.tsx (1)
1-72: Well-structured IDESidebar ComponentThe
IDESidebarcomponent is well-implemented with clear separation of concerns and effective use of React hooks for performance optimization. The component logic for handling button clicks and rendering based on the app state is correctly implemented.app/client/src/pages/Editor/IDE/Sidebar.tsx (1)
1-78: Comprehensive Sidebar Component with Dynamic Button StatesThe
Sidebarcomponent effectively integrates with Redux and uses hooks to manage side effects and state transitions. The logic to dynamically update button states based on datasource availability is crucial for a responsive user experience.app/client/src/IDE/index.ts (2)
43-47: Export of IDESidebar ComponentThe export of the
IDESidebarcomponent from./Components/Sidebaris correctly implemented. This aligns with the PR's goal of restructuring the Sidebar component within the IDE module.
55-56: Export of Interfaces and TypesThe export of
ISidebarButtonandConditioninterfaces is correctly implemented. It's essential to ensure that these interfaces are used consistently throughout the project.Verification successful
Export of Interfaces and Types
The export of
ISidebarButtonandConditioninterfaces is correctly implemented and these interfaces are used consistently throughout the project.
Instances of
ISidebarButtonfound in:
app/client/src/IDE/Interfaces/ISidebarButton.tsapp/client/src/IDE/Components/Sidebar/index.tsxapp/client/src/IDE/Components/Sidebar/SidebarButton.tsxapp/client/src/ce/entities/IDE/constants.tsInstances of
Conditionfound in:
app/client/src/IDE/Interfaces/Condition.tsapp/client/src/IDE/Components/Sidebar/SidebarButton.tsxapp/client/src/widgets/TableWidgetV2/component/Constants.tsapp/client/src/widgets/TableWidgetV2/component/header/actions/filter/CascadeFields.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent use of `ISidebarButton` and `Condition` interfaces across the project. # Test: Search for the interface usage. Expect: Consistent usage across the project. rg --type typescript $'ISidebarButton|Condition'Length of output: 84
Script:
#!/bin/bash # Description: Verify consistent use of `ISidebarButton` and `Condition` interfaces across the project. # Test: Search for the interface usage. Expect: Consistent usage across the project. rg 'ISidebarButton|Condition' --glob '*.ts' --glob '*.tsx'Length of output: 38386
hetunandu
added
ok-to-test
hetunandu
requested review from
alex-golovanov,
albinAppsmith and
AmanAgarwal041
github-actions
bot
added
IDE Navigation
hetunandu
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- app/client/src/IDE/Components/Sidebar/Sidebar.tsx (1 hunks)
- app/client/src/IDE/Components/Sidebar/SidebarButton/SidebarButton.tsx (1 hunks)
- app/client/src/IDE/index.ts (1 hunks)
- app/client/src/pages/Editor/IDE/Sidebar.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- app/client/src/IDE/Components/Sidebar/Sidebar.tsx
- app/client/src/IDE/Components/Sidebar/SidebarButton/SidebarButton.tsx
- app/client/src/IDE/index.ts
- app/client/src/pages/Editor/IDE/Sidebar.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/src/IDE/Components/Sidebar/SidebarButton/SidebarButton.test.tsx (1 hunks)
- app/client/src/IDE/Components/Sidebar/SidebarButton/SidebarButton.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/client/src/IDE/Components/Sidebar/SidebarButton/SidebarButton.test.tsx
- app/client/src/IDE/Components/Sidebar/SidebarButton/SidebarButton.tsx
|
@coderabbitai generate unit tests |
|
@ApekshaBhosale, I've generated the unit tests for the SidebarButton.test.tsximport React from "react";
import { render, fireEvent } from "@testing-library/react";
import SidebarButton, { SidebarButtonProps } from "./SidebarButton";
import { Condition } from "../../../enums";
describe("SidebarButton", () => {
const defaultProps: SidebarButtonProps = {
condition: Condition.DEFAULT,
onClick: jest.fn(),
// other default props
};
it("should render the button with default condition", () => {
const { getByRole } = render(<SidebarButton {...defaultProps} />);
const button = getByRole("button");
expect(button).toBeInTheDocument();
// Add more assertions based on default condition
});
it("should render the button with warning condition", () => {
const props = { ...defaultProps, condition: Condition.WARNING };
const { getByRole } = render(<SidebarButton {...props} />);
const button = getByRole("button");
expect(button).toBeInTheDocument();
// Add more assertions based on warning condition
});
it("should call onClick when button is clicked", () => {
const { getByRole } = render(<SidebarButton {...defaultProps} />);
const button = getByRole("button");
fireEvent.click(button);
expect(defaultProps.onClick).toHaveBeenCalled();
});
});IDESidebar.test.tsximport React from "react";
import { render, fireEvent } from "@testing-library/react";
import IDESidebar, { IDESidebarProps } from "./IDESidebar";
import { Condition } from "../../../enums";
describe("IDESidebar", () => {
const defaultProps: IDESidebarProps = {
buttons: [
{ id: "1", label: "Button 1", condition: Condition.DEFAULT, onClick: jest.fn() },
{ id: "2", label: "Button 2", condition: Condition.WARNING, onClick: jest.fn() },
],
// other default props
};
it("should render all buttons", () => {
const { getByText } = render(<IDESidebar {...defaultProps} />);
defaultProps.buttons.forEach(button => {
expect(getByText(button.label)).toBeInTheDocument();
});
});
it("should handle button click events", () => {
const { getByText } = render(<IDESidebar {...defaultProps} />);
const button = getByText("Button 1");
fireEvent.click(button);
expect(defaultProps.buttons[0].onClick).toHaveBeenCalled();
});
// Add more tests based on different conditions and props
});These tests cover the basic rendering and interaction scenarios for the Actions performedReview triggered.
|
|
@coderabbitai generate test plan |
|
Tip For best results, initiate chat on the files or code changes. @ApekshaBhosale, here is the detailed test plan for the Test Plan for
|
| condition?: Condition; | ||
| } | ||
|
|
||
| const Container = styled(Flex)` |
There was a problem hiding this comment.
All these can be passed to the Flex component as prop.
There was a problem hiding this comment.
## Description Separate the Sidebar Component from the various IDEs and move it to the IDE module. Fixes appsmithorg#34554 ## Automation /ok-to-test tags="@tag.IDE" ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9869212078> > Commit: 0b685d4 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9869212078&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.IDE` > Spec: > <hr>Wed, 10 Jul 2024 06:47:16 UTC <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new sidebar component (`IDESidebar`) managing button states and handling interactions. - Added `Condition` enum for better condition management with icons and colors in the sidebar buttons. - **Enhancements** - Improved click handling for sidebar buttons with a new `handleOnClick` function. - **Tests** - Added test cases for `SidebarButton` component to validate different conditions and click behaviors. - **Components** - New React components and interfaces for managing the IDE's sidebar functionality and state. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Separate the Sidebar Component from the various IDEs and move it to the IDE module.
Fixes #34554
Automation
/ok-to-test tags="@tag.IDE"
Communication
Should the DevRel and Marketing teams inform users about this change?
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9869212078
Commit: 0b685d4
Cypress dashboard.
Tags:
@tag.IDESpec:
Wed, 10 Jul 2024 06:47:16 UTC
Summary by CodeRabbit
New Features
IDESidebar) managing button states and handling interactions.Conditionenum for better condition management with icons and colors in the sidebar buttons.Enhancements
handleOnClickfunction.Tests
SidebarButtoncomponent to validate different conditions and click behaviors.Components