fix(page-side-panel): scope portal via usePortalContainer, drop the root marker (#16555)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: gujiaming <52187003+AtomsH4@users.noreply.github.com>
This commit is contained in:
Gu JiaMing
2026-06-30 20:06:57 +08:00
committed by GitHub
parent 366ecf63da
commit c993719569
7 changed files with 117 additions and 196 deletions

View File

@@ -355,10 +355,10 @@ There are two different drawer patterns. Do not collapse them into one generic "
Source: `PageSidePanel` from `@cherrystudio/ui` (`packages/ui/src/components/composites/page-side-panel/index.tsx`).
Use `PageSidePanel` for page-owned management surfaces such as mini-app display settings, translate settings, and translate history. The panel and backdrop portal to `document.body` so page scroll containers, transformed ancestors, and nested layout shells cannot clip or re-base the drawer.
Use `PageSidePanel` for page-owned management surfaces such as mini-app display settings, translate settings, and translate history. By default, the panel and backdrop portal to `document.body` so page scroll containers, transformed ancestors, and nested layout shells cannot clip or re-base the drawer. In app shell route tabs, the tab content root is provided via `PortalContainerProvider` on every platform; `PageSidePanel` reads it with `usePortalContainer()`, portals into its owning tab root, and switches to absolute positioning, so a still-open panel stays hidden with its owning tab instead of surfacing over the active tab. The same provider scopes tab-owned Radix floating overlays (popovers, dropdown menus, selects, tooltips, hover cards, context menus) to that root, so the panel and those overlays share one portal container per tab.
- Backdrop: fixed `inset-0`, `z-[60]`, `bg-black/50`, fades over `0.15s`
- Panel: fixed `top-3 bottom-3`, `right-3` or `left-3`, `z-[70]`
- Backdrop: default fixed `inset-0`, scoped absolute `inset-0`, `z-60`, `bg-black/50`, fades over `0.15s`
- Panel: default fixed `top-3 bottom-3`, scoped absolute `top-3 bottom-3`, `right-3` or `left-3`, `z-70`
- Size / shell: `w-100`, `rounded-3xl`, `bg-card`, `text-card-foreground`, `shadow-xl`, `overflow-hidden`
- Motion: horizontal slide from the chosen side with spring transition (`damping: 30`, `stiffness: 350`)
- Header: `px-6 pt-6 pb-3`, optional header content plus ghost close button
@@ -846,7 +846,7 @@ Use icon-library defaults unless a component has a documented reason to override
- "Build a settings card: `var(--color-card)` background, 1px `var(--color-border)`, `var(--radius-lg)`. Title in `var(--font-size-heading-sm)` with the matching heading line-height. Description in `var(--font-size-body-sm)` `var(--font-weight-regular)`, `var(--color-foreground-secondary)`. Toggles and inputs at `var(--radius-md)`."
- "Create a dark-mode conversation view: `var(--color-background)` page. Message cards on `var(--color-card)`. Assistant code blocks use the code-rendering component's mono font stack at `var(--font-size-body-sm)` on `var(--color-popover)` with `var(--radius-md)`. Borders at `var(--color-border)`."
- "Design a destructive confirmation dialog with the shared Dialog shell: `bg-card`, `text-card-foreground`, `rounded-3xl`, `border-0`, `p-6`, `gap-4`, `shadow-xl`, default overlay. Footer uses outline cancel + destructive delete."
- "Build a page-owned settings side panel with `PageSidePanel`: body-portaled full-viewport `bg-black/50` backdrop, fixed `top-3 bottom-3 right-3`, `w-100`, `bg-card`, `rounded-3xl`, `shadow-xl`, `title` for the shared `text-base` heading, body `px-6 py-4`, `PageSidePanelSection` groups separated by `gap-8`, and `PageSidePanelItem` rows separated by `gap-5` inside each group. Use only `PageSidePanel` for non-settings history/list/detail drawers, with a task-specific body layout."
- "Build a page-owned settings side panel with `PageSidePanel`: it reads `usePortalContainer()` to scope into the owning route tab/page root when a `PortalContainerProvider` is present, otherwise falls back to the body portal; default fixed and scoped absolute `bg-black/50` backdrop, `top-3 bottom-3 right-3`, `w-100`, `bg-card`, `rounded-3xl`, `shadow-xl`, `title` for the shared `text-base` heading, body `px-6 py-4`, `PageSidePanelSection` groups separated by `gap-8`, and `PageSidePanelItem` rows separated by `gap-5` inside each group. Use only `PageSidePanel` for non-settings history/list/detail drawers, with a task-specific body layout."
- "Build a modal bottom drawer with the shared `Drawer` primitive: `bg-background`, edge-attached bottom content, `max-h-[80vh]`, `rounded-t-lg`, `border-t`, built-in drag handle, header/footer `p-4`. Do not use the floating `PageSidePanel` shell for this."
- "Floating toolbar: `bg-popover`, 1px `var(--color-border)`, `var(--radius-xl)`, `var(--shadow-md)`. Icon buttons inside use the shared `Button` with `variant=\"ghost\"` and `size=\"icon-sm\"`."
- "Dense row actions: use low-emphasis icon-only controls with muted default text, no static fill, tooltip/`aria-label`, hover-only emphasis, and active tint only when the action has persistent state. Promote this pattern into a shared `IconButton` before reusing it across pages."

View File

@@ -1,7 +1,8 @@
// @vitest-environment jsdom
import '@testing-library/jest-dom/vitest'
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import { PortalContainerProvider } from '@cherrystudio/ui/components/primitives/portal-container'
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
import * as React from 'react'
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
@@ -175,6 +176,20 @@ describe('PageSidePanel', () => {
})
describe('placement', () => {
function ScopedPanel() {
const [container, setContainer] = React.useState<HTMLDivElement | null>(null)
return (
<div data-testid="page-shell">
<div ref={setContainer} data-testid="panel-root">
<PortalContainerProvider container={container}>
<PageSidePanel open={true} onClose={vi.fn()} />
</PortalContainerProvider>
</div>
</div>
)
}
it('uses the design shell classes by default', () => {
render(
<PageSidePanel open={true} onClose={vi.fn()} header={<span>Panel title</span>}>
@@ -217,15 +232,11 @@ describe('PageSidePanel', () => {
expect(screen.getByRole('dialog')).toHaveClass('fixed')
})
it('portals into a scoped page side panel root when present', () => {
const { container } = render(
<div data-testid="page-shell">
<div data-page-side-panel-root="true" data-testid="panel-root" />
<PageSidePanel open={true} onClose={vi.fn()} />
</div>
)
it('portals into a provided page side panel container', async () => {
const { container } = render(<ScopedPanel />)
const root = screen.getByTestId('panel-root')
await waitFor(() => expect(root.querySelector('[data-slot="page-side-panel"]')).toBeInTheDocument())
const panel = root.querySelector('[data-slot="page-side-panel"]')
const backdrop = root.querySelector('[data-slot="page-side-panel-backdrop"]')
expect(container).toContainElement(root)
@@ -235,15 +246,11 @@ describe('PageSidePanel', () => {
expect(backdrop?.parentElement).toBe(root)
})
it('uses absolute positioning when portaled into a scoped root', () => {
render(
<div data-page-side-panel-root="true">
<PageSidePanel open={true} onClose={vi.fn()} />
</div>
)
it('uses absolute positioning when portaled into a provided container', async () => {
render(<ScopedPanel />)
await waitFor(() => expect(screen.getByRole('dialog')).toHaveClass('absolute'))
expect(document.querySelector('[data-slot="page-side-panel-backdrop"]')).toHaveClass('absolute')
expect(screen.getByRole('dialog')).toHaveClass('absolute')
})
it('applies design inset classes by default', () => {

View File

@@ -1,27 +1,23 @@
import { Button } from '@cherrystudio/ui/components/primitives/button'
import { usePortalContainer } from '@cherrystudio/ui/components/primitives/portal-container'
import { cn } from '@cherrystudio/ui/lib/utils'
import { XIcon } from 'lucide-react'
import { AnimatePresence, motion } from 'motion/react'
import * as React from 'react'
import { useCallback, useEffect, useId, useLayoutEffect, useRef, useState } from 'react'
import { useCallback, useEffect, useId, useRef } from 'react'
import { createPortal } from 'react-dom'
import Scrollbar from '../scrollbar'
/**
* A page-owned floating side panel. It portals to `document.body` so the fixed
* panel and viewport backdrop are not clipped or re-based by page layout,
* transformed ancestors, virtualized lists, or scroll containers.
* A page-owned floating side panel. It portals into the nearest portal container
* provided by `PortalContainerProvider` (e.g. a route tab root) when present,
* otherwise to `document.body`. Scoped to a container it positions `absolute`
* within it; at `document.body` it stays `fixed` to the viewport.
*
* For edge-attached modal sheets, use the shadcn `Drawer` primitive instead.
*/
type PageSidePanelPlacement = 'left' | 'right'
const PAGE_SIDE_PANEL_ROOT_SELECTOR = '[data-page-side-panel-root="true"]'
function resolvePortalContainer() {
if (typeof document === 'undefined') return null
return document.querySelector<HTMLElement>(PAGE_SIDE_PANEL_ROOT_SELECTOR) ?? document.body
}
interface PageSidePanelProps {
open: boolean
@@ -65,17 +61,11 @@ function PageSidePanel({
const panelRef = useRef<HTMLDivElement>(null)
const triggerRef = useRef<HTMLElement | null>(null)
const closedByPointerDownRef = useRef(false)
const [portalContainer, setPortalContainer] = useState<HTMLElement | null>(resolvePortalContainer)
const scopedContainer = usePortalContainer()
const portalContainer = scopedContainer ?? (typeof document === 'undefined' ? null : document.body)
const isScopedPortal =
typeof document !== 'undefined' && portalContainer !== null && portalContainer !== document.body
useLayoutEffect(() => {
const nextPortalContainer = resolvePortalContainer()
setPortalContainer((currentPortalContainer) =>
currentPortalContainer === nextPortalContainer ? currentPortalContainer : nextPortalContainer
)
}, [])
const handleClose = useCallback(
(event?: React.MouseEvent | React.PointerEvent | React.KeyboardEvent) => {
event?.preventDefault()

View File

@@ -1,11 +1,10 @@
import { PortalContainerProvider } from '@cherrystudio/ui'
import { TabIdProvider } from '@renderer/components/layout/TabIdProvider'
import { routeTree } from '@renderer/routeTree.gen'
import { isMac } from '@renderer/utils/platform'
import type { Tab } from '@shared/data/cache/cacheValueTypes'
import { createMemoryHistory, createRouter, RouterProvider } from '@tanstack/react-router'
import { Activity } from 'react'
import { useEffect, useMemo, useState } from 'react'
import { useCallback, useEffect, useMemo, useState } from 'react'
interface TabRouterProps {
tab: Tab
@@ -46,14 +45,20 @@ export const TabRouter = ({ tab, isActive, onUrlChange }: TabRouterProps) => {
}, [router, tab.url])
const [tabPortalContainer, setTabPortalContainer] = useState<HTMLElement | null>(null)
// Latch the captured node across Activity hide/show: a hidden tab detaches the ref
// (node === null) while its DOM node lives on, and clearing the container would
// un-scope a still-open overlay/PageSidePanel to a full-window document.body portal.
const captureTabPortalContainer = useCallback((node: HTMLElement | null) => {
if (node) setTabPortalContainer(node)
}, [])
return (
<Activity mode={isActive ? 'visible' : 'hidden'}>
<TabIdProvider tabId={tab.id}>
<div
ref={setTabPortalContainer}
data-page-side-panel-root={!isMac && isActive ? 'true' : undefined}
className="relative flex h-full min-h-0 w-full flex-1 flex-col">
{/* This tab's content root is the portal target for overlays and PageSidePanel
scoped to the tab (`relative` anchors the scoped panel's absolute layout), so a
background tab's still-open surface stays hidden with its owning tab. */}
<div ref={captureTabPortalContainer} className="relative flex h-full min-h-0 w-full flex-1 flex-col">
<PortalContainerProvider container={tabPortalContainer}>
<RouterProvider router={router} />
</PortalContainerProvider>

View File

@@ -1,11 +1,10 @@
// @vitest-environment jsdom
import '@testing-library/jest-dom/vitest'
import { cleanup, render, screen } from '@testing-library/react'
import { cleanup, render } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
isMac: false,
commandHandlers: new Map<string, () => void>(),
showSearchPopup: vi.fn()
}))
@@ -67,14 +66,8 @@ vi.mock('../AppShellTabBar', () => ({
AppShellTabBar: () => <header data-testid="tab-bar" />
}))
// Mirror TabRouter's real scoped-root marking so AppShell's chrome layout can be
// asserted without mounting the full router tree.
vi.mock('../TabRouter', () => ({
TabRouter: ({ isActive }: { isActive: boolean }) => (
<section data-testid="tab-router">
{!mocks.isMac && isActive ? <div data-page-side-panel-root="true" data-testid="scoped-root" /> : null}
</section>
)
TabRouter: () => <section data-testid="tab-router" />
}))
import { AppShell } from '../AppShell'
@@ -82,30 +75,9 @@ import { AppShell } from '../AppShell'
afterEach(() => {
cleanup()
vi.clearAllMocks()
mocks.isMac = false
mocks.commandHandlers.clear()
})
describe('AppShell page side panel root', () => {
it('scopes the page side panel root to the tab content area, excluding app chrome, outside macOS', () => {
mocks.isMac = false
render(<AppShell />)
const root = document.querySelector('[data-page-side-panel-root="true"]')
expect(root).toBeInTheDocument()
expect(root).not.toContainElement(screen.getByTestId('tab-bar'))
expect(root).not.toContainElement(screen.getByTestId('sidebar'))
expect(screen.getByTestId('tab-router')).toContainElement(root as HTMLElement)
})
it('does not mark a scoped page side panel root on macOS', () => {
mocks.isMac = true
render(<AppShell />)
expect(document.querySelector('[data-page-side-panel-root="true"]')).not.toBeInTheDocument()
})
})
describe('AppShell', () => {
it('opens global search from the shell-level shortcut', () => {
render(<AppShell />)

View File

@@ -11,44 +11,32 @@ import * as React from 'react'
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
const knobs = vi.hoisted(() => ({
isMac: false,
renderPage: (() => null) as (url: string) => React.ReactNode
}))
const routerMocks = vi.hoisted(() => ({
portalContainer: {
current: null as HTMLElement | null
},
navigate: vi.fn(),
subscribe: vi.fn(() => vi.fn())
}))
vi.mock('@renderer/utils/platform', () => ({
get isMac() {
return knobs.isMac
}
}))
vi.mock('@cherrystudio/ui', () => ({
PortalContainerProvider: ({ children, container }: { children: React.ReactNode; container: HTMLElement | null }) => {
routerMocks.portalContainer.current = container
return (
<div
data-has-portal-container={String(container instanceof HTMLElement)}
data-portal-container-is-body={String(container === document.body)}
data-testid="portal-container-provider">
{children}
</div>
// PageSidePanel scopes to its owning tab by reading the SAME PortalContainerContext that
// TabRouter's provider sets. PageSidePanel pulls the hook from the deep path while TabRouter
// pulls the provider from the barrel; mock the deep path to the real module so every importer
// resolves to one context instance, then re-export it from the barrel (otherwise the global
// @cherrystudio/ui stub shadows it).
vi.mock('@cherrystudio/ui/components/primitives/portal-container', async (importOriginal) => importOriginal())
vi.mock('@cherrystudio/ui', async () => {
const { PortalContainerProvider, usePortalContainer } = await import(
'@cherrystudio/ui/components/primitives/portal-container'
)
},
usePortalContainer: () => routerMocks.portalContainer.current
}))
return { PortalContainerProvider, usePortalContainer }
})
vi.mock('@renderer/routeTree.gen', () => ({ routeTree: {} }))
// Stub the router so TabRouter can mount without the real route tree. Each tab's
// history carries its url so the injected page can tell tabs apart, and the
// provider exposes the resolved portal container for the scoping assertions.
// Stub the router so TabRouter can mount without the real route tree. Each tab's history
// carries its url so the injected page can tell tabs apart, and the provider exposes the
// resolved portal container for the scoping assertions.
vi.mock('@tanstack/react-router', async () => {
const { usePortalContainer } = await import('@cherrystudio/ui')
@@ -69,6 +57,7 @@ vi.mock('@tanstack/react-router', async () => {
return (
<div
data-testid="router-provider"
data-router-url={router.state.location.href}
data-has-portal-container={String(container instanceof HTMLElement)}
data-portal-container-is-body={String(container === document.body)}>
{knobs.renderPage(router.state.location.href)}
@@ -92,39 +81,26 @@ beforeAll(() => {
afterEach(() => {
cleanup()
knobs.isMac = false
knobs.renderPage = () => null
routerMocks.portalContainer.current = null
vi.clearAllMocks()
})
describe('TabRouter page side panel root', () => {
it('exposes the scoped root on the active tab subtree outside macOS', () => {
const { container } = render(<TabRouter tab={tab('a', '/a')} isActive onUrlChange={() => {}} />)
expect(container.querySelector('[data-page-side-panel-root="true"]')).toBeInTheDocument()
})
describe('TabRouter portal container', () => {
it('provides the tab root as a scoped portal container, not document.body', async () => {
render(<TabRouter tab={tab('a', '/a')} isActive onUrlChange={() => {}} />)
it('does not expose the scoped root on an inactive tab', () => {
const { container } = render(<TabRouter tab={tab('a', '/a')} isActive={false} onUrlChange={() => {}} />)
expect(container.querySelector('[data-page-side-panel-root="true"]')).not.toBeInTheDocument()
})
it('does not expose a scoped root on macOS', () => {
knobs.isMac = true
const { container } = render(<TabRouter tab={tab('a', '/a')} isActive onUrlChange={() => {}} />)
expect(container.querySelector('[data-page-side-panel-root="true"]')).not.toBeInTheDocument()
await waitFor(() =>
expect(screen.getByTestId('router-provider')).toHaveAttribute('data-has-portal-container', 'true')
)
expect(screen.getByTestId('router-provider')).toHaveAttribute('data-portal-container-is-body', 'false')
})
})
describe('TabRouter PageSidePanel portal isolation', () => {
// Regression for the non-mac scoped portal: a PageSidePanel opened in one tab
// must not stay visible after switching to another tab.
it('hides a still-open panel from the previous tab after switching tabs', () => {
function Page({ url }: { url: string }) {
const [open] = React.useState(url === '/a')
const [open] = React.useState(url === '/b')
return <PageSidePanel open={open} onClose={() => {}} title={`panel ${url}`} />
}
knobs.renderPage = (url) => <Page url={url} />
function Shell({ activeId }: { activeId: string }) {
return (
@@ -135,50 +111,33 @@ describe('TabRouter PageSidePanel portal isolation', () => {
)
}
const { rerender } = render(<Shell activeId="a" />)
// The real PortalContainerProvider renders no DOM, so each router-provider's parent IS the
// owning tab's content root — the element the panel portals into.
const tabRoot = (url: string) =>
document.querySelector<HTMLElement>(`[data-router-url="${url}"]`)?.parentElement as HTMLElement
let roots = document.querySelectorAll('[data-page-side-panel-root="true"]')
expect(roots).toHaveLength(1)
const aRoot = roots[0] as HTMLElement
expect(aRoot.querySelector('[role="dialog"]')).toBeInTheDocument()
// Core regression: a background tab's open panel must never surface inside the active tab.
// Open b's panel while b is active so b captures its own root, then switch to a.
it('keeps a panel opened on the active tab scoped to that tab after switching away', async () => {
knobs.renderPage = (url) => <Page url={url} />
rerender(<Shell activeId="b" />)
const { rerender } = render(<Shell activeId="b" />)
const aRoot = tabRoot('/a')
const bRoot = tabRoot('/b')
expect(aRoot).toBeInstanceOf(HTMLElement)
expect(bRoot).toBeInstanceOf(HTMLElement)
await waitFor(() => expect(bRoot.querySelector('[role="dialog"]')).toBeInTheDocument())
roots = document.querySelectorAll('[data-page-side-panel-root="true"]')
expect(roots).toHaveLength(1)
expect(roots[0]).not.toBe(aRoot)
rerender(<Shell activeId="a" />)
expect(aRoot.querySelector('[role="dialog"]')).toBeInTheDocument()
expect(aRoot.style.display).toBe('none')
expect(roots[0].querySelector('[role="dialog"]')).not.toBeInTheDocument()
// b's panel stays in b's now-hidden root; it never migrates to active a.
expect(bRoot.querySelector('[role="dialog"]')).toBeInTheDocument()
expect(bRoot.style.display).toBe('none')
expect(aRoot.querySelector('[role="dialog"]')).not.toBeInTheDocument()
})
})
describe('TabRouter', () => {
it('provides the tab root as scoped portal containers', async () => {
render(
<TabRouter
tab={{
id: 'translate-tab',
type: 'route',
url: '/app/translate',
title: 'Translate',
lastAccessTime: 1,
isDormant: false
}}
isActive
onUrlChange={vi.fn()}
/>
)
await waitFor(() =>
expect(screen.getByTestId('router-provider')).toHaveAttribute('data-has-portal-container', 'true')
)
expect(screen.getByTestId('router-provider')).toHaveAttribute('data-portal-container-is-body', 'false')
expect(screen.getByTestId('portal-container-provider')).toHaveAttribute('data-has-portal-container', 'true')
expect(screen.getByTestId('portal-container-provider')).toHaveAttribute('data-portal-container-is-body', 'false')
})
it('uses the tab entry URL even when instance metadata points to another key', () => {
render(
<TabRouter

View File

@@ -6,9 +6,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
const tabs = [{ id: 'home', type: 'route', url: '/home', title: 'Home' }]
async function renderSubWindowAppShell(isMac: boolean) {
async function renderSubWindowAppShell() {
vi.resetModules()
vi.doMock('@renderer/utils/platform', () => ({ isMac, isWin: false, isLinux: false }))
vi.doMock('@renderer/utils/platform', () => ({ isMac: false, isWin: false, isLinux: false }))
vi.doMock('@renderer/databases', () => ({}))
vi.doMock('@renderer/hooks/useWindowInitData', () => ({
useWindowInitData: () => null
@@ -35,11 +35,7 @@ async function renderSubWindowAppShell(isMac: boolean) {
SubWindowTitleBar: () => <header data-testid="sub-window-title-bar" />
}))
vi.doMock('@renderer/components/layout/TabRouter', () => ({
TabRouter: ({ isActive }: { isActive: boolean }) => (
<section data-testid="tab-router">
{!isMac && isActive ? <div data-page-side-panel-root="true" data-testid="scoped-root" /> : null}
</section>
)
TabRouter: () => <section data-testid="tab-router" />
}))
vi.doMock('@renderer/components/MiniApp/MiniAppTabsPool', () => ({
default: () => <div data-testid="mini-app-pool" />
@@ -55,19 +51,11 @@ afterEach(() => {
vi.resetModules()
})
describe('SubWindowAppShell page side panel root', () => {
it('scopes the page side panel root to the tab content area, excluding app chrome, outside macOS', async () => {
await renderSubWindowAppShell(false)
describe('SubWindowAppShell', () => {
it('renders the title bar and tab router', async () => {
await renderSubWindowAppShell()
const root = document.querySelector('[data-page-side-panel-root="true"]')
expect(root).toBeInTheDocument()
expect(root).not.toContainElement(screen.getByTestId('sub-window-title-bar'))
expect(screen.getByTestId('tab-router')).toContainElement(root as HTMLElement)
})
it('does not mark a scoped page side panel root on macOS', async () => {
await renderSubWindowAppShell(true)
expect(document.querySelector('[data-page-side-panel-root="true"]')).not.toBeInTheDocument()
expect(screen.getByTestId('sub-window-title-bar')).toBeInTheDocument()
expect(screen.getByTestId('tab-router')).toBeInTheDocument()
})
})