Skip to content
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

chore: Reorganise IDE Panels #35114

Merged
merged 27 commits into from
Jul 31, 2024
Merged

chore: Reorganise IDE Panels #35114

merged 27 commits into from
Jul 31, 2024

Conversation

hetunandu
Copy link
Member

@hetunandu hetunandu commented Jul 23, 2024

Description

By re organising the panels in the IDE, we are creating a new Code Editor only panel which is separate from the main / canvas and the explorer panel. This ensures that the Code Editor is not remounted when switching between Split screen and Full screen. This PR is also improving the Preview mode by not changing urls when entering and exiting preview modes.

EE PR: https://github.com/appsmithorg/appsmith-ee/pull/4760

Fixes #31707
Fixes #31387
Fixes #34545
Fixes #34546

Automation

/ok-to-test tags="@tag.All"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10177085462
Commit: 26688b0
Cypress dashboard.
Tags: @tag.All
Spec:


Wed, 31 Jul 2024 09:42:50 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced EditorPaneExplorer and QueryExplorer components for enhanced navigation and query management within the IDE.
    • Added visual enhancements to ListJSObjects and AddWidgets components with new style properties.
  • Improvements

    • Implemented clamping in the useDynamicAppLayout hook to prevent negative width values, enhancing layout stability.
    • Updated layout management to ensure a responsive user experience based on IDE view modes.
  • Style

    • Enhanced visual clarity by adding borders to various components, improving UI separation and organization.
  • Bug Fixes

    • Streamlined test cases in the Postgres2_Spec.ts to maintain coherent sequencing after removing unnecessary tests.

Copy link
Contributor

coderabbitai bot commented Jul 23, 2024

Walkthrough

The recent changes enhance the structure and routing within the application's editor interface. The introduction of components like EditorPaneExplorer and QueryExplorer improves the management of IDE functionalities and streamlines navigation. Enhancements to layout calculations ensure more stable rendering and clearer visual separation, contributing to a better user experience overall.

Changes

