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: Split library side pane for adding package control section #36926

Merged
merged 6 commits into from
Oct 22, 2024

Conversation

ashit-rath
Copy link
Contributor

@ashit-rath ashit-rath commented Oct 17, 2024

Description

Purpose of this PR is to split the Libraries section to extend it in EE and introduced a tab layout that has both js libraries and packages
Changes:

  1. Increased width of Libraries pane to 384px
  2. Added a route /package which opens the same as library
  3. Extracted the JSLibraries section to be reused as a tab panel body in EE
  4. Created SidePaneWrapper common component to be reused in EE
  5. Split the title of the Libraries section under a hook to return empty string if package feature flag is enabled.
  6. Minor linting refactors.

In CE there is only one visual change i.e the increase in width of the Libraries side pane from 250px to 384px

Figma

PR for https://github.com/appsmithorg/appsmith-ee/pull/5362

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/11434291701
Commit: ec76cc5
Cypress dashboard.
Tags: @tag.All
Spec:


Mon, 21 Oct 2024 08:54:31 UTC

Communication

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

  • Yes
  • No

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced new functions for constructing URLs for application libraries and packages.
    • Added new route constants and matching functions for improved routing of application packages and libraries.
    • Implemented a custom hook for dynamic header titles in the libraries section.
    • Added a new LibrarySidePane component to enhance the editor interface.
    • Introduced a new SidePaneWrapper component for better layout management.
  • Bug Fixes

    • Enhanced entity identification logic to recognize both libraries and packages.
  • Chores

    • Updated routing logic for improved maintainability.
    • Added constants for configurable UI component widths.

These updates aim to enhance user experience and improve the overall functionality of the application.

@ashit-rath ashit-rath self-assigned this Oct 17, 2024
@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Oct 17, 2024
@ashit-rath
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

Copy link

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

Copy link

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

@ashit-rath ashit-rath added the ok-to-test Required label for CI label Oct 21, 2024
Copy link
Contributor

coderabbitai bot commented Oct 21, 2024

Walkthrough

This pull request introduces several enhancements to the application’s routing and component structure. It adds new functions for constructing URLs related to libraries and packages, introduces new route constants and matching functions, and modifies existing components to utilize these new functionalities. Additionally, it introduces new constants for UI layout and enhances error handling in the application. The changes are aimed at improving the organization and flexibility of the codebase, particularly in relation to the libraries and packages features.

Changes

File Path Change Summary
app/client/src/ce/RouteBuilder.ts Added functions: appLibrariesURL and appPackagesURL for constructing URLs.
app/client/src/ce/constants/routes/appRoutes.ts Added constant: APP_PACKAGES_EDITOR_PATH, functions: matchAppPackagesPath and matchAppLibrariesPath.
app/client/src/ce/entities/Engine/actionHelpers.ts Updated getPageDependencyActions to include a new parameter applicationId.
app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts Added a custom hook useLibraryHeaderTitle for generating library header titles.
app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx Introduced a new component LibrarySidePane for rendering the library section.
app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts Added constant APP_PACKAGES_EDITOR_PATH to the routing logic.
app/client/src/constants/AppConstants.ts Added constant APP_LIBRARIES_PANE_WIDTH for UI component width.
app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts Re-exported all exports from ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.
app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx Re-exported all exports from ce/pages/Editor/IDE/LeftPane/LibrarySidePane.
app/client/src/entities/Engine/AppEditorEngine.ts Updated methods loadPluginsAndDatasources and loadAppEntities to include applicationId.
app/client/src/navigation/FocusEntity.ts Modified identifyEntityFromPath to recognize both libraries and packages.
app/client/src/pages/Editor/IDE/Header/index.tsx Integrated useLibraryHeaderTitle hook into HeaderTitleComponent for dynamic title rendering.
app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts Utilized APP_LIBRARIES_PANE_WIDTH to set the libraries pane width dynamically.
app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx Renamed LibrarySidePane to JSLibrariesSection and updated its declaration.
app/client/src/pages/Editor/IDE/LeftPane/index.tsx Added useMemo for path definitions and updated routing logic for DataSidePane and LibrarySidePane.
app/client/src/IDE/Components/SidePaneWrapper.tsx Introduced SidePaneWrapper component for side pane layout with conditional padding.
app/client/src/IDE/index.ts Added export for IDESidePaneWrapper component from SidePaneWrapper.

