[WIKI-887] fix: add scroll in heading layout #8596
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughNavigation pane tab panels receive UI and layout updates across four files. Changes include adding horizontal padding to the assets panel, restructuring the info panel with scrollable regions, implementing ScrollArea in the outline panel, and replacing Headless UI Tabs with a custom Tabs component for rendering. No functional or control flow modifications are introduced. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR aims to add scroll functionality to the heading layout in the page navigation pane by migrating from headlessui's Tab components to @plane/propel's Tabs components and implementing ScrollArea for better scroll management.
Changes:
- Migrated tab panels from headlessui Tab.Panel to @plane/propel Tabs.Content
- Added ScrollArea component to the outline tab for improved scroll handling
- Restructured the info tab layout with flex containers and overflow handling
- Added horizontal padding (px-4) to the assets tab content
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx | Removed React and headlessui imports, replaced Tab.Panels with fragment and Tab.Panel with Tabs.Content |
| apps/web/core/components/pages/navigation-pane/tab-panels/outline.tsx | Wrapped content in ScrollArea component for improved scroll handling |
| apps/web/core/components/pages/navigation-pane/tab-panels/info/root.tsx | Added flex container structure with overflow-y-auto for scrolling |
| apps/web/core/components/pages/navigation-pane/tab-panels/assets.tsx | Added px-4 padding to content container |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -23,12 +25,18 @@ export function PageNavigationPaneOutlineTabPanel(props: Props) { | |||
| } = page; | |||
|
|
|||
There was a problem hiding this comment.
The ScrollArea component has both "overflow-y-auto" and "hide-scrollbar" classes applied. The "hide-scrollbar" class conflicts with the scrollbar functionality. Looking at other usages of ScrollArea in the codebase (e.g., apps/web/core/components/sidebar/sidebar-wrapper.tsx:69-77), the pattern is to use scrollType="hover" and size="sm" without the hide-scrollbar class, allowing the ScrollArea component to manage the scrollbar display internally. Remove the "hide-scrollbar" class from the className prop.
| <div className="mt-5 space-y-4 px-4"> | ||
| {assetsList?.map((asset) => ( | ||
| <AssetItem key={asset.id} asset={asset} page={page} /> | ||
| ))} |
There was a problem hiding this comment.
The assets tab panel is missing a scrolling container. The original implementation had overflow-y-auto on the Tab.Panel wrapper (see removed line with "overflow-y-auto vertical-scrollbar scrollbar-sm"). After the refactor, while the outline tab got a ScrollArea component and the info tab has an overflow-y-auto wrapper, the assets tab only has static content without any scrolling mechanism.
For consistency with the other tabs and to prevent content from being cut off when the asset list is long, this should either:
- Be wrapped in a ScrollArea component like the outline tab, or
- Have an outer wrapper div with flex-1 and overflow-y-auto like the info tab
The most consistent approach would be to use ScrollArea like the outline tab.
| <div className="mt-5 space-y-4 px-4"> | |
| {assetsList?.map((asset) => ( | |
| <AssetItem key={asset.id} asset={asset} page={page} /> | |
| ))} | |
| <div className="flex-1 overflow-y-auto"> | |
| <div className="mt-5 space-y-4 px-4"> | |
| {assetsList?.map((asset) => ( | |
| <AssetItem key={asset.id} asset={asset} page={page} /> | |
| ))} | |
| </div> |
| @@ -23,12 +25,18 @@ export function PageNavigationPaneOutlineTabPanel(props: Props) { | |||
| } = page; | |||
|
|
|||
There was a problem hiding this comment.
The ScrollArea component already handles scrolling internally, so the "overflow-y-auto" class in the className prop is redundant. The ScrollArea component from @plane/propel/scrollarea manages its own overflow behavior through the orientation and scrollType props. Remove "overflow-y-auto" from the className and let the ScrollArea component handle the scrolling behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/core/components/pages/navigation-pane/tab-panels/outline.tsx`:
- Around line 28-40: The ScrollArea usage currently combines scrollType="hover"
with a conflicting CSS utility; remove the hide-scrollbar token from the
ScrollArea's className so the hover-based scrollbar opacity (scrollType="hover"
/ data-[hovering]:opacity-100) can take effect—update the ScrollArea component
where it is rendered (the ScrollArea element wrapping PageContentBrowser with
props orientation="vertical", size="sm", scrollType="hover",
viewportClassName="px-4") to keep overflow-y-auto but drop hide-scrollbar.
🧹 Nitpick comments (1)
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx (1)
18-18: Import ordering: Move@plane/propelimport to the plane imports section.The
Tabsimport from@plane/propel/tabsshould be grouped with other plane imports (around lines 10-11) rather than after local imports, to maintain consistent import organization.♻️ Suggested import reordering
// components import type { TPageRootHandlers } from "@/components/pages/editor/page-root"; // plane web imports +import { Tabs } from "@plane/propel/tabs"; import { ORDERED_PAGE_NAVIGATION_TABS_LIST } from "@/plane-web/components/pages/navigation-pane"; import { PageNavigationPaneAdditionalTabPanelsRoot } from "@/plane-web/components/pages/navigation-pane/tab-panels/root"; // store import type { TPageInstance } from "@/store/pages/base-page"; // local imports import { PageNavigationPaneAssetsTabPanel } from "./assets"; import { PageNavigationPaneInfoTabPanel } from "./info/root"; import { PageNavigationPaneOutlineTabPanel } from "./outline"; -import { Tabs } from "@plane/propel/tabs";
| <ScrollArea | ||
| orientation="vertical" | ||
| size="sm" | ||
| scrollType="hover" | ||
| className="size-full overflow-y-auto hide-scrollbar" | ||
| viewportClassName="px-4" | ||
| > | ||
| <PageContentBrowser | ||
| className="mt-0" | ||
| editorRef={editorRef} | ||
| emptyState={<PageNavigationPaneOutlineTabEmptyState />} | ||
| /> | ||
| </div> | ||
| </ScrollArea> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ScrollArea component location
fd -e ts -e tsx "scroll" | grep -i area | head -20Repository: makeplane/plane
Length of output: 190
🏁 Script executed:
# Search for ScrollArea component definition in the codebase
rg -n "export.*ScrollArea" --type=ts --type=tsx -A 10Repository: makeplane/plane
Length of output: 86
🏁 Script executed:
# Check the actual file content to verify the code state
cat -n "apps/web/core/components/pages/navigation-pane/tab-panels/outline.tsx" | head -50Repository: makeplane/plane
Length of output: 1460
🏁 Script executed:
# Search for scrollType prop usage and its documentation
rg -n "scrollType" --type=ts --type=tsx -B 2 -A 2 | head -50Repository: makeplane/plane
Length of output: 86
🏁 Script executed:
# Check `@plane/propel` package structure
fd "scrollarea" -type dRepository: makeplane/plane
Length of output: 229
🏁 Script executed:
# Read ScrollArea implementation
cat -n "packages/propel/src/scrollarea/scrollarea.tsx"Repository: makeplane/plane
Length of output: 4535
🏁 Script executed:
# Check the alternative scroll-area implementation
cat -n "packages/ui/src/scroll-area.tsx"Repository: makeplane/plane
Length of output: 3459
🏁 Script executed:
# Search for hide-scrollbar class usage patterns
rg "hide-scrollbar" -nRepository: makeplane/plane
Length of output: 190
🏁 Script executed:
# Check if scrollType is defined/documented anywhere
rg "scrollType" -n -A 5 -B 5Repository: makeplane/plane
Length of output: 16436
🏁 Script executed:
# Verify hide-scrollbar class definition to confirm it hides scrollbars
rg "hide-scrollbar" -B 2 -A 2Repository: makeplane/plane
Length of output: 545
🏁 Script executed:
# Check if there are any docs or comments explaining the intent
cat -n "packages/propel/src/scrollarea/scrollarea.tsx" | grep -A 5 -B 5 "hover"Repository: makeplane/plane
Length of output: 1675
Remove hide-scrollbar class—it conflicts with scrollType="hover".
The scrollType="hover" prop uses opacity transitions to show the scrollbar on hover (data-[hovering]:opacity-100), but the hide-scrollbar class completely hides it regardless of hover state. These are mutually exclusive. Remove hide-scrollbar from the className to allow the scrollbar to appear on hover as intended. The overflow-y-auto class is appropriate and consistent with other ScrollArea usages in the codebase.
🤖 Prompt for AI Agents
In `@apps/web/core/components/pages/navigation-pane/tab-panels/outline.tsx` around
lines 28 - 40, The ScrollArea usage currently combines scrollType="hover" with a
conflicting CSS utility; remove the hide-scrollbar token from the ScrollArea's
className so the hover-based scrollbar opacity (scrollType="hover" /
data-[hovering]:opacity-100) can take effect—update the ScrollArea component
where it is rendered (the ScrollArea element wrapping PageContentBrowser with
props orientation="vertical", size="sm", scrollType="hover",
viewportClassName="px-4") to keep overflow-y-auto but drop hide-scrollbar.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.