Files and Paths Change Summary
app/client/src/ce/pages/Editor/IDE/EditorPane/* Introduction of EditorPaneExplorer and QueryExplorer for improved management of IDE functionalities.
app/client/src/utils/hooks/useDynamicAppLayout.tsx Clamping added to canvas width calculations to ensure valid dimensions and prevent layout issues.
app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx Added a borderRight style to JSContainer for improved visual separation in the ListJSObjects component.
app/client/src/pages/Editor/IDE/EditorPane/UI/Add.tsx Introduced a borderRight property in Container of AddWidgets to enhance UI clarity.
app/client/src/pages/Editor/IDE/EditorPane/Query/Explorer.tsx The QueryExplorer component now conditionally renders queries based on the IDE's full-screen mode.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts Test cases reordered and renumbered for clarity after the removal of a redundant test case.

Assessment against linked issues

Objective Addressed Explanation
Transition between edit and preview modes appear glitchy (#31707) Changes may address the issue; further testing needed.
Entering or exiting split pane mode with query selected (#31387) No modifications related to this issue were made.
Render only Data and Canvas on the main pane (#34545)
Add snapshots for text widgets (#34546) No snapshot tests have been added in this PR.

🌟 In the realm of code, where logic takes flight,
New components arise, bringing order and light.
With borders that guide and layouts refined,
A seamless experience for all users designed.
So let's celebrate changes that make the code shine,
Enhancing our journey, one line at a time! 🎉


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Jul 23, 2024
@hetunandu hetunandu added ok-to-test Required label for CI and removed skip-changelog Adding this label to a PR prevents it from being listed in the changelog labels Jul 23, 2024
@github-actions github-actions bot added IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE Task A simple Todo skip-changelog Adding this label to a PR prevents it from being listed in the changelog labels Jul 23, 2024
@hetunandu hetunandu added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 24, 2024
@hetunandu hetunandu marked this pull request as ready for review July 24, 2024 06:12
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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/pages/Editor/IDE/EditorPane/Query/index.tsx (1)

1-1: Export QueryEditor seems to be incorrect!

The search results indicate that there is no implementation of QueryEditor as a class or function in the codebase. All references point to QueryEditor.json in Cypress test files. Please verify the existence and correctness of the QueryEditor export in index.tsx.

  • File: app/client/src/pages/Editor/IDE/EditorPane/Query/index.tsx
  • Line: 1
Analysis chain

Export QueryEditor looks good!

The export statement is correct. Ensure that QueryEditor is correctly implemented and used in the codebase.


Let's correct the file type and re-run the search to verify the implementation and usage of QueryEditor.


Let's search for QueryEditor in a broader context to confirm its existence and implementation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and usage of `QueryEditor`.

# Test: Search for the implementation of `QueryEditor`. Expect: Only one occurrence in `Editor.tsx`.
rg --type tsx 'const QueryEditor' --files-with-matches

# Test: Search for the usage of `QueryEditor`. Expect: Multiple occurrences in the codebase.
rg --type tsx 'QueryEditor'

Length of output: 140


Script:

#!/bin/bash
# Description: Verify the implementation and usage of `QueryEditor`.

# Test: Search for the implementation of `QueryEditor`. Expect: Only one occurrence in `Editor.tsx`.
rg --type js 'const QueryEditor' --files-with-matches

# Test: Search for the usage of `QueryEditor`. Expect: Multiple occurrences in the codebase.
rg --type js 'QueryEditor'

Length of output: 1808


Script:

#!/bin/bash
# Description: Verify the implementation and usage of `QueryEditor`.

# Test: Search for the implementation of `QueryEditor` in a broader context.
rg --type js 'QueryEditor'

# Test: Search for the implementation of `QueryEditor` as a class or function.
rg --type js 'class QueryEditor'
rg --type js 'function QueryEditor'

Length of output: 1825

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d39eb58 and 809cd30.

Files selected for processing (20)
  • app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts (3 hunks)
  • app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (4 hunks)
  • app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts (6 hunks)
  • app/client/src/ce/sagas/PageSagas.tsx (3 hunks)
  • app/client/src/components/editorComponents/PropertyPaneSidebar.tsx (1 hunks)
  • app/client/src/constants/AppConstants.ts (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/Editor.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/JS/Editor.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/JS/Explorer.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/JS/index.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/Query/Editor.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/Query/Explorer.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/Query/index.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/UI/index.tsx (2 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/index.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/EditorTabs/List.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/MainPane/index.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/hooks.ts (5 hunks)
  • app/client/src/pages/Editor/IDE/index.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
  • app/client/src/components/editorComponents/PropertyPaneSidebar.tsx
Additional comments not posted (54)
app/client/src/pages/Editor/IDE/EditorPane/JS/index.tsx (2)

1-1: Export JSExplorer looks good!

The export statement is correct. Ensure that JSExplorer is correctly implemented and used in the codebase.

Verification successful

Let's correct the previous script and re-verify the implementation and usage of JSExplorer.


Export JSExplorer looks good!

The export statement is correct, and JSExplorer is properly implemented and used in the codebase.

  • JSExplorer is implemented in app/client/src/pages/Editor/IDE/EditorPane/JS/Explorer.tsx.
  • It is used in app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx and app/client/src/pages/Editor/IDE/EditorPane/JS/index.tsx.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and usage of `JSExplorer`.

# Test: Search for the implementation of `JSExplorer`. Expect: Only one occurrence in `Explorer.tsx`.
rg --type tsx 'const JSExplorer' --files-with-matches

# Test: Search for the usage of `JSExplorer`. Expect: Multiple occurrences in the codebase.
rg --type tsx 'JSExplorer'

Length of output: 138


Script:

#!/bin/bash
# Description: Verify the implementation and usage of `JSExplorer`.

# Test: Search for the implementation of `JSExplorer`. Expect: Only one occurrence in `Explorer.tsx`.
rg 'const JSExplorer' --files-with-matches

# Test: Search for the usage of `JSExplorer`. Expect: Multiple occurrences in the codebase.
rg 'JSExplorer'

Length of output: 564


2-2: Export JSEditorPane looks good!

The export statement is correct. Ensure that JSEditorPane is correctly implemented and used in the codebase.

Verification successful

Let's correct the script to search for JSEditorPane implementation and usage without specifying the file type.


Export JSEditorPane looks good!

The export statement is correct, and JSEditorPane is properly implemented and used in the codebase.

  • Implementation found in app/client/src/pages/Editor/IDE/EditorPane/JS/Editor.tsx.
  • Usage confirmed in app/client/src/pages/Editor/IDE/EditorPane/Editor.tsx and app/client/src/pages/Editor/IDE/EditorPane/JS/index.tsx.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and usage of `JSEditorPane`.

# Test: Search for the implementation of `JSEditorPane`. Expect: Only one occurrence in `Editor.tsx`.
rg --type tsx 'const JSEditorPane' --files-with-matches

# Test: Search for the usage of `JSEditorPane`. Expect: Multiple occurrences in the codebase.
rg --type tsx 'JSEditorPane'

Length of output: 142


Script:

#!/bin/bash
# Description: Verify the implementation and usage of `JSEditorPane`.

# Test: Search for the implementation of `JSEditorPane`. Expect: Only one occurrence in `Editor.tsx`.
rg 'const JSEditorPane' --files-with-matches

# Test: Search for the usage of `JSEditorPane`. Expect: Multiple occurrences in the codebase.
rg 'JSEditorPane'

Length of output: 566

app/client/src/pages/Editor/IDE/EditorPane/Query/index.tsx (1)

2-2: Export QueryExplorer looks good!

The export statement is correct. Ensure that QueryExplorer is correctly implemented and used in the codebase.

Verification successful

Let's verify the implementation and usage of QueryExplorer again!

We'll adjust our approach to ensure we correctly gather the necessary information.


Export QueryExplorer looks good!

The export statement is correct, and QueryExplorer is properly implemented and used in the codebase.

  • Implementation: app/client/src/pages/Editor/IDE/EditorPane/Query/Explorer.tsx
  • Usage:
    • app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx
    • app/client/src/pages/Editor/IDE/EditorPane/Query/index.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation and usage of `QueryExplorer`.

# Test: Search for the implementation of `QueryExplorer`. Expect: Only one occurrence in `Explorer.tsx`.
rg --type tsx 'const QueryExplorer' --files-with-matches

# Test: Search for the usage of `QueryExplorer`. Expect: Multiple occurrences in the codebase.
rg --type tsx 'QueryExplorer'

Length of output: 144


Script:

#!/bin/bash
# Description: Verify the implementation and usage of `QueryExplorer`.

# Test: Search for the implementation of `QueryExplorer`. Expect: Only one occurrence in `Explorer.tsx`.
rg 'const QueryExplorer' --glob '*.tsx' --files-with-matches

# Test: Search for the usage of `QueryExplorer`. Expect: Multiple occurrences in the codebase.
rg 'QueryExplorer' --glob '*.tsx'

Length of output: 630

app/client/src/pages/Editor/IDE/EditorPane/JS/Explorer.tsx (3)

1-5: Import statements look good!

The imported modules are necessary for the implementation of JSExplorer.


7-13: JSExplorer component logic looks good!

The component correctly uses the useSelector hook to get the IDE view mode and conditionally renders the List component based on the view mode.


15-15: Export statement looks good!

The export statement is correct.

app/client/src/pages/Editor/IDE/EditorPane/JS/Editor.tsx (4)

1-5: Good practice: Import statements are clear and concise.

The import statements are well-organized and only include necessary modules.


6-8: Well done: Functional component definition is clear.

The functional component JSEditorPane is defined clearly and uses useRouteMatch to get the current path.


9-20: Excellent use of routing logic.

The routing logic is well-implemented using useJSEditorRoutes and SentryRoute. This ensures that the routes are dynamically generated based on the current path.


23-23: Good practice: Named export used.

The named export ensures that the component can be easily imported in other files.

app/client/src/pages/Editor/IDE/EditorPane/Query/Editor.tsx (4)

1-5: Good practice: Import statements are clear and concise.

The import statements are well-organized and only include necessary modules.


7-9: Well done: Functional component definition is clear.

The functional component QueryEditor is defined clearly and uses useRouteMatch to get the current path.


10-21: Excellent use of routing logic.

The routing logic is well-implemented using useQueryEditorRoutes and SentryRoute. This ensures that the routes are dynamically generated based on the current path.


24-24: Good practice: Named export used.

The named export ensures that the component can be easily imported in other files.

app/client/src/pages/Editor/IDE/EditorPane/index.tsx (4)

4-8: Good practice: Import statements are clear and concise.

The import statements are well-organized and only include necessary modules.


10-12: Well done: Functional component definition is clear.

The functional component EditorPane is defined clearly and uses useEditorPaneWidth and useSelector to get the necessary state.


13-25: Excellent use of conditional rendering.

The component uses conditional rendering to set the flexDirection of the Flex container based on ideViewMode. This ensures a dynamic layout.


27-27: Good practice: Default export used.

The default export ensures that the component can be easily imported in other files.

app/client/src/pages/Editor/IDE/EditorTabs/List.tsx (2)

27-27: Good job on adjusting the top margin!

The change from top-[78px] to top-[32px] effectively moves the component higher on the page, which can help improve the layout and alignment of the user interface.


29-29: Nice update on the height calculation!

The change from h="calc(100% - 78px)" to h="calc(100% - 32px)" ensures that the height calculation reflects the new top position, maintaining the overall layout integrity.

app/client/src/pages/Editor/IDE/EditorPane/Query/Explorer.tsx (4)

1-7: Great choice of imports!

The imported modules and constants are appropriate and necessary for the component's functionality.


9-18: Well-defined styled component!

The QueriesContainer styled component is defined correctly and aligns with the design system.


20-35: Nice implementation of the QueryExplorer component!

The component is implemented correctly with sound conditional rendering logic and an appropriate structure.


37-37: Correct export statement!

The QueryExplorer component is exported correctly, aligning with the file's purpose.

app/client/src/pages/Editor/IDE/MainPane/index.tsx (3)

7-8: Good update on the import statements!

The updated imports align with the new logic and functionality of the component.


15-18: Well-implemented state variable and rendering logic!

The new state variable isCombinedPreviewMode and the updated rendering logic are implemented correctly, aligning with the objectives.


26-26: Appropriate removal of the key!

The removal of the key tied to BUILDER_PATH aligns with the refactor and does not introduce any issues.

app/client/src/pages/Editor/IDE/EditorPane/Editor.tsx (8)

1-3: Great start with imports!

The imports are well-organized and include necessary dependencies.


4-8: Good use of constants for routes!

Using constants for routes helps maintain consistency and makes the code more readable.


9-10: Component imports are clear and concise!

Importing components in this manner keeps the code clean and maintainable.


12-13: Excellent use of custom hooks and constants!

The custom hook useCurrentEditorState and the constant EditorEntityTab are used effectively to manage state and conditional rendering.


15-20: Conditional rendering based on editor state is well-implemented!

The condition to return null if the segment is EditorEntityTab.UI is clear and concise.


21-28: Good use of Flex for layout!

Using the Flex component from the design system ensures a responsive and flexible layout.


29-39: Switch and SentryRoute usage is spot on!

The Switch component along with SentryRoute ensures that the correct components are rendered based on the route. Mapping the routes dynamically using jsSegmentRoutes and querySegmentRoutes is a good practice.


44-44: Exporting the component as default is good practice!

Exporting the Editor component as default makes it easy to import and use in other parts of the application.

app/client/src/pages/Editor/IDE/index.tsx (1)

45-48: Ensure the removal of h-full doesn't affect layout adversely.

The removal of the h-full class might impact the height of the div. Verify that this change aligns with the intended design and doesn't cause layout issues.

app/client/src/pages/Editor/IDE/EditorPane/UI/index.tsx (2)

21-21: Good use of constants for layout!

Importing DEFAULT_EXPLORER_PANE_WIDTH from constants/AppConstants ensures that the layout is consistent and easily configurable.


45-45: Dynamic width calculation is well-implemented!

Using DEFAULT_EXPLORER_PANE_WIDTH - 1 + "px" for the width ensures that the layout is responsive and aligns with design standards.

app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx (3)

1-23: Ensure all imports are necessary.

While all imports seem relevant to the file, it's always a good practice to review and ensure that each imported module and constant is necessary and used within the file. Unused imports can clutter the code and affect maintainability.


25-69: Check for proper handling of ideViewMode.

The ideViewMode state is used to conditionally render styles and widths. Ensure that all possible values of ideViewMode are handled correctly and that the default case is covered.


29-67: Ensure proper usage of Switch and SentryRoute.

The Switch component is used to render different routes based on the path. Ensure that all paths and components are correctly defined and that the routing logic is accurate.

app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts (3)

Line range hint 1-43:
Ensure proper handling of currentEntityInfo.

The currentEntityInfo state is used in both callback functions. Ensure that all possible values of currentEntityInfo are handled correctly and that the default case is covered.


Line range hint 44-52:
Ensure proper handling of isCreating state.

The isCreating state is used to determine if a JS module is being created. Ensure that all possible values of isCreating are handled correctly and that the default case is covered.


77-95: Ensure proper route definitions.

The function returns an array of route objects. Ensure that all routes are correctly defined and that the routing logic is accurate.

app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts (1)

Line range hint 1-113:
Ensure proper route definitions.

The function returns an array of route objects. Ensure that all routes are correctly defined and that the routing logic is accurate.

app/client/src/constants/AppConstants.ts (2)

15-15: Good job on renaming the constant for clarity!

The change from DEFAULT_EDITOR_PANE_WIDTH to DEFAULT_EXPLORER_PANE_WIDTH makes the purpose of this constant clearer.

However, ensure that all references to the old constant name are updated throughout the codebase.

Verification successful

Let's ensure that the old constant name DEFAULT_EDITOR_PANE_WIDTH has been completely replaced by DEFAULT_EXPLORER_PANE_WIDTH across the entire codebase.


Good job on renaming the constant for clarity!

The change from DEFAULT_EDITOR_PANE_WIDTH to DEFAULT_EXPLORER_PANE_WIDTH makes the purpose of this constant clearer.

The verification process confirms that all references to the old constant name have been updated throughout the codebase, as no occurrences of DEFAULT_EDITOR_PANE_WIDTH were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the old constant name are updated.

# Test: Search for the old constant name. Expect: No occurrences of the old constant name.
rg --type ts 'DEFAULT_EDITOR_PANE_WIDTH'

Length of output: 40


Script:

#!/bin/bash
# Description: Verify all references to the old constant name are updated across the entire codebase.

# Test: Search for the old constant name in all file types. Expect: No occurrences of the old constant name.
rg 'DEFAULT_EDITOR_PANE_WIDTH'

Length of output: 30


16-16: Nice improvement on the constant representation!

The change from DEFAULT_SPLIT_SCREEN_WIDTH to SPLIT_SCREEN_RATIO and the transition from a string to a numeric value enhances type safety and calculation accuracy.

However, ensure that all references to the old constant name are updated throughout the codebase.

Verification successful

Let's verify that the new constant SPLIT_SCREEN_RATIO is being used correctly throughout the codebase.


Nice improvement on the constant representation!

The change from DEFAULT_SPLIT_SCREEN_WIDTH to SPLIT_SCREEN_RATIO and the transition from a string to a numeric value enhances type safety and calculation accuracy.

All references to the old constant name have been successfully updated throughout the codebase.

  • app/client/src/pages/Editor/IDE/hooks.ts
  • app/client/src/constants/AppConstants.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the old constant name are updated.

# Test: Search for the old constant name. Expect: No occurrences of the old constant name.
rg --type ts 'DEFAULT_SPLIT_SCREEN_WIDTH'

Length of output: 41


Script:

#!/bin/bash
# Description: Verify the usage of the new constant name.

# Test: Search for the new constant name. Expect: Occurrences of the new constant name.
rg --type ts 'SPLIT_SCREEN_RATIO'

Length of output: 278

app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (2)

48-54: Simplified dependencies, well done!

The removal of segmentMode from the dependencies of openAddQuery and closeAddQuery simplifies the dependency list. Ensure that this removal does not affect the functionality of these callbacks.

However, verify that the removal of segmentMode does not introduce any issues.


105-143: Excellent refactor for better readability!

The renaming from useQuerySegmentRoutes to useQueryEditorRoutes and the consolidation of the routing logic enhance code readability and maintainability.

However, ensure that all references to the old function name are updated throughout the codebase.

Verification successful

Let's ensure that all references to the old function name useQuerySegmentRoutes have been updated throughout the codebase. We will run a more comprehensive search to confirm this.


Let's verify that all references to the old function name useQuerySegmentRoutes have been updated throughout the codebase by running a comprehensive search.


Let's ensure that all references to the old function name useQuerySegmentRoutes have been updated. We will verify the search command's functionality and conduct a comprehensive search across all file types.


Excellent refactor for better readability!

The renaming from useQuerySegmentRoutes to useQueryEditorRoutes and the consolidation of the routing logic enhance code readability and maintainability.

All references to the old function name useQuerySegmentRoutes have been successfully updated throughout the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the old function name are updated.

# Test: Search for the old function name. Expect: No occurrences of the old function name.
rg --type ts 'useQuerySegmentRoutes'

Length of output: 36


Script:

#!/bin/bash
# Description: Verify all references to the old function name are updated.

# Test: Search for the old function name in all relevant file types.
rg 'useQuerySegmentRoutes'

Length of output: 26


Script:

#!/bin/bash
# Description: Verify all references to the old function name are updated.

# Test: Search for the old function name in all file types.
rg 'useQuerySegmentRoutes'

Length of output: 26


Script:

#!/bin/bash
# Description: Verify all references to the old function name are updated.

# Test: Verify search functionality with a known term.
echo "test search functionality" > test_search.txt
rg 'test search functionality' test_search.txt

# Test: Search for the old function name in all file types.
rg 'useQuerySegmentRoutes'

Length of output: 136

app/client/src/pages/Editor/IDE/hooks.ts (2)

79-99: Great job on improving the responsiveness!

The updated logic for calculating the editor pane width using useWindowDimensions and the simplified conditional logic improve responsiveness and clarity.

However, verify that the new logic works correctly in all scenarios.

Verification successful

Let's take a closer look at how useEditorPaneWidth is being used in EditorPane/index.tsx to ensure the new logic is functioning correctly.


Great job on improving the responsiveness!

The updated logic for calculating the editor pane width using useWindowDimensions and the simplified conditional logic improve responsiveness and clarity. The implementation in EditorPane/index.tsx correctly applies the calculated width to the Flex component, ensuring the new logic works as intended.

  • File: app/client/src/pages/Editor/IDE/EditorPane/index.tsx
  • Lines: Around the usage of useEditorPaneWidth
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the functionality of `useEditorPaneWidth`.

# Test: Search for the usage of `useEditorPaneWidth`. Expect: No issues related to the new logic.
rg --type ts 'useEditorPaneWidth'

Length of output: 315


Script:

#!/bin/bash
# Extract lines around the usage of `useEditorPaneWidth` in `EditorPane/index.tsx` to understand its context.
rg --type ts 'useEditorPaneWidth' -A 10 -B 10 app/client/src/pages/Editor/IDE/EditorPane/index.tsx

Length of output: 775


175-175: Nice enhancement on the effect dependencies!

Adding dispatch to the dependencies ensures that the effect runs correctly when the dispatch function changes. The refined logic improves the functionality.

However, verify that the new logic works correctly.

app/client/src/ce/sagas/PageSagas.tsx (3)

Line range hint 696-705:
Simplified navigation logic in setPreviewModeInitSaga.

The removal of delay and history.push calls simplifies the function, potentially improving performance and maintainability. Ensure that the new implementation meets all functional requirements.

However, verify that the function is correctly used throughout the codebase.

Verification successful

Let's correct the file type specification and re-run the script to verify the usage of the setPreviewModeInitSaga function throughout the codebase.


Simplified navigation logic in setPreviewModeInitSaga.

The removal of delay and history.push calls simplifies the function, potentially improving performance and maintainability. The function is correctly referenced and used in app/client/src/ee/sagas/PageSagas.tsx.

  • app/client/src/ce/sagas/PageSagas.tsx: Definition of setPreviewModeInitSaga.
  • app/client/src/ee/sagas/PageSagas.tsx: References to setPreviewModeInitSaga in takeEvery and takeLatest.

The new implementation meets the functional requirements.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `setPreviewModeInitSaga` match the new implementation.

# Test: Search for the function usage. Expect: Only occurances of the new implementation.
rg --type tsx -A 5 $'setPreviewModeInitSaga'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify all function calls to `setPreviewModeInitSaga` match the new implementation.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type-add 'tsx:*.{ts,tsx}' --type tsx -A 5 $'setPreviewModeInitSaga'

Length of output: 1536


696-696: Verify the function usage of createPageSaga.

Ensure that the changes in setPreviewModeInitSaga do not affect the createPageSaga function.

Verification successful

Let's correct the file type issue and rerun the search to verify the usage of the createPageSaga function.


Let's search for the usage of setPreviewModeInitSaga to ensure there are no dependencies or side effects that impact createPageSaga.


Verified: The function createPageSaga is not affected by changes in setPreviewModeInitSaga.

The search results indicate that createPageSaga and setPreviewModeInitSaga operate independently, with no direct dependencies or side effects between them.

  • createPageSaga is used in takeLeading effects.
  • setPreviewModeInitSaga is used in takeEvery effects.

Both functions handle different Redux action types, ensuring no interference.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `createPageSaga` to ensure no side effects from changes in `setPreviewModeInitSaga`.

# Test: Search for the function usage. Expect: No side effects from changes in `setPreviewModeInitSaga`.
rg --type tsx -A 5 $'createPageSaga'

Length of output: 63


Script:

#!/bin/bash
# Description: Verify all function calls to `createPageSaga` to ensure no side effects from changes in `setPreviewModeInitSaga`.

# Test: Search for the function usage. Expect: No side effects from changes in `setPreviewModeInitSaga`.
rg --type ts -A 5 $'createPageSaga'

Length of output: 1273


Script:

#!/bin/bash
# Description: Verify all function calls to `setPreviewModeInitSaga` to ensure no side effects on `createPageSaga`.

# Test: Search for the function usage. Expect: No side effects on `createPageSaga`.
rg --type ts -A 5 $'setPreviewModeInitSaga'

Length of output: 1507


696-696: Verify the function usage of fetchPublishedPageSaga.

Ensure that the changes in setPreviewModeInitSaga do not affect the fetchPublishedPageSaga function.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 809cd30 and d97752b.

Files selected for processing (1)
  • app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx

@hetunandu hetunandu added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 24, 2024
@hetunandu hetunandu added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 31, 2024
@albinAppsmith
Copy link
Collaborator

/build-deploy-preview skip-test=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10174895566.
Workflow: On demand build Docker image and deploy preview.
skip-tests: . env: .
PR: 35114.
recreate: .

Copy link

Deploy-Preview-URL: https://ce-35114.dp.appsmith.com

@hetunandu hetunandu added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 31, 2024
@hetunandu
Copy link
Member Author

/build-deploy-preview skip-test=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10175706992.
Workflow: On demand build Docker image and deploy preview.
skip-tests: . env: .
PR: 35114.
recreate: .

@hetunandu
Copy link
Member Author

/build-deploy-preview recreate=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10175719704.
Workflow: On demand build Docker image and deploy preview.
skip-tests: . env: .
PR: 35114.
recreate: true.

Copy link

Deploy-Preview-URL: https://ce-35114.dp.appsmith.com

@hetunandu hetunandu added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 31, 2024
@hetunandu
Copy link
Member Author

/build-deploy-preview recreate=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10177802100.
Workflow: On demand build Docker image and deploy preview.
skip-tests: . env: .
PR: 35114.
recreate: true.

Copy link

Deploy-Preview-URL: https://ce-35114.dp.appsmith.com

@hetunandu hetunandu merged commit e0d0ee1 into release Jul 31, 2024
98 of 220 checks passed
@hetunandu hetunandu deleted the chore/reorg-panes branch July 31, 2024 15:53
hetunandu pushed a commit that referenced this pull request Aug 12, 2024
…menu (#35609)

## Description
There was an IDE PR : #35114
, that got merged which added the `overflow: hidden` property to the
entity explorer which made the show bindings menu disappear from the
view port. Hence, that change is fixed with this PR.


Fixes #35584
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.IDE"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10347893824>
> Commit: 05dd349
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10347893824&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.IDE`
> Spec:
> <hr>Mon, 12 Aug 2024 08:12:07 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **User Interface Changes**
- Adjusted the rendering behavior of the editor pane, allowing overflow
content to be visible, which may enhance the user experience within the
IDE.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@coderabbitai coderabbitai bot mentioned this pull request Oct 30, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Anvil Pod Issue related to Anvil project Bug Something isn't working High This issue blocks a user from building or impacts a lot of users IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE Needs Triaging Needs attention from maintainers to triage ok-to-test Required label for CI Production skip-changelog Adding this label to a PR prevents it from being listed in the changelog Task A simple Todo
Projects
None yet
3 participants