Possibly related PRs

Suggested labels

Enhancement, Task, IDE Product, IDE Pod

Suggested reviewers

  • albinAppsmith
  • hetunandu
  • ankitakinger

🎉 In the realm of code, new paths we pave,
With libraries and packages, our structure is brave.
Functions and constants, all neatly aligned,
A journey of coding, where clarity we find.
So let’s celebrate changes, both big and small,
In this world of development, we’re having a ball! 🎈


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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

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: 4

🧹 Outside diff range and nitpick comments (9)
app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx (1)

5-11: Excellent work on your component structure!

Your LibrarySidePane component is well-organized and follows React best practices. However, let's make it even better!

Consider adding a prop types definition to improve code readability and maintainability. Here's an example:

import { FC } from 'react';

interface LibrarySidePaneProps {
  // Add any props here if needed in the future
}

const LibrarySidePane: FC<LibrarySidePaneProps> = () => {
  // ... component body
};

This change will make it easier to add props in the future if needed.

app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts (1)

8-10: Let's take a closer look at our function implementation, students.

The useLibraryHeaderTitle function is well-implemented and aligns nicely with its description. Good job on using a named function - it'll make debugging easier if needed.

However, I have a small suggestion for you to ponder: Since this function doesn't use any React hooks or state, have you considered making it a regular function instead of a custom hook? It might be more appropriate unless you plan to add hook-specific logic in the future.

app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx (2)

22-29: Good job simplifying the component structure!

Class, let's examine the changes in the return statement:

  1. The Flex component has been removed, simplifying our structure. This is good if we don't need any specific flex layout properties.

  2. A fragment (<>...</>) is now used instead. This is an excellent practice when we don't need an extra DOM node.

  3. The rightIcon is now passed to PaneHeader as a prop, which is a good example of component composition.

These changes make our code cleaner and more efficient. However, I have a small suggestion:

Consider adding a descriptive comment above the transitions mapping to explain its purpose. This will help future developers (including yourself!) understand the code more quickly.

For example:

{/* Render library entities with fade-in animation */}
{transitions((style, lib) => (
  // ... existing code
))}

Keep up the great work!


Line range hint 1-33: Excellent job maintaining core functionality while improving structure!

Class, let's take a moment to appreciate the overall structure of this component:

  1. The core logic, including the use of useSelector to get libraries and useTransition for animations, remains intact. This shows good judgment in preserving what works well.

  2. The improvements we've discussed earlier (renaming, restructuring, and optimizing) have been made without disrupting the main functionality.

This is a great example of how to refactor code: improve what you can while ensuring the core functionality remains stable.

For a future improvement, consider this:

As our application grows, we might want to consider extracting the library data fetching logic into a custom hook. This could look something like:

function useLibraries() {
  return useSelector(selectLibrariesForExplorer);
}

This would further separate concerns and make our component even more focused on rendering. It would also make it easier to reuse this logic in other components if needed.

Keep up the excellent work, and always look for ways to improve our code structure!

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

86-86: Well done on adding the new path, but let's tidy up our classroom a bit!

The addition of ${path}${APP_PACKAGES_EDITOR_PATH} to the "Canvas" route paths is correct and follows the existing pattern. It's like adding a new destination to our field trip list!

However, class, let's make our code more readable. Can you add a line break before this new path? It will help us keep our code neat and tidy, just like our classroom!

Here's a small suggestion to improve readability:

 `${path}${APP_LIBRARIES_EDITOR_PATH}`,
+
 `${path}${APP_PACKAGES_EDITOR_PATH}`,
 `${path}${APP_SETTINGS_EDITOR_PATH}`,
app/client/src/ce/RouteBuilder.ts (1)

212-215: Excellent work! Now, let's discuss this function.

The appPackagesURL function is another stellar example of URL construction. It's like a twin to appLibrariesURL, isn't it? However, class, can you spot an opportunity for improvement?

Here's a little homework for you:

Consider grouping these two new functions together, right after the appLibrariesURL function. It's like keeping your pencils and pens in the same pencil case – it makes everything easier to find!

app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts (1)

100-104: Excellent work on updating the Libraries pane width, students!

