chore: Moving static layout styled components to a common file for re-using on EE#39379
Merged
ankitakinger merged 5 commits intoreleasefrom Feb 20, 2025
Merged
chore: Moving static layout styled components to a common file for re-using on EE#39379ankitakinger merged 5 commits intoreleasefrom
ankitakinger merged 5 commits intoreleasefrom
Conversation
Contributor
WalkthroughThe changes update the application's layout and library components. New styled components, Changes
Sequence Diagram(s)sequenceDiagram
participant LP as LeftPane
participant LSP as LibrarySidePane
participant JSL as JSLibrariesSection
participant ALP as AddLibraryPopover
LP->>LSP: Render with showAddButton prop
LSP->>JSL: Pass showAddButton prop
alt showAddButton is true
JSL->>ALP: Render AddLibraryPopover
else showAddButton is false
JSL->>JSL: Skip rendering AddLibraryPopover
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (60)
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
app/client/src/IDE/Components/LayoutComponents.tsx (2)
3-7: Consider using width: 100% instead of 100vwUsing
100vwfor width can cause horizontal scrollbars to appear unexpectedly when the vertical scrollbar is present, as it includes the scrollbar width in the calculation.- width: 100vw; + width: 100%;
9-12: Consider adding a Props interface for better type documentationThe inline prop type could be moved to a named interface for better reusability and documentation.
+interface LayoutContainerProps { + name: string; +} + -export const LayoutContainer = styled.div<{ name: string }>` +export const LayoutContainer = styled.div<LayoutContainerProps>`app/client/src/ce/pages/AppIDE/components/LibrariesList/LibrarySidePane.tsx (1)
5-7: Add Props interface and default prop valueConsider adding a Props interface and defaultProps to handle cases where showAddButton is not provided.
+interface LibrarySidePaneProps { + showAddButton: boolean; +} + -const LibrarySidePane = (props: { showAddButton: boolean }) => { +const LibrarySidePane = (props: LibrarySidePaneProps) => { const { showAddButton } = props; +LibrarySidePane.defaultProps = { + showAddButton: false, +};app/client/src/pages/AppIDE/components/LibrariesList/JSLibrariesSection.tsx (1)
10-11: Add Props interface for better type documentationConsider adding a Props interface for better code organization and documentation.
+interface JSLibrariesSectionProps { + showAddButton: boolean; +} + -function JSLibrariesSection(props: { showAddButton: boolean }) { +function JSLibrariesSection(props: JSLibrariesSectionProps) {app/client/src/pages/AppIDE/layouts/StaticLayout.tsx (2)
18-21: Group related imports togetherConsider grouping the layout component imports with other component imports for better code organization.
import Sidebar from "./routers/Sidebar"; import LeftPane from "./routers/LeftPane"; import MainPane from "./routers/MainPane"; import RightPane from "./routers/RightPane"; +import { + GridContainer, + LayoutContainer, +} from "IDE/Components/LayoutComponents"; import { ProtectedCallout } from "../components/ProtectedCallout"; import { useGridLayoutTemplate } from "./hooks/useGridLayoutTemplate"; import { Areas } from "./constants"; -import { - GridContainer, - LayoutContainer, -} from "IDE/Components/LayoutComponents";
38-38: Consider adding comparison function to React.memoThe component uses React.memo without a comparison function. Consider adding one to optimize re-renders if needed.
-export const StaticLayout = React.memo(() => { +export const StaticLayout = React.memo( + () => { // Component implementation + }, + (prevProps, nextProps) => { + // Add comparison logic if needed + return true; + }, +);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/IDE/Components/LayoutComponents.tsx(1 hunks)app/client/src/ce/pages/AppIDE/components/LibrariesList/LibrarySidePane.tsx(1 hunks)app/client/src/pages/AppIDE/AppIDE.tsx(1 hunks)app/client/src/pages/AppIDE/components/LibrariesList/JSLibrariesSection.tsx(2 hunks)app/client/src/pages/AppIDE/layouts/StaticLayout.tsx(1 hunks)app/client/src/pages/AppIDE/layouts/routers/LeftPane.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/pages/AppIDE/AppIDE.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (1)
app/client/src/pages/AppIDE/layouts/routers/LeftPane.tsx (1)
65-67: LGTM! Good use of render props pattern.The change from component prop to render prop allows passing the showAddButton prop while maintaining route functionality.
app/client/src/pages/AppIDE/components/LibrariesList/JSLibrariesSection.tsx
Show resolved
Hide resolved
alex-golovanov
approved these changes
Feb 20, 2025
github-actions
bot
added
IDE Pod
2 tasks
github-actions bot
pushed a commit
to Zeral-Zhang/appsmith
that referenced
this pull request
Apr 12, 2025
…-using on EE (appsmithorg#39379) ## Description - Moving static layout styled components to a common file for re-using on EE - Updating AppIDE folder name: `layout` to `layouts` - Showing the Add button for libraries via props to re-use the component on EE Fixes [appsmithorg#39037](appsmithorg#39037) ## 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/13436155470> > Commit: bafebbe > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13436155470&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 20 Feb 2025 14:41:02 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 ## Summary by CodeRabbit - **New Features** - Introduced an option in the libraries panel to conditionally display an “add” button, offering a more dynamic experience. - Added new layout components to facilitate better layout management within the application. - **Refactor** - Enhanced layout management through unified grid-based components for improved visual consistency. - Updated component interfaces to accept new props for enhanced functionality. - Adjusted import paths to reflect a restructured file organization. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
layouttolayoutsFixes #39037
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/13436155470
Commit: bafebbe
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 20 Feb 2025 14:41:02 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor