Skip to content

[WEB-4827] chore: sidebar content wrapper and code refactor #7840

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.

Sign up for GitHub

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

Closed
anmolsinghbhatia wants to merge 1 commit into from
Closed

[WEB-4827] chore: sidebar content wrapper and code refactor #7840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/web/ce/components/sidebar/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from "./app-switcher";
export * from "./project-navigation-root";
export * from "./sidebar-content-wrapper";
11 changes: 11 additions & 0 deletions apps/web/ce/components/sidebar/sidebar-content-wrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"use client";

type TSidebarContentWrapperProps = {
children: React.ReactNode;
};

export const SidebarContentWrapper = ({ children }: TSidebarContentWrapperProps) => (
<div className="flex flex-col gap-3 overflow-x-hidden scrollbar-sm h-full w-full overflow-y-auto vertical-scrollbar px-3 pt-3">
Copy link

@cursor cursor bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Sidebar Content Padding Issue

Missing pb-0.5 class in the SidebarContentWrapper. The original div in sidebar-wrapper.tsx had pb-0.5 for bottom padding, but the new SidebarContentWrapper component omits this class, which could affect the bottom spacing of the scrollable content.

Fix in Cursor Fix in Web

{children}
</div>
Comment on lines +7 to +10
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix flex/height: h-full here will force overflow and hide the bottom bar.

When used between a header and a footer in a column flex container, this middle section should flex, not be h-full. Use flex-1 min-h-0 to allow internal scrolling and keep the footer visible.

Apply this diff:

-export const SidebarContentWrapper = ({ children }: TSidebarContentWrapperProps) => (
-  <div className="flex flex-col gap-3 overflow-x-hidden scrollbar-sm h-full w-full overflow-y-auto vertical-scrollbar px-3 pt-3">
+export const SidebarContentWrapper = ({ children }: TSidebarContentWrapperProps) => (
+  <div className="flex flex-col gap-3 overflow-x-hidden scrollbar-sm flex-1 min-h-0 w-full overflow-y-auto vertical-scrollbar px-3 pt-3">
     {children}
   </div>
 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const SidebarContentWrapper = ({ children }: TSidebarContentWrapperProps) => (
<div className="flex flex-col gap-3 overflow-x-hidden scrollbar-sm h-full w-full overflow-y-auto vertical-scrollbar px-3 pt-3">
{children}
</div>
export const SidebarContentWrapper = ({ children }: TSidebarContentWrapperProps) => (
<div className="flex flex-col gap-3 overflow-x-hidden scrollbar-sm flex-1 min-h-0 w-full overflow-y-auto vertical-scrollbar px-3 pt-3">
{children}
</div>
);
🤖 Prompt for AI Agents 
In apps/web/ce/components/sidebar/sidebar-content-wrapper.tsx around lines 7 to
10, the wrapper uses h-full which forces full height and can push the footer out
of view; change the container class to use flex-1 min-h-0 instead of h-full so
the middle section flexes between header and footer and internal scrolling works
(keep the existing overflow-y-auto/overflow-x-hidden and other spacing classes).

);
16 changes: 13 additions & 3 deletions apps/web/ce/components/workspace/edition-badge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { useState } from "react";
import { observer } from "mobx-react";
// ui
import packageJson from "package.json";
import { useTranslation } from "@plane/i18n";
import { Tooltip } from "@plane/propel/tooltip";
import { Button } from "@plane/ui";
Comment on lines +4 to 7
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The packageJson import should be moved after the React-related imports to follow conventional import ordering (external dependencies, then internal dependencies).

Suggested change
import packageJson from "package.json";
import { useTranslation } from "@plane/i18n";
import { Tooltip } from "@plane/propel/tooltip";
import { Button } from "@plane/ui";
import { useTranslation } from "@plane/i18n";
import { Tooltip } from "@plane/propel/tooltip";
import { Button } from "@plane/ui";
import packageJson from "package.json";

Copilot uses AI. Check for mistakes.
import { AppSidebarToggleButton } from "@/components/sidebar/sidebar-toggle-button";
import { HelpMenu } from "@/components/workspace/sidebar/help-menu";
// hooks
import { useAppRail } from "@/hooks/use-app-rail";
import { usePlatformOS } from "@/hooks/use-platform-os";
import packageJson from "package.json";
// local components
import { PaidPlanUpgradeModal } from "../license";

Expand All @@ -15,11 +18,14 @@ export const WorkspaceEditionBadge = observer(() => {
const [isPaidPlanPurchaseModalOpen, setIsPaidPlanPurchaseModalOpen] = useState(false);
// translation
const { t } = useTranslation();
// hooks
const { shouldRenderAppRail, isEnabled: isAppRailEnabled } = useAppRail();

// platform
const { isMobile } = usePlatformOS();

return (
<>
<div className="flex items-center justify-between p-3 border-t border-custom-border-200 bg-custom-sidebar-background-100 h-12">
<PaidPlanUpgradeModal
isOpen={isPaidPlanPurchaseModalOpen}
handleClose={() => setIsPaidPlanPurchaseModalOpen(false)}
Expand All @@ -36,6 +42,10 @@ export const WorkspaceEditionBadge = observer(() => {
Community
</Button>
</Tooltip>
</>
<div className="flex items-center gap-2">
{!shouldRenderAppRail && <HelpMenu />}
{!isAppRailEnabled && <AppSidebarToggleButton />}
</div>
</div>
Copy link

@cursor cursor bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Modal Rendering Inside Positioned Container

The PaidPlanUpgradeModal component is now rendered inside a positioned div container with specific styling (border, background, padding, height). Previously, it was rendered at the top level in a React fragment. This change could cause modal positioning or z-index issues since modals should typically be rendered at the DOM tree's top level, not inside positioned containers, even when using portals.

Fix in Cursor Fix in Web

);
});
16 changes: 4 additions & 12 deletions apps/web/core/components/sidebar/sidebar-wrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import { useOutsideClickDetector } from "@plane/hooks";
// components
import { AppSidebarToggleButton } from "@/components/sidebar/sidebar-toggle-button";
import { SidebarDropdown } from "@/components/workspace/sidebar/dropdown";
import { HelpMenu } from "@/components/workspace/sidebar/help-menu";
// hooks
import { useAppTheme } from "@/hooks/store/use-app-theme";
import { useAppRail } from "@/hooks/use-app-rail";
import useSize from "@/hooks/use-window-size";
// plane web components
import { SidebarContentWrapper } from "@/plane-web/components/sidebar";
import { WorkspaceEditionBadge } from "@/plane-web/components/workspace/edition-badge";

type TSidebarWrapperProps = {
Expand Down Expand Up @@ -40,7 +40,7 @@ export const SidebarWrapper: FC<TSidebarWrapperProps> = observer((props) => {
}, [windowSize]);

return (
<div ref={ref} className="flex flex-col h-full w-full">
<div ref={ref} className="relative flex flex-col h-full w-full">
<div className="flex flex-col gap-3 px-3">
{/* Workspace switcher and settings */}
{!shouldRenderAppRail && <SidebarDropdown />}
Expand All @@ -56,17 +56,9 @@ export const SidebarWrapper: FC<TSidebarWrapperProps> = observer((props) => {
{/* Quick actions */}
{quickActions}
</div>
<div className="flex flex-col gap-3 overflow-x-hidden scrollbar-sm h-full w-full overflow-y-auto vertical-scrollbar px-3 pt-3 pb-0.5">
{children}
</div>
<SidebarContentWrapper>{children}</SidebarContentWrapper>
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Ensure the middle section flexes; otherwise the bottom badge may be pushed off-screen.

After switching to SidebarContentWrapper, make sure it uses flex-1 min-h-0 (not h-full). See suggested change in the wrapper component.

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show the referenced file around the snippet
echo "=== apps/web/core/components/sidebar/sidebar-wrapper.tsx ==="
sed -n '1,240p' apps/web/core/components/sidebar/sidebar-wrapper.tsx || true
echo

# 2) Search the repo for SidebarContentWrapper definition/usage
echo "=== rg matches for SidebarContentWrapper ==="
rg -n "SidebarContentWrapper" || true
echo

# 3) If a definition file is found, print its contents (attempt common patterns)
echo "=== Possible wrapper component files (searching for 'Sidebar' files) ==="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!**/build/**' "Sidebar" || true
echo

# 4) Search for components in the same directory
echo "=== Files in apps/web/core/components/sidebar ==="
ls -la apps/web/core/components/sidebar || true
echo

# 5) If a file named sidebar-content-wrapper.* exists, print it
if [ -f apps/web/core/components/sidebar/sidebar-content-wrapper.tsx ]; then
  echo "=== apps/web/core/components/sidebar/sidebar-content-wrapper.tsx ==="
  sed -n '1,240p' apps/web/core/components/sidebar/sidebar-content-wrapper.tsx || true
fi

# 6) Grep for patterns of flex-1 or min-h-0 in the sidebar components
echo "=== Search for 'flex-1' and 'min-h-0' in sidebar dir ==="
rg -n "flex-1|min-h-0|h-full" apps/web/core/components/sidebar || true

Length of output: 4850


---

🏁 Script executed:

```shell
#!/bin/bash
set -euo pipefail

echo "=== files in apps/web/ce/components/sidebar ==="
ls -la apps/web/ce/components/sidebar || true
echo

echo "=== apps/web/ce/components/sidebar/sidebar-content-wrapper.tsx ==="
sed -n '1,240p' apps/web/ce/components/sidebar/sidebar-content-wrapper.tsx || true
echo

echo "=== Search for className usage in that file ==="
rg -n "className" apps/web/ce/components/sidebar/sidebar-content-wrapper.tsx || true
echo

echo "=== Search for flex-1|min-h-0|h-full in that directory ==="
rg -n "flex-1|min-h-0|h-full" apps/web/ce/components/sidebar || true

echo
echo "=== Search for exports in apps/web/ce/components/sidebar ==="
rg -n "export .*SidebarContentWrapper|export \\{.*SidebarContentWrapper" apps/web/ce/components/sidebar || true

echo
echo "=== Search for 'plane-web/components/sidebar' alias usages in repo ==="
rg -n "@/plane-web/components/sidebar" || true

Length of output: 1815

Make SidebarContentWrapper use flex-1 + min-h-0 (not h-full)

apps/web/ce/components/sidebar/sidebar-content-wrapper.tsx — replace the root div's "h-full w-full" with "flex-1 min-h-0" (e.g. "flex-1 min-h-0 flex flex-col gap-3 overflow-x-hidden scrollbar-sm overflow-y-auto vertical-scrollbar px-3 pt-3") so the middle section can shrink and the WorkspaceEditionBadge stays visible.

{/* Help Section */}
<div className="flex items-center justify-between p-3 border-t border-custom-border-200 bg-custom-sidebar-background-100 h-12">
<WorkspaceEditionBadge />
<div className="flex items-center gap-2">
{!shouldRenderAppRail && <HelpMenu />}
{!isAppRailEnabled && <AppSidebarToggleButton />}
</div>
</div>
<WorkspaceEditionBadge />
</div>
);
});
13 changes: 5 additions & 8 deletions packages/propel/src/dialog/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,11 @@ const OVERLAY_CLASSNAME = cn("fixed inset-0 z-backdrop bg-custom-backdrop");
const BASE_CLASSNAME = "relative text-left bg-custom-background-100 rounded-lg shadow-md w-full z-modal";

// Utility functions
const getPositionClassNames = React.useCallback(
(position: DialogPosition) =>
cn("isolate fixed z-modal", {
"top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2": position === "center",
"top-8 left-1/2 -translate-x-1/2": position === "top",
}),
[]
);
const getPositionClassNames = (position: DialogPosition) =>
cn("isolate fixed z-modal", {
"top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2": position === "center",
"top-8 left-1/2 -translate-x-1/2": position === "top",
});

const DialogPortal = React.memo<React.ComponentProps<typeof BaseDialog.Portal>>(({ children, ...props }) => (
<BaseDialog.Portal data-slot="dialog-portal" {...props}>
Expand Down
Loading