You've successfully replaced the hardcoded value with the new constant APP_LIBRARIES_PANE_WIDTH. This change improves maintainability and aligns with our lesson on using constants for configuration.

However, let's take this opportunity for a small improvement. Can anyone tell me how we might make this code even more readable?

Consider extracting the calculation for the main editor area width into a separate variable for improved readability. Here's a suggestion:

const mainEditorWidth = windowWidth - APP_SIDEBAR_WIDTH - APP_LIBRARIES_PANE_WIDTH;
setColumns([
  SIDEBAR_WIDTH,
  `${APP_LIBRARIES_PANE_WIDTH}px`,
  `${mainEditorWidth}px`,
  "0px",
]);

This change would make the code easier to understand at a glance. Who would like to implement this improvement?

app/client/src/navigation/FocusEntity.ts (1)

281-284: Good job extending the entity handling, class! Let's make it even better!

Your changes to include the packages entity alongside libraries are on the right track. This modification aligns well with our lesson plan of splitting the library side pane and introducing a package control section.

To improve readability, consider using an array and the includes method. Here's a gold star suggestion:

if (["libraries", "packages"].includes(match.params.entity)) {
  // ... existing code ...
}

This approach makes it easier to add more entities in the future without cluttering the condition. Remember, clean code is happy code!

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

96-97: Excellent use of the new hook, students!

You've correctly implemented the useLibraryHeaderTitle hook to dynamically generate the library header title. This is a great improvement for flexibility.

To enhance readability, consider adding a comment explaining the purpose of this hook:

// Use the custom hook to generate a dynamic title for the library section
const libraryHeaderTitle = useLibraryHeaderTitle();

This will help future developers understand the intention behind using this hook.

Also applies to: 116-116

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 81b2146 and 2d96cc6.

📒 Files selected for processing (16)
  • app/client/src/ce/RouteBuilder.ts (1 hunks)
  • app/client/src/ce/constants/routes/appRoutes.ts (2 hunks)
  • app/client/src/ce/entities/Engine/actionHelpers.ts (1 hunks)
  • app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts (1 hunks)
  • app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx (1 hunks)
  • app/client/src/ce/pages/Editor/IDE/MainPane/useRoutes.ts (2 hunks)
  • app/client/src/constants/AppConstants.ts (1 hunks)
  • app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts (1 hunks)
  • app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx (1 hunks)
  • app/client/src/entities/Engine/AppEditorEngine.ts (3 hunks)
  • app/client/src/navigation/FocusEntity.ts (1 hunks)
  • app/client/src/pages/Editor/IDE/Header/index.tsx (3 hunks)
  • app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts (2 hunks)
  • app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx (2 hunks)
  • app/client/src/pages/Editor/IDE/LeftPane/index.tsx (3 hunks)
  • app/client/src/pages/common/SidePaneWrapper.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • app/client/src/ee/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts
  • app/client/src/ee/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx
🧰 Additional context used
🔇 Additional comments (30)
app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx (2)

1-3: Well done on your imports, class!

You've demonstrated a good understanding of organizing your imports. Keep up the excellent work!


13-13: A+ on your export statement!

Your default export is spot-on. It matches the file name and follows best practices. Keep up the great work!

app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts (2)

3-7: Excellent job on your documentation, class!

Your comment here is very informative. It clearly explains the purpose of the hook and its relationship with the Enterprise Edition. This kind of clear documentation helps everyone understand the code better. Keep up the good work!


12-12: Well done on your export statement!

Your default export here is perfectly appropriate for a single function. It's clear and follows common practices. Keep up the good work!

app/client/src/pages/common/SidePaneWrapper.tsx (5)

1-4: Excellent work on the imports, class!

You've done a splendid job organizing your imports. They're all present and accounted for, just like a well-prepared student on the first day of school!


6-9: A gold star for your interface definition!

Your SidePaneContainerProps interface is as clear as a well-written chalkboard. The optional props give flexibility to the component users, just like how we offer extra credit assignments!


11-13: Your styled component gets an A+!

The StyledContainer is as well-crafted as a perfectly organized classroom. Your use of conditional padding is like adjusting the seating arrangement based on class size - very adaptable!


15-27: Your SidePaneWrapper component deserves a spot on the honor roll!

