[WEB-4328] feat: sidebar revamp#7217
Conversation
WalkthroughThis update introduces a new resizable, collapsible, and peekable sidebar architecture, refactoring the sidebar's state management and UI. It removes all conditional rendering based on sidebar collapse, centralizes sidebar state in the theme store, and adds new components for sidebar resizing and extended sidebar handling. Tooltips and mobile-specific sidebar behaviors are removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectAppSidebar
participant ResizableSidebar
participant ThemeStore
User->>ProjectAppSidebar: Interact (resize/collapse/peek)
ProjectAppSidebar->>ThemeStore: Read/write sidebar state
ProjectAppSidebar->>ResizableSidebar: Pass state & handlers
ResizableSidebar->>ThemeStore: Notify on collapse/peek/resize
ResizableSidebar-->>User: Update UI (resize/peek/collapse)
sequenceDiagram
participant User
participant ExtendedSidebarWrapper
participant ThemeStore
User->>ExtendedSidebarWrapper: Click outside sidebar
ExtendedSidebarWrapper->>ThemeStore: Update isExtendedSidebarOpened = false
ExtendedSidebarWrapper-->>User: Hide extended sidebar
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
anmolsinghbhatia
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (1)
60-128: Effect dependency set can cause unnecessary re-subscriptions
elementRef?.currentis always the same reference for the component lifetime; including it re-runs the DnD registration on every render even when nothing relevant changed.-}, [elementRef?.current, isDragging, isLastChild, favorite.id]); +// elementRef.current is stable; handleDrop/parentId should be tracked instead +}, [isLastChild, favorite.id, handleDrop, parentId]);This avoids needless
combine()churn while ensuring closures stay fresh.web/app/(all)/[workspaceSlug]/(projects)/sidebar.tsx (1)
27-42: Outside-click detector no longer works –refis never attached
useOutsideClickDetector(ref, …)relies on the DOM node stored inref,
but after the refactor the root element withref={ref}was removed.
Result: sidebar can’t auto-close on outside clicks → broken UX on mobile.Add
ref={ref}to the top-level wrapper (or another encompassing element).-<div className="px-4"> +<div ref={ref} className="px-4">Don’t forget to export the correct typing if you wrap the sidebar in another component.
🧹 Nitpick comments (25)
web/core/components/sidebar/index.ts (1)
1-2: Wildcard re-exports may hide name collisionsUsing
export *from multiple modules can silently mask duplicate symbol names (e.g., two modules exposingSidebarContext). TypeScript will surface an error only if the symbols collide and are imported downstream withimport { … }.If both
sidebar-navigationandresizable-sidebarexport identically named helpers, the later one wins at runtime. Consider re-exporting explicitly to make any overlaps obvious:-export * from "./sidebar-navigation"; -export * from "./resizable-sidebar"; +export { SidebarNavigation } from "./sidebar-navigation"; +export { ResizableSidebar } from "./resizable-sidebar";web/ce/components/workspace/sidebar/app-search.tsx (1)
17-17: Minor accessibility enhancementWhile the unconditional border fixes the visual jump when the sidebar collapses, the button still lacks a
focus-visible:ringfor keyboard users.-className="flex-shrink-0 size-8 aspect-square grid place-items-center rounded hover:bg-custom-sidebar-background-90 outline-none border-[0.5px] border-custom-sidebar-border-300" +className="flex-shrink-0 size-8 aspect-square grid place-items-center rounded hover:bg-custom-sidebar-background-90 focus-visible:ring-2 focus-visible:ring-primary outline-none border-[0.5px] border-custom-sidebar-border-300"web/core/components/workspace-notifications/notification-app-sidebar-option.tsx (1)
37-38: String concatenation produces a leading spaceThe template literal
\@ ${count}`yields"@ 12"` (double space). Use trimming or drop the trailing space:-<CountChip count={`${isMentionsEnabled ? `@ ` : ``}${getNumberCount(totalNotifications)}`} /> +<CountChip count={`${isMentionsEnabled ? "@" : ""}${getNumberCount(totalNotifications)}`} />web/core/components/workspace/sidebar/workspace-menu-header.tsx (1)
34-37: Replace theanycast with a proper union to avoid losing type-safetyThe
as anycast here bypasses the compiler and can hide real permission-checking bugs.
Create an explicit union or overload forallowPermissionsinstead of falling back toany.-const isAdmin = allowPermissions([EUserWorkspaceRoles.ADMIN] as any, EUserPermissionsLevel.WORKSPACE); +const isAdmin = allowPermissions( + [EUserWorkspaceRoles.ADMIN] as readonly EUserWorkspaceRoles[], + EUserPermissionsLevel.WORKSPACE +);This retains strict typing while satisfying the function signature.
web/core/components/workspace/sidebar/workspace-menu.tsx (1)
3-3: UnnecessaryReactimport with the new JSX runtimeSince the project uses the automatic JSX runtime, importing
Reactsolely for JSX transform is redundant.-import React from "react";Removing unused imports cleans up the bundle and avoids ESLint “no-unused-vars” warnings.
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-wrapper.tsx (1)
13-27: Remove redundant React fragment & duplicategroupclass
- The
<>...</>wrapper around the single<div>adds an unnecessary node in the React tree."group/project-item"already applies thegroupvariant – the extra"group"is therefore superfluous.return ( - <> - <div + <div ref={elementRef} className={cn( "group/project-item cursor-pointer relative group flex items-center justify-between w-full gap-1.5 rounded px-2 py-1 outline-none text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-90 active:bg-custom-sidebar-background-90", { "bg-custom-sidebar-background-90": isMenuActive, } )} > {children} </div> - </> );web/core/components/core/sidebar/sidebar-menu-hamburger-toggle.tsx (1)
20-22: Consider typing the event genericconst handleClick = (e: React.MouseEvent<HTMLButtonElement>) => { … }Typing the target element helps editors offer better intellisense and prevents accidental misuse.
web/app/(all)/[workspaceSlug]/(projects)/header.tsx (1)
3-4: Prefermobx-react-liteovermobx-react
mobx-reactre-exports the lite API but also bundles legacy class-component helpers, increasing bundle size ~3-4 kB gzipped. Switching imports keeps tree-shaking tight:-import { observer } from "mobx-react"; +import { observer } from "mobx-react-lite";web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (1)
133-144: Pass stable callbacks to child to avoid re-render loops
handleQuickActionis recreated each render, triggering unnecessary updates inFavoriteItemQuickAction.
Wrap withuseCallback:-const handleQuickAction = (value: boolean) => setIsMenuActive(value); +const handleQuickAction = useCallback( + (value: boolean) => setIsMenuActive(value), + [] +);web/core/components/core/app-header.tsx (1)
4-5: Same note: switch to the leanmobx-react-litepackage-import { observer } from "mobx-react"; +import { observer } from "mobx-react-lite";web/core/components/workspace/sidebar/quick-actions.tsx (2)
24-58:isDraftButtonOpenstate & hover handlers are now dead codeAfter the collapse-aware UI was removed, nothing references
isDraftButtonOpen.
Keeping the state, ref and mouse-over logic adds needless complexity and cognitive load.- const [isDraftButtonOpen, setIsDraftButtonOpen] = useState(false); - // ... - const handleMouseEnter = () => { … setIsDraftButtonOpen(true); }; - const handleMouseLeave = () => { … setIsDraftButtonOpen(false); };You can drop the state, the handlers, the
timeoutRef, and theonMouseEnter/onMouseLeaveprops on the<button>without changing behaviour.
Less code → easier maintenance.
76-84: Superfluous empty object passed tocn
cn("flex …", {})yields the same class string ascn("flex …").
Remove the empty object to avoid noise.-<div className={cn("flex items-center justify-between gap-1 cursor-pointer", {})}> +<div className={cn("flex items-center justify-between gap-1 cursor-pointer")}>web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar.tsx (1)
98-104: Double-negation of a boolean
isExtendedSidebarOpenedis already a boolean.
{!!isExtendedSidebarOpened}is redundant and obscures intent.- isExtendedSidebarOpened={!!isExtendedSidebarOpened} + isExtendedSidebarOpened={isExtendedSidebarOpened}web/app/(all)/[workspaceSlug]/(projects)/sidebar.tsx (1)
61-69: Consider making scroll container focusable for keyboard usersThe new scroll container has
overflow-y-autobut no focus ring.
AddingtabIndex={0}lets keyboard users scroll it with arrow keys/PageUp/Down.-<div className="flex flex-col gap-1 … vertical-scrollbar px-4"> +<div tabIndex={0} className="flex flex-col gap-1 … vertical-scrollbar px-4">web/core/components/workspace/sidebar/projects-list.tsx (1)
67-71: UseforEach(orreduce) instead ofmapfor side-effects
Array.prototype.mapis meant to transform arrays and return a new one.
Here it’s used only for its side-effect of pushing elements intojoinedProjectsList, which is slightly misleading for future readers.-joinedProjects.map((projectId) => { +joinedProjects.forEach((projectId) => {web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx (1)
46-50: Drop the redundant React fragmentThe fragment adds an extra node (and Biome flags it). Pass the component directly:
- extendedSidebar={ - <> - <ExtendedAppSidebar /> - </> - } + extendedSidebar={<ExtendedAppSidebar />}web/ce/components/workspace/sidebar/sidebar-item.tsx (1)
32-39: Minor readability: early-return guard can exit soonerYou already close the extended sidebar when it’s open.
Consider flipping the condition for early exit to avoid the nestedif:- if (window.innerWidth < 768) { - toggleSidebar(); - } - if (isExtendedSidebarOpened) toggleExtendedSidebar(false); + if (window.innerWidth < 768) { + toggleSidebar(); + } + if (isExtendedSidebarOpened) { + toggleExtendedSidebar(false); + }web/core/components/workspace/sidebar/sidebar-menu-items.tsx (1)
63-64: Hard-coded “More” string bypasses i18nReplace with a translation key to keep the UI localisable.
- <span>More</span> + <span>{t("sidebar.more")}</span>web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx (1)
24-24: Consider extracting the localStorage key as a constant.The hardcoded string "sidebarWidth" should be defined as a constant to improve maintainability and prevent typos.
Add a constant at the top of the file:
+const SIDEBAR_WIDTH_STORAGE_KEY = "sidebarWidth"; + export const ExtendedSidebarWrapper: FC<Props> = observer((props) => {Then update line 24:
- const { storedValue } = useLocalStorage("sidebarWidth", 250); + const { storedValue } = useLocalStorage(SIDEBAR_WIDTH_STORAGE_KEY, 250);web/core/components/sidebar/resizable-sidebar.tsx (1)
214-219: Consider adding keyboard accessibility for sidebar toggle.The resize handle supports double-click to toggle the sidebar, but lacks keyboard accessibility.
Consider making the resize handle keyboard accessible:
<div className={cn( "transition-all duration-200 cursor-ew-resize absolute h-full w-1 z-[20]", !isResizing && "hover:bg-custom-background-90", isResizing && "w-1.5 bg-custom-background-80", "top-0 right-0" )} onDoubleClick={() => toggleCollapsed()} onMouseDown={startResizing} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + toggleCollapsed(); + } + }} + tabIndex={0} role="separator" aria-label="Resize sidebar" />Also applies to: 275-280
web/core/components/workspace/sidebar/help-section.tsx (1)
154-160: Hard-coded tooltip breaks i18n consistency
"Expand"/"Hide"are user-visible strings but bypassuseTranslation().
Please pass them throught()like the rest of the file to avoid untranslated UI.web/core/components/workspace/sidebar/dropdown.tsx (1)
72-77: Missing dependency inuseEffect
useEffectreferencestoggleAnySidebarDropdownbut omits it from the dependency array.
Add it (or wrap the store action inuseCallback) to satisfy exhaustive-deps and avoid stale closure issues.web/core/components/workspace/sidebar/projects-list-item.tsx (2)
193-196: Same dependency omission as above
toggleAnySidebarDropdownis used insideuseEffectbut not listed in the dependency array.
Add it or disable the lint rule explicitly.
282-283: Possible stale toggle
setIsMenuActive(!isMenuActive)relies on the stale value ofisMenuActive.
Prefer the functional form to avoid race conditions:- onClick={() => setIsMenuActive(!isMenuActive)} + onClick={() => setIsMenuActive((prev) => !prev)}web/core/store/theme.store.ts (1)
32-37: Consider defaulting observable booleans tofalseInitialising all sidebar flags to
undefinedforces downstream code to coerce three states (undefined | true | false).
Setting them tofalsesimplifies logic and avoids accidental truthy/falsy bugs.Also applies to: 73-80
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/extended-project-sidebar.tsx(5 hunks)web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/header.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/layout.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/sidebar.tsx(1 hunks)web/ce/components/sidebar/project-navigation-root.tsx(1 hunks)web/ce/components/workspace/sidebar/app-search.tsx(1 hunks)web/ce/components/workspace/sidebar/extended-sidebar-item.tsx(0 hunks)web/ce/components/workspace/sidebar/sidebar-item.tsx(2 hunks)web/core/components/core/app-header.tsx(1 hunks)web/core/components/core/sidebar/sidebar-menu-hamburger-toggle.tsx(1 hunks)web/core/components/sidebar/index.ts(1 hunks)web/core/components/sidebar/resizable-sidebar.tsx(1 hunks)web/core/components/workspace-notifications/notification-app-sidebar-option.tsx(1 hunks)web/core/components/workspace-notifications/sidebar/header/root.tsx(2 hunks)web/core/components/workspace/sidebar/dropdown.tsx(4 hunks)web/core/components/workspace/sidebar/favorites/favorite-folder.tsx(2 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-wrapper.tsx(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx(2 hunks)web/core/components/workspace/sidebar/favorites/favorites-menu.tsx(3 hunks)web/core/components/workspace/sidebar/help-section.tsx(4 hunks)web/core/components/workspace/sidebar/project-navigation.tsx(3 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx(6 hunks)web/core/components/workspace/sidebar/projects-list.tsx(5 hunks)web/core/components/workspace/sidebar/quick-actions.tsx(3 hunks)web/core/components/workspace/sidebar/sidebar-menu-items.tsx(2 hunks)web/core/components/workspace/sidebar/user-menu-item.tsx(2 hunks)web/core/components/workspace/sidebar/user-menu.tsx(2 hunks)web/core/components/workspace/sidebar/workspace-menu-header.tsx(2 hunks)web/core/components/workspace/sidebar/workspace-menu-item.tsx(2 hunks)web/core/components/workspace/sidebar/workspace-menu.tsx(2 hunks)web/core/store/theme.store.ts(6 hunks)
💤 Files with no reviewable changes (1)
- web/ce/components/workspace/sidebar/extended-sidebar-item.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/workspace/sidebar/project-navigation.tsx
[error] 175-175: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
web/core/components/workspace/sidebar/favorites/favorites-menu.tsx
[error] 256-262: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 264-271: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx
[error] 47-49: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (16)
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx (1)
8-8: Double-check relative import & eager loading of the new sidebar
- The underscore-prefixed
./_sidebarfile will be bundled eagerly because it’s imported at the top level of a client component.
• Make sure you actually want the entire resizable-sidebar code in the first paint; if not, consider a dynamic import with React’slazy()to defer ~35-40 KB of JS from the critical path.- Confirm that all other modules have been migrated away from the old
./sidebarpath to avoid duplicate side-bar instances in the bundle.No code change required if eager loading is intentional.
Also applies to: 16-16
web/core/components/workspace/sidebar/user-menu.tsx (1)
55-55: Static container class looks goodSwitching to a constant class string simplifies the component and avoids unnecessary re-renders. 👍
web/core/components/workspace-notifications/sidebar/header/root.tsx (1)
22-29: Hamburger toggle now hidden on mobile when sidebar is expanded – verify UX parity
SidebarHamburgerToggleis rendered only whensidebarCollapsedis true.
On small screens the sidebar often starts expanded; users will lose the toggle and be unable to collapse it.Confirm that:
sidebarCollapseddefaults totrueon mobile, orSidebarHamburgerTogglehas its own responsive visibility rules.If neither is true, re-introduce a mobile-only wrapper:
-{sidebarCollapsed && <SidebarHamburgerToggle />} +{(sidebarCollapsed || isMobile) && <SidebarHamburgerToggle />}web/core/components/workspace/sidebar/workspace-menu.tsx (1)
69-70: Static padding may mis-align when nested inside the resizable sidebarPreviously the panel’s padding varied with
sidebarCollapsed. After the refactor it’s hard-coded (mt-0.5).
Double-check spacing in both collapsed and peek states; you might need responsive utility classes or a parent flex gap instead.web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx (1)
26-30: Fixed 8-px left margin is no longer conditional – confirm visual alignment
className="ml-8"was previously applied only when the sidebar was collapsed to compensate for the hidden title.
Now the margin is always present, which may shift icons/text rightward in the full-width sidebar.If the intent is to keep previous spacing, move the margin back behind a layout context check or drop it:
-<Tooltip ... className="ml-8"> +<Tooltip ...>web/ce/components/sidebar/project-navigation-root.tsx (1)
13-15: ```shell
#!/bin/bashRetry searching for any references to the obsolete prop across TS/TSX files
rg -n "isSidebarCollapsed" -g ".ts" -g ".tsx" --color never
</details> <details> <summary>web/core/components/core/sidebar/sidebar-menu-hamburger-toggle.tsx (1)</summary> `11-15`: **👍 Explicit click handler with propagation cancelation looks solid** The new `handleClick` prevents both bubbling & default behaviour, eliminating accidental container-click side-effects while still delegating the toggle logic to the store. No further action needed. </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/header.tsx (1)</summary> `31-32`: **Great: flex util cleans up spacing** The extra `flex items-center gap-2` wrapper fixes breadcrumb mis-alignment on narrow widths. Nice touch. </details> <details> <summary>web/core/components/core/app-header.tsx (1)</summary> `25-26`: **Conditional render improves a11y & layout – good job** Showing the hamburger only when the sidebar is collapsed keeps the DOM clean and prevents duplicate tab-stops on wide screens. </details> <details> <summary>web/core/components/workspace/sidebar/user-menu-item.tsx (1)</summary> `55-63`: **Refactor looks good** Tooltip and collapse logic were cleanly removed; the component is now simpler and easier to reason about. </details> <details> <summary>web/core/components/workspace/sidebar/workspace-menu-item.tsx (1)</summary> `50-64`: **Simplification acknowledged** Removal of tooltip/collapse branches makes the markup leaner while preserving behaviour. </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx (1)</summary> `33-43`: ```shell #!/bin/bash grep -R "storedValue" -n web/appweb/app/(all)/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (1)
18-18: Clean refactor to use the ExtendedSidebarWrapper component.The migration to use
ExtendedSidebarWrappersimplifies the component by delegating container rendering and outside click detection logic. The state management integration with the theme store is well implemented.Also applies to: 30-31, 77-77, 98-103
web/core/components/workspace/sidebar/favorites/favorite-folder.tsx (1)
169-257: Excellent simplification by removing sidebar collapse logic.The removal of conditional rendering based on sidebar state improves code maintainability and provides a consistent user experience. The UI elements now render uniformly, making the component behavior more predictable.
web/core/components/workspace/sidebar/favorites/favorites-menu.tsx (1)
177-233: Well-implemented UI improvements.The addition of the create folder button with proper tooltip and the simplified menu structure improves usability. The removal of sidebar collapse dependencies makes the component behavior more consistent.
web/core/components/sidebar/resizable-sidebar.tsx (1)
28-288: Well-architected resizable sidebar component.The component demonstrates excellent separation of concerns, proper state management, and thoughtful UX considerations with the peek functionality. The accessibility attributes and cleanup logic are particularly well implemented.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx (1)
40-40: Use SIDEBAR_WIDTH constant instead of magic numberThe hardcoded
250should be replaced with the importedSIDEBAR_WIDTHconstant for consistency.- defaultWidth={storedValue ?? 250} + defaultWidth={storedValue ?? SIDEBAR_WIDTH}
🧹 Nitpick comments (3)
web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx (3)
31-31: Remove redundant variable assignmentThe
isAnyExtendedSidebarOpenvariable is just aliasingisExtendedSidebarOpenedwithout any transformation. Consider using the original variable directly or adding a meaningful comment if this alias serves a specific purpose.- // derived values - const isAnyExtendedSidebarOpen = isExtendedSidebarOpened;Then update line 55:
- isAnyExtendedSidebarExpanded={isAnyExtendedSidebarOpen} + isAnyExtendedSidebarExpanded={isExtendedSidebarOpened}
50-54: Remove unnecessary Fragment wrapperThe Fragment around
ExtendedAppSidebaris redundant since it only contains a single child element.- extendedSidebar={ - <> - <ExtendedAppSidebar /> - </> - } + extendedSidebar={<ExtendedAppSidebar />}
15-62: Consider component complexity and prop drillingThe
ProjectAppSidebarcomponent is passing many props toResizableSidebar(14 props). While functional, this indicates high coupling. Consider if some of these props could be:
- Grouped into configuration objects
- Handled internally by ResizableSidebar with sensible defaults
- Moved to a context provider for shared sidebar state
The current implementation works well but may benefit from simplification in future iterations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/constants/src/workspace.ts(1 hunks)packages/ui/src/icons/ai-icon.tsx(1 hunks)packages/ui/src/icons/index.ts(1 hunks)packages/ui/src/icons/plane-icon.tsx(1 hunks)packages/ui/src/icons/wiki-icon.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/header.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/layout.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/sidebar.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(settings)/layout.tsx(1 hunks)web/app/(all)/[workspaceSlug]/layout.tsx(1 hunks)web/app/(all)/profile/layout.tsx(1 hunks)web/ce/components/app-rail/index.ts(1 hunks)web/ce/components/app-rail/root.tsx(1 hunks)web/ce/components/workspace/app-switcher.tsx(1 hunks)web/ce/components/workspace/content-wrapper.tsx(1 hunks)web/ce/components/workspace/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (13)
- web/app/(all)/profile/layout.tsx
- web/ce/components/app-rail/root.tsx
- web/ce/components/workspace/app-switcher.tsx
- web/app/(all)/[workspaceSlug]/layout.tsx
- web/ce/components/workspace/content-wrapper.tsx
- packages/ui/src/icons/plane-icon.tsx
- packages/ui/src/icons/index.ts
- packages/ui/src/icons/wiki-icon.tsx
- packages/constants/src/workspace.ts
- web/ce/components/workspace/index.ts
- web/app/(all)/[workspaceSlug]/(settings)/layout.tsx
- packages/ui/src/icons/ai-icon.tsx
- web/ce/components/app-rail/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/app/(all)/[workspaceSlug]/(projects)/layout.tsx
- web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar.tsx
- web/app/(all)/[workspaceSlug]/(projects)/header.tsx
- web/app/(all)/[workspaceSlug]/(projects)/sidebar.tsx
🧰 Additional context used
🧠 Learnings (1)
web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx (1)
Learnt from: vamsikrishnamathala
PR: makeplane/plane#7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
🪛 Biome (1.9.4)
web/app/(all)/[workspaceSlug]/(projects)/_sidebar.tsx
[error] 51-53: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (3)
web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx (3)
1-11: LGTM: Clean imports and dependency managementThe imports are well-organized and the custom hook integration for outside click detection is appropriate for this wrapper component.
20-25: LGTM: Proper MobX integration and hook usageThe component correctly uses the observer wrapper for reactivity and integrates the custom outside click detection hook appropriately.
28-44: LGTM: Well-implemented sidebar container with smooth animationsThe sidebar container implementation is solid with:
- Proper conditional styling based on open state
- Smooth transform animations for show/hide
- Dynamic positioning based on stored sidebar width
- Appropriate z-index and styling classes
The animation approach using transform and opacity provides good performance compared to changing layout properties.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/constants/src/sidebar.ts (1)
1-2: Good practice using centralized constants, but consider responsive design.The centralized constants approach is excellent for maintainability and consistency across components. The 50px difference between default and extended sidebar widths provides a reasonable visual distinction.
However, consider whether fixed pixel values are optimal for responsive design. For smaller screens or mobile devices, these widths might be too large and could negatively impact usability.
Consider making these responsive or configurable:
-export const SIDEBAR_WIDTH = 250; -export const EXTENDED_SIDEBAR_WIDTH = 300; +export const SIDEBAR_WIDTH = { + DESKTOP: 250, + TABLET: 200, + MOBILE: 280 // Could be full width on mobile +}; +export const EXTENDED_SIDEBAR_WIDTH = { + DESKTOP: 300, + TABLET: 250, + MOBILE: 320 +};Alternatively, consider using CSS custom properties or a configuration system for user preferences.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/constants/src/sidebar.ts(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx
apps/web/app/(all)/[workspaceSlug]/(projects)/extended-sidebar-wrapper.tsx
Outdated
Show resolved
Hide resolved
| const { workspaceSlug, projectId } = useParams(); | ||
| // store hooks | ||
| const { t } = useTranslation(); | ||
| const {} = useAppTheme(); |
* chore: sidebar peek state added to theme store * chore: extended sidebar wrapper added * chore: resizeable sidebar component added * chore: appsidebar root component * chore: updated sidebar and applied necessary changes across codebase * chore: code refactor * chore: code refactor * chore: code refactor * chore: breadcrumb changes * chore: sidebar improvements and fixes * chore: enhancements and fixes * fix: peek sidebar leave * chore: code refactor * chore: code refactor * chore: code refactor * chore: icons added * chore: add dock variable and toggle function to theme store * chore: code refactor * chore: code refactor * chore: code refactor * chore: theme and workspace store updated * chore: workspace content wrapper and apprail context * chore: workspace and project wrapper updated * chore: app rail component * chore: content wrapper * chore: sidebar component updated * chore: layout changes and code refactoring * chore: code refactor * chore: code refactor * chore: code refactor * chore: code refactor * chore: code refactor * chore: code refactor * chore: appsidebar toggle button * chore: code refactor * chore: workspace menu improvements * chore: sidebar spacing and padding improvements * chore: settings layout improvement * chore: enhancements * chore: extended sidebar code refactor * chore: code refactor * fix: merge conflict * fix: merge conflict * chore: code refactor * chore: code refactor * chore: code refactor * chore: code refactor
4 participants
Summary
Type of Change
Reference
[WEB-4328]
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores