-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-5527] fix: extended sidebar #8197
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
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdates to the extended project sidebar: adds a className prop and collapse-driven close behavior to the wrapper, switches visibility from translate to opacity, and adjusts inner sidebar styling, padding, button classes, and EmptyStateCompact props. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,mts,cts}📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-10-21T17:22:05.204ZApplied to files:
📚 Learning: 2025-09-02T08:14:49.260ZApplied to files:
📚 Learning: 2025-10-09T20:42:31.843ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses two bugs in the extended sidebar functionality: ensuring the extended sidebar closes when the main sidebar is collapsed via keyboard shortcut, and fixing scrollbar positioning through padding adjustments.
Key changes:
- Added
useEffecthook to automatically close extended sidebar when main sidebar collapses - Restructured padding system using className prop to allow fine-grained control over child component spacing
- Removed transform-based animations in favor of opacity transitions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extended-sidebar-wrapper.tsx | Added useEffect to sync extended sidebar state with main sidebar collapse, introduced className prop for padding customization, removed translate transforms and dynamic width in favor of simpler opacity-based visibility |
| extended-project-sidebar.tsx | Adjusted padding structure by overriding wrapper padding and applying specific padding to child elements (header and scrollable list) to fix scrollbar positioning, added hover transition styles to add button |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx
Show resolved
Hide resolved
| import { cn } from "@plane/utils"; | ||
| // hooks | ||
| import { useAppTheme } from "@/hooks/store/use-app-theme"; | ||
| // hooks |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate comment "// hooks" on lines 7 and 9. Consider removing one of these duplicate comments to improve code readability.
| // hooks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx (1)
8-9: Remove duplicate comment.Line 9 contains a duplicate
// hookscomment that already appears on line 7.Apply this diff to remove the duplicate:
import { useAppTheme } from "@/hooks/store/use-app-theme"; -// hooks import useExtendedSidebarOutsideClickDetector from "@/hooks/use-extended-sidebar-overview-outside-click";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-project-sidebar.tsx(3 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/extended-project-sidebar.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx
🧬 Code graph analysis (1)
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (2)
packages/i18n/src/store/index.ts (1)
t(223-244)packages/constants/src/event-tracker/core.ts (1)
PROJECT_TRACKER_ELEMENTS(51-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (6)
105-105: LGTM! Padding refactor enables proper scrollbar positioning.Removing horizontal padding from the wrapper and delegating it to individual sections allows better control of the scrollbar position, which aligns with the PR objective of fixing the scrollbar placement.
107-107: LGTM! Horizontal padding added to header section.The change from
pt-0topx-4adds appropriate horizontal spacing to the header, working in tandem with the wrapper'spx-0to achieve proper scrollbar positioning.
115-115: LGTM! Enhanced button hover states.The added text colors and transition classes improve the visual feedback for the Add Project button, providing a better user experience.
137-137: LGTM! Simplified empty state padding.The change from
px-6 pt-10top-10provides uniform padding, creating a more balanced empty state layout.
141-143: LGTM! Enhanced empty state configuration.The additional props (
assetKey,assetClassName,align) improve the presentation of the empty search results state with a properly sized and centered search icon.
147-147: LGTM! Padding adjustment fixes scrollbar position.The added horizontal padding (
pl-9 pr-2) to the projects list container directly addresses the PR objective of correcting the scrollbar position.apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx (5)
1-1: LGTM! Required import for new effect.The
useEffectimport is correctly added to support the new sidebar closing behavior.
13-13: LGTM! Extensibility improvement.The optional
classNameprop enables external styling customization, which is used effectively in the consuming component to control padding.
24-25: LGTM! Enables sidebar closing behavior.The
sidebarCollapsedstate is correctly extracted fromuseAppThemeto implement the bug fix where the extended sidebar should close when the main sidebar collapses.
41-46: LGTM! Opacity-based visibility with custom className support.The change from translate-based to opacity-based visibility combined with the
hiddenclass provides proper accessibility behavior. The customclassNameprop is correctly merged usingcn(), enabling the padding customization used in the parent component.
50-50: No layout issues - the fixed width implementation is correct and intentional for smooth transitions.The sidebar's fixed width is not a problem. When
isExtendedSidebarOpenedisfalse, thehiddenclass (Tailwind'sdisplay: none) removes the element from the document layout flow entirely, preventing any spacing issues. The fixed width combined with the opacity-based visibility (opacity-100/opacity-0) enables smooth CSS transitions. The absolute positioning also ensures the sidebar doesn't affect the normal layout flow when visible.
Description
This PR fixes the following bugs related to the extended sidebar-
Type of Change
Summary by CodeRabbit
Style
Bug Fixes
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.