This component is structured as neatly as a well-organized lesson plan. The default value for padded is like having a backup activity ready - always prepared! Your use of flex properties ensures the component fills its space efficiently, just like how we arrange desks to maximize classroom space.


29-29: Your export statement passes with flying colors!

Exporting SidePaneWrapper as the default is like dismissing class on time - it's the right thing to do and everyone appreciates it!

app/client/src/pages/Editor/IDE/LeftPane/JSLibrariesSection.tsx (3)

1-10: Well done on improving the import statements and component declaration!

Class, let's take a moment to appreciate the positive changes made here:

  1. The import statements now use absolute paths, which is a good practice. It makes our code more maintainable and less prone to issues when files are moved around.

  2. The component has been aptly renamed from LibrarySidePane to JSLibrariesSection. This new name is more descriptive and better reflects the component's purpose.

  3. The component declaration has been changed from an arrow function to a regular function declaration. While both are valid, this change aligns with common React practices.

Keep up the good work!


19-20: Excellent use of useMemo for performance optimization!

Students, pay attention to this clever use of the useMemo hook:

const rightIcon = useMemo(() => <AddLibraryPopover />, []);

This change memoizes the AddLibraryPopover component, which means it will only be re-created when its dependencies change (in this case, never, since the dependency array is empty). This can help prevent unnecessary re-renders and improve the performance of our application.

Moreover, this separation of concerns makes the code more readable and maintainable. Well done!


33-33: Correct update to the export statement!

Good job, class! You've correctly updated the export statement to match the new component name:

export default JSLibrariesSection;

This change ensures that other parts of our application can correctly import and use this component. It's a small but crucial detail that maintains the consistency of our codebase. Well done!

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

1-1: Class, let's examine these import changes.

Good job on updating the imports! The addition of useMemo shows you're thinking about optimization, which is excellent. The new APP_PACKAGES_EDITOR_PATH import suggests we're expanding our feature set. And moving LibrarySidePane to a new location? That's a smart organizational move.

Keep up the good work! These changes lay a solid foundation for the rest of your code.

Also applies to: 7-7, 17-17


30-38: Let's discuss this new useMemo hook, class.

Excellent use of useMemo here! You've created a memoized array of data side pane paths, which is a smart optimization. This means the array won't be recreated on every render unless path changes.

By centralizing these paths, you've made our code more maintainable. If we need to add or modify paths in the future, we only need to update this one spot. That's thinking ahead!

Remember, though: with great power comes great responsibility. Use useMemo wisely, as overuse can sometimes lead to more complexity than it's worth.


40-46: Now, let's turn our attention to the librarySidePanePaths hook.

Well done! You've applied the same useMemo pattern here as you did with dataSidePanePaths. Consistency in coding is like consistency in grading - it makes everything clearer and fairer.

I see you've included both libraries and packages paths in this array. This unified approach suggests you're thinking about how these features relate to each other. That's the kind of big-picture thinking we like to see!

Just like before, you've correctly included path in the dependency array. You're showing a good understanding of how to use useMemo effectively.


51-51: Let's examine this update to the SentryRoute for DataSidePane.

Excellent simplification here! By using the dataSidePanePaths array instead of inline path definitions, you've made this component much cleaner and easier to maintain.

This change is like using a well-organized lesson plan instead of scattered notes. It makes your code more readable and less prone to errors. Plus, by leveraging the memoized paths array, you're ensuring consistent performance.

Keep up this kind of thoughtful refactoring!


52-56: Finally, let's look at the changes to the LibrarySidePane route.

Wonderful job maintaining consistency! Just like with the DataSidePane route, you've updated this one to use the memoized librarySidePanePaths array.

This change is like using the same formatting for all your assignments - it makes everything easier to understand and grade. It also means that if you need to add more library-related routes in the future, you can do so easily by updating the librarySidePanePaths array.

Your consistent approach here shows a good understanding of the principles we've been learning. Keep up the excellent work!

app/client/src/ce/entities/Engine/actionHelpers.ts (1)

Line range hint 1-114: Class, let's recap our lesson on code changes.

Today, we learned about adding new parameters to functions. We saw this in action with the getPageDependencyActions function.

Remember, when we make changes like this, it's important to:

  1. Ensure the change is consistent throughout our "textbook" (codebase).
  2. Update any "homework assignments" (unit tests) that might be affected.
  3. Check if our "study group" (other developers) needs to know about this change.

