[WEB-2108] refactor: enhance workspace and project wrapper modularity #6207
Conversation
WalkthroughThis pull request introduces several changes across the Plane web application's layout and authentication components. The modifications primarily involve reorganizing layout wrappers, updating import paths, and enhancing loading state management for workspace and project authentication components. A new modal width enum value is also added to the UI constants. The changes appear to be part of a broader refactoring effort to improve code structure and component flexibility. Changes
Sequence DiagramsequenceDiagram
participant Parent as Parent Component
participant Wrapper as Auth Wrapper
participant Core as Core Auth Logic
Parent->>Wrapper: Render with children
Wrapper->>Wrapper: Check loading state
alt Is Loading
Wrapper-->>Parent: Render Loading Spinner
else Not Loading
Wrapper->>Core: Validate Authentication
Core-->>Wrapper: Return Authentication Status
Wrapper-->>Parent: Render Children
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
packages/ui/src/modals/constants.ts (1)
16-16: Consider adding documentation for modal width use cases.The new
VIIXLenum value follows the existing pattern correctly. However, it would be helpful to document when each modal width should be used.Consider adding a comment block above the enum explaining the intended use case for each size:
export enum EModalWidth { + // SM (max-w-sm): Use for simple confirmation dialogs + // MD (max-w-md): Use for forms with few fields + // ... + // VIIXL (max-w-7xl): Use for full-width workspace views VIIXL = "sm:max-w-7xl",web/ce/layouts/project-wrapper.tsx (1)
6-8: Consider simplifying the wrapper if no additional functionality is needed.The interface currently only includes
children. If no additional props or functionality will be added, consider whether this extra wrapper layer is necessary.web/ce/layouts/workspace-wrapper.tsx (1)
10-15: Consider adding workspace-specific logicThe current implementation is a simple pass-through wrapper. Consider adding workspace-specific logic such as:
- Workspace-level permission checks
- Workspace data pre-fetching
- Workspace-specific error boundaries
web/core/layouts/auth-layout/project-wrapper.tsx (1)
41-41: Consider extracting complex loading condition to a derived valueThe loading condition combines multiple checks. Consider improving readability by extracting it to a derived value:
- if (isParentLoading || (!projectMemberInfo && projectId && hasPermissionToCurrentProject === null)) + const isLoadingState = isParentLoading || (!projectMemberInfo && projectId && hasPermissionToCurrentProject === null); + if (isLoadingState)Also applies to: 155-155
web/core/layouts/auth-layout/workspace-wrapper.tsx (1)
122-122: Consider consistent loading state management across wrappersFor consistency with the suggested improvement in project-wrapper.tsx, consider extracting the loading condition:
- if (isParentLoading || allWorkspaces === undefined || loader || isDBInitializing) { + const isLoadingState = isParentLoading || allWorkspaces === undefined || loader || isDBInitializing; + if (isLoadingState) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/ui/src/modals/constants.ts(1 hunks)web/app/[workspaceSlug]/(projects)/layout.tsx(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/layout.tsx(1 hunks)web/ce/layouts/project-wrapper.tsx(1 hunks)web/ce/layouts/workspace-wrapper.tsx(1 hunks)web/core/components/workspace/settings/invitations-list-item.tsx(1 hunks)web/core/layouts/auth-layout/project-wrapper.tsx(3 hunks)web/core/layouts/auth-layout/workspace-wrapper.tsx(4 hunks)web/ee/layouts/project-wrapper.tsx(1 hunks)web/ee/layouts/workspace-wrapper.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/ee/layouts/workspace-wrapper.tsx
- web/ee/layouts/project-wrapper.tsx
🔇 Additional comments (3)
web/core/components/workspace/settings/invitations-list-item.tsx (1)
123-129: LGTM! Error handling improvement
The error message is now more specific and maintains proper error object handling.
web/core/layouts/auth-layout/project-wrapper.tsx (1)
37-37: LGTM: Interface enhancement improves component flexibility
The addition of the optional isLoading prop enhances the component's modularity by allowing parent components to control the loading state.
web/core/layouts/auth-layout/workspace-wrapper.tsx (1)
29-31:
Verify the impact of removing interface export
The export keyword has been removed from the interface declaration. If this interface is used in other files, they might be affected by this change.
There was a problem hiding this comment.
why not just move the whole thing to CE?
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
VIIXLfor modal width.ProjectAuthWrapperandWorkspaceAuthWrappercomponents to enhance layout functionality.ProjectAuthWrapperandWorkspaceAuthWrapper.WorkspaceInvitationsListItemfor better user feedback.