For our final exercise, let's verify if this change affects any other parts of our codebase:

#!/bin/bash
# Let's search for all uses of getPageDependencyActions
echo "Searching for uses of getPageDependencyActions:"
rg "getPageDependencyActions\(" --type ts

Class, after running this verification, please review the results and ensure all calls to getPageDependencyActions are updated with the new applicationId parameter.

Any questions before we end our lesson?

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

7-7: Good job adding the new import, class!

The addition of APP_PACKAGES_EDITOR_PATH to the import list is correct and follows the existing pattern. It's like adding a new book to our library shelf, neat and orderly!

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

12-12: Excellent addition, class! Let's discuss the new constant.

Students, pay attention to this new constant APP_LIBRARIES_PANE_WIDTH. It's a fine example of how we define our application's layout parameters. Notice how it follows our naming convention and is exported for use throughout our project.

Now, can anyone tell me why we might choose 384 pixels for our libraries pane? That's right, it's significantly wider than our other panes! This gives us more room to display our libraries, making them easier to read and work with. Remember, in UI design, every pixel counts!

app/client/src/ce/RouteBuilder.ts (1)

208-211: Well done, class! Let's examine this new function.

The appLibrariesURL function is a fine addition to our URL-building toolkit. It constructs a URL for accessing application libraries, following the same pattern as its classmates. Remember, consistency is key in coding!

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

18-18: Well done, class! You've imported the new constant correctly.

The import statement for APP_LIBRARIES_PANE_WIDTH is properly placed and follows the established naming convention. Keep up the good work!


Line range hint 1-164: Class, let's review what we've learned today!

In this lesson on updating the useGridLayoutTemplate hook, we've seen:

  1. The introduction of a new constant APP_LIBRARIES_PANE_WIDTH.
  2. Proper use of this constant in the EditorState.LIBRARIES case.
  3. Correct adjustment of calculations for the main editor area width.

These changes align well with our objective of making the Libraries pane width configurable and increasing its size. The code is now more maintainable and flexible.

For homework, I'd like you all to think about other areas in the application where we might apply similar improvements. Are there other hardcoded values that could benefit from being turned into constants?

Great job, everyone! Class dismissed.

app/client/src/ce/constants/routes/appRoutes.ts (4)

70-70: Well done, class! A new constant has been added.

The addition of APP_PACKAGES_EDITOR_PATH is a good step towards organizing our routes. This constant will be very useful for navigating to the packages section of our application. Remember, clear and consistent naming is key to maintaining a tidy codebase!


132-133: Excellent work on creating a new matching function!

The matchAppLibrariesPath function is a great addition to our routing toolkit. It will help us identify when a user is navigating to the libraries section. This is particularly useful for the planned extensions in the Enterprise Edition. Keep up the good work in making our routing more robust!


135-136: A gold star for adding another matching function!

The matchAppPackagesPath function is a wonderful complement to our new packages route. It will be instrumental in determining when a user is accessing the packages section of our application. This function, along with the new constant, forms a complete set for handling package-related routing. Excellent job in maintaining consistency with our existing routing patterns!


70-70: Class, let's recap our lesson on routing today!

We've made some fantastic additions to our routing system:

  1. A new constant APP_PACKAGES_EDITOR_PATH for our packages route.
  2. Two new matching functions: matchAppLibrariesPath and matchAppPackagesPath.

These changes are like adding new chapters to our routing textbook. They will help us navigate our application more efficiently, especially when we start working on the packages and libraries sections. Remember, good routing is the map that guides users through our application. Keep up the excellent work in making our code more organized and user-friendly!

Also applies to: 132-136

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

80-80: Good job importing the new hook, class!

The import statement for useLibraryHeaderTitle is well-placed and follows proper naming conventions. This new hook will help us dynamically generate the header title for the library section.

app/client/src/entities/Engine/AppEditorEngine.ts (2)

215-220: Good Use of applicationId in Dependency Actions

Including applicationId when calling getPageDependencyActions ensures that dependencies are accurately fetched for the specific application context. This enhances the precision of your data handling.


268-273: Updating Method Calls Consistently

You've correctly updated the call to loadPluginsAndDatasources to include the new applicationId parameter. This consistency is crucial for the proper functioning of your application's loading process.

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 and nitpick comments (1)
app/client/src/IDE/Components/SidePaneWrapper.tsx (1)

15-27: Great job on your SidePaneWrapper component!

Your component structure is clear and concise. The use of default prop values and appropriate styling is commendable. However, let's make it even better!

Consider adding a comment explaining the purpose of this component. It will help your classmates understand its role in the larger application. For example:

/**
 * SidePaneWrapper: A reusable component for creating side panes in the IDE.
 * It provides consistent styling and optional padding for its children.
 */
function SidePaneWrapper({ children, padded = false }: SidePaneContainerProps) {
  // ... rest of the component
}

Remember, good documentation is as important as good code!

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2d96cc6 and 3639094.

📒 Files selected for processing (3)
  • app/client/src/IDE/Components/SidePaneWrapper.tsx (1 hunks)
  • app/client/src/IDE/index.ts (1 hunks)
  • app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/client/src/ce/pages/Editor/IDE/LeftPane/LibrarySidePane.tsx
🧰 Additional context used
🔇 Additional comments (5)
app/client/src/IDE/Components/SidePaneWrapper.tsx (4)

1-4: Well done on your imports, class!

You've correctly imported all the necessary modules and types. It's good to see you're using type checking with the ReactNode import. Keep up the good work!


6-9: Excellent interface definition, students!

Your SidePaneContainerProps interface is well-structured. Making both properties optional is a smart choice, allowing for flexible use of the component. Remember, clear interfaces make for easier collaboration!


11-13: A+ for your styled component, class!

Your StyledContainer is a great example of using styled-components. The conditional padding based on the padded prop shows good thinking about component flexibility. Remember, responsive design is key in modern web development!


29-29: Correct export statement, class!

Your default export of the SidePaneWrapper component is spot on. This makes it easy for other parts of the application to import and use this component. Well done!

app/client/src/IDE/index.ts (1)

57-61: Well done, class! This addition is exemplary.

I'm pleased to see such a well-documented and properly structured export. The comment clearly explains the purpose and functionality of the IDESidePaneWrapper component. This is precisely the kind of thorough work we expect in our codebase.

Remember, class, good documentation is like a well-organized backpack - it helps everyone find what they need quickly and easily!

@ashit-rath ashit-rath dismissed sagar-qa007’s stale review October 22, 2024 04:51

Responded to changes requested, no further action needed.

@ashit-rath ashit-rath merged commit 67046f8 into release Oct 22, 2024
83 checks passed
@ashit-rath ashit-rath deleted the chore/split-for-pkg-version-control branch October 22, 2024 05:18
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Nov 20, 2024
…psmithorg#36926)

## Description
Purpose of this PR is to split the Libraries section to extend it in EE
and introduced a tab layout that has both js libraries and packages
Changes:
1. Increased width of Libraries pane to 384px
2. Added a route `/package` which opens the same as library 
3. Extracted the JSLibraries section to be reused as a tab panel body in
EE
4. Created `SidePaneWrapper` common component to be reused in EE
5. Split the title of the Libraries section under a hook to return empty
string if package feature flag is enabled.
6. Minor linting refactors.

In CE there is only one visual change i.e the increase in width of the
Libraries side pane from 250px to 384px


[Figma](https://www.figma.com/design/Z0QsSdGOydURn6WIMQ3rHM/Appsmith-Reusability?node-id=6816-319511&node-type=frame&t=x0DzN9HIxjefe3J1-0)

PR for appsmithorg/appsmith-ee#5362
## Automation

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

### 🔍 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/11434291701>
> Commit: ec76cc5
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11434291701&attempt=3"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Mon, 21 Oct 2024 08:54:31 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

## Release Notes

- **New Features**
- Introduced new functions for constructing URLs for application
libraries and packages.
- Added new route constants and matching functions for improved routing
of application packages and libraries.
- Implemented a custom hook for dynamic header titles in the libraries
section.
- Added a new `LibrarySidePane` component to enhance the editor
interface.
- Introduced a new `SidePaneWrapper` component for better layout
management.

- **Bug Fixes**
- Enhanced entity identification logic to recognize both libraries and
packages.

- **Chores**
  - Updated routing logic for improved maintainability.
  - Added constants for configurable UI component widths.

These updates aim to enhance user experience and improve the overall
functionality of the application.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants