diff --git a/packages/components/src/__tests__/resizable-orientation.test.tsx b/packages/components/src/__tests__/resizable-orientation.test.tsx new file mode 100644 index 000000000..b1942e2fd --- /dev/null +++ b/packages/components/src/__tests__/resizable-orientation.test.tsx @@ -0,0 +1,133 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The stacked resizable divider — `custom/resizable.tsx`. + * + * `ui/resizable.tsx` is the react-resizable-panels **v3** Shadcn file: its + * whole stacked-group appearance hangs off `data-[panel-group-direction=vertical]`, + * an attribute v4 (the installed major) never emits. So in a stacked group the + * divider kept the base `w-px` and came out a 1px-wide, 16px-tall sliver — its + * only height being the grip — instead of a full-width hairline, with a 4x16px + * drag target pinned to the group's left edge and an unrotated grip. + * `custom/resizable.tsx` re-keys those styles onto `aria-orientation`, which v4 + * does emit on the separator. + * + * These tests pin the *class contract* and the attribute it depends on; happy-dom + * has no Tailwind, so resolved geometry (478x1px rule, 478x4px hit strip, grip at + * `rotate: 90deg`, and a real off-grip drag moving the split 50 → 75.21) is + * verified in a browser instead — see the PR. + * + * The load-bearing case is the one that reads backwards: `aria-orientation` + * describes the SEPARATOR, so a `orientation="vertical"` group (stacked panels) + * has a `aria-orientation="horizontal"` divider. Keying the stacked styles on + * `aria-[orientation=vertical]` would be the easy, silent way to reintroduce + * exactly this bug — hence the first test. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; + +import { + ResizableHandle, + ResizablePanel, + ResizablePanelGroup, +} from '../custom/resizable'; + +afterEach(cleanup); + +function renderGroup(orientation: 'horizontal' | 'vertical') { + const { container } = render( + + one + + two + , + ); + const separator = container.querySelector('[role="separator"]'); + if (!separator) throw new Error('no separator rendered'); + return { container, separator }; +} + +/** + * Every `aria-[name=value]:`-prefixed utility on the element, as the + * [attribute, value] pair the variant actually selects on — Tailwind's `aria-[…]` + * variant prepends `aria-` to the bracketed name, so `aria-[orientation=horizontal]:` + * compiles to `&[aria-orientation="horizontal"]`. + */ +function ariaVariantsOn(el: HTMLElement): Array<[string, string]> { + return [...el.classList].flatMap((cls) => { + const m = /(?:^|:)aria-\[([a-z-]+)=([^\]]+)\]:/.exec(cls); + return m ? [[`aria-${m[1]}`, m[2]] as [string, string]] : []; + }); +} + +describe('custom/resizable — the stacked divider (react-resizable-panels v4)', () => { + it('gives the stacked divider aria-orientation=horizontal, not vertical', () => { + // The inversion that makes this bug easy to reintroduce: the group stacks + // vertically, so the rule between the panels runs horizontally. + expect(renderGroup('vertical').separator.getAttribute('aria-orientation')).toBe('horizontal'); + expect(renderGroup('horizontal').separator.getAttribute('aria-orientation')).toBe('vertical'); + }); + + it('keys the stacked divider on that attribute, so a hairline replaces the sliver', () => { + const { separator } = renderGroup('vertical'); + const classes = [...separator.classList]; + + // The rule: full-width, 1px tall — overriding the base `w-px` that made it a + // vertical sliver in a stacked group. + expect(classes).toContain('aria-[orientation=horizontal]:h-px'); + expect(classes).toContain('aria-[orientation=horizontal]:w-full'); + // The `::after` drag target: a strip ALONG the rule, not across it. + expect(classes).toContain('aria-[orientation=horizontal]:after:h-1'); + expect(classes).toContain('aria-[orientation=horizontal]:after:w-full'); + expect(classes).toContain('aria-[orientation=horizontal]:after:left-0'); + expect(classes).toContain('aria-[orientation=horizontal]:after:translate-x-0'); + expect(classes).toContain('aria-[orientation=horizontal]:after:-translate-y-1/2'); + // The grip crosses the rule only when rotated. + expect(classes).toContain('[&[aria-orientation=horizontal]>div]:rotate-90'); + }); + + it('leaves the side-by-side divider on the base classes', () => { + const { separator } = renderGroup('horizontal'); + // Same class string either way — nothing above matches, so the base + // `w-px` / `after:left-1/2` / `after:-translate-x-1/2` styling stands and + // the grip is not rotated. This is the case that already worked; it must + // keep working. + expect(separator.getAttribute('aria-orientation')).toBe('vertical'); + for (const [attr, value] of ariaVariantsOn(separator)) { + expect(separator.matches(`[${attr}="${value}"]`)).toBe(false); + } + }); + + it('only keys on attributes the installed library actually emits', () => { + // The guard that would have caught the original bug, and catches the next + // rename: every `aria-[…]` variant the wrapper adds must match the real DOM + // in the orientation it targets. A variant naming an attribute/value the + // library never writes is a dead selector — which is exactly what + // `data-[panel-group-direction=vertical]` became when v4 landed. + const { separator } = renderGroup('vertical'); + const variants = ariaVariantsOn(separator); + + expect(variants.length).toBeGreaterThan(0); + for (const [attr, value] of variants) { + expect( + separator.matches(`[${attr}="${value}"]`), + `dead selector: no [${attr}="${value}"] on the rendered separator`, + ).toBe(true); + } + }); + + it('documents why the wrapper exists: v4 emits no panel-group-direction', () => { + // If this ever fails, the library started emitting the v3 attribute again + // and `ui/resizable.tsx`'s own variants would carry the stacked case — at + // which point this wrapper is redundant rather than load-bearing. + const { container } = renderGroup('vertical'); + expect(container.querySelectorAll('[data-panel-group-direction]')).toHaveLength(0); + }); +}); diff --git a/packages/components/src/custom/index.ts b/packages/components/src/custom/index.ts index 3db4a40f8..1e1438985 100644 --- a/packages/components/src/custom/index.ts +++ b/packages/components/src/custom/index.ts @@ -12,6 +12,7 @@ export * from './item'; export * from './kbd'; export * from './native-select'; export * from './navigation-overlay'; +export * from './resizable'; export * from './section-header'; export * from './spinner'; export * from './sort-builder'; diff --git a/packages/components/src/custom/navigation-overlay.tsx b/packages/components/src/custom/navigation-overlay.tsx index 971bc9159..5860463f1 100644 --- a/packages/components/src/custom/navigation-overlay.tsx +++ b/packages/components/src/custom/navigation-overlay.tsx @@ -64,7 +64,7 @@ import { ResizablePanelGroup, ResizablePanel, ResizableHandle, -} from '../ui/resizable'; +} from './resizable'; import { usePopperAwareInteractOutside } from './mobile-dialog-content'; /** Navigation mode type — matches ViewNavigationConfig.mode */ diff --git a/packages/components/src/custom/resizable.tsx b/packages/components/src/custom/resizable.tsx new file mode 100644 index 000000000..60c9436dd --- /dev/null +++ b/packages/components/src/custom/resizable.tsx @@ -0,0 +1,85 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use client'; + +import * as React from 'react'; + +import { cn } from '../lib/utils'; +import { + ResizableHandle as ShadcnResizableHandle, + ResizablePanel, + ResizablePanelGroup, +} from '../ui/resizable'; + +/** + * Orientation-correct `ResizableHandle`. + * + * `ui/resizable.tsx` (a no-touch Shadcn-sync file, AGENTS.md #7) is from the + * `react-resizable-panels` **v3** generation: every one of its stacked-group + * styles is keyed on `data-[panel-group-direction=vertical]`. v4 — the + * installed major — never emits that attribute. The only data attributes it + * puts in the DOM are `data-group`, `data-panel`, `data-separator`, + * `data-disabled` and `data-testid`, so all seven variants plus the grip's + * `[&[data-panel-group-direction=vertical]>div]:rotate-90` are dead selectors. + * + * The visible consequence: in a **stacked** group the divider kept the base + * `w-px` and rendered as a 1px-wide, 16px-tall sliver (its whole height came + * from the grip) instead of a full-width 1px-tall rule, and the grip never + * rotated. Measured in a browser before this wrapper existed: 1×16px, with a + * 4×16px `::after` hit area pinned to the group's left edge. + * + * What v4 *does* emit on the separator is `aria-orientation`, so that is what + * these variants key on — matching upstream Shadcn, which has since been + * regenerated for v4 the same way. + * + * ⚠️ The attribute value is the **inverse** of the group's `orientation`, and + * deliberately so: `aria-orientation` describes the separator, not the group. + * A `orientation="vertical"` group stacks its panels, so the divider between + * them is a *horizontal* line — hence `aria-orientation="horizontal"`. + * + * group orientation="horizontal" → panels side-by-side → separator aria-orientation="vertical" (base classes) + * group orientation="vertical" → panels stacked → separator aria-orientation="horizontal" (these classes) + * + * Each variant below is the live counterpart of a dead `data-[…]` one still + * carried by the base class string. They win on specificity: an attribute + * selector scores above the bare utility class it overrides, which is the same + * mechanism upstream relies on within its single file. + */ +const HORIZONTAL_SEPARATOR_CLASSES = [ + // The rule itself: a full-width hairline instead of a 1px-wide sliver. + 'aria-[orientation=horizontal]:h-px', + 'aria-[orientation=horizontal]:w-full', + // The `::after` drag hit area: a 4px strip along the rule, not across it. + 'aria-[orientation=horizontal]:after:left-0', + 'aria-[orientation=horizontal]:after:h-1', + 'aria-[orientation=horizontal]:after:w-full', + 'aria-[orientation=horizontal]:after:translate-x-0', + 'aria-[orientation=horizontal]:after:-translate-y-1/2', + // The grip glyph reads as a drag affordance only when it crosses the rule. + '[&[aria-orientation=horizontal]>div]:rotate-90', +].join(' '); + +export type ResizableHandleProps = React.ComponentProps; + +const ResizableHandle = ({ className, ...props }: ResizableHandleProps) => ( + +); + +/** + * `ResizablePanelGroup` and `ResizablePanel` need no wrapping and are + * re-exported as-is: the group's `flex-direction` comes from an inline style + * the library writes itself (`row` / `column` from `orientation`), so its own + * dead `data-[panel-group-direction=vertical]:flex-col` is inert but harmless. + * They pass through here purely so callers have one import site for the trio. + * + * These are the *only* public `Resizable*` exports — `ui/index.ts` deliberately + * does not re-export `./resizable`, so no consumer can reach the unpatched + * handle through `@object-ui/components` and silently lose the stacked case. + */ +export { ResizableHandle, ResizablePanel, ResizablePanelGroup }; diff --git a/packages/components/src/renderers/complex/resizable.tsx b/packages/components/src/renderers/complex/resizable.tsx index 626fd0cb5..ab16d8024 100644 --- a/packages/components/src/renderers/complex/resizable.tsx +++ b/packages/components/src/renderers/complex/resizable.tsx @@ -9,11 +9,11 @@ import React from 'react'; import { ComponentRegistry } from '@object-ui/core'; import type { ResizableSchema } from '@object-ui/types'; -import { - ResizablePanelGroup, - ResizablePanel, - ResizableHandle -} from '../../ui'; +import { + ResizablePanelGroup, + ResizablePanel, + ResizableHandle +} from '../../custom/resizable'; import { renderChildren } from '../../lib/utils'; ComponentRegistry.register('resizable', diff --git a/packages/components/src/renderers/form/__tests__/form-field-panes.test.tsx b/packages/components/src/renderers/form/__tests__/form-field-panes.test.tsx index f7db5d830..61bcff558 100644 --- a/packages/components/src/renderers/form/__tests__/form-field-panes.test.tsx +++ b/packages/components/src/renderers/form/__tests__/form-field-panes.test.tsx @@ -214,4 +214,24 @@ describe('FormSchema.fieldPanes — one form across the split (#2153)', () => { const separator = container.querySelector('[role="separator"]')!; expect(separator.getAttribute('aria-disabled')).not.toBe('true'); }); + + it('gives a vertical split a full-width divider, not a 1px sliver', () => { + // `fieldPanesOrientation: 'vertical'` stacks the panes, so the divider is a + // horizontal rule. It used to render as a 1px-wide vertical sliver: the + // Shadcn handle styled that case with `data-[panel-group-direction=vertical]` + // variants, which react-resizable-panels v4 never triggers. The form renders + // through `custom/resizable`, which re-keys them onto the separator's own + // `aria-orientation` — see custom/resizable.tsx and + // __tests__/resizable-orientation.test.tsx. + const { container } = renderSplitForm({ fieldPanesOrientation: 'vertical' }); + const separator = container.querySelector('[role="separator"]')!; + + expect(separator.getAttribute('aria-orientation')).toBe('horizontal'); + expect([...separator.classList]).toContain('aria-[orientation=horizontal]:w-full'); + + // …and the default (side-by-side) split still gets the vertical divider. + cleanup(); + const sideBySide = renderSplitForm().container.querySelector('[role="separator"]')!; + expect(sideBySide.getAttribute('aria-orientation')).toBe('vertical'); + }); }); diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index a469c19b7..58b266ce8 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -24,7 +24,7 @@ import { } from '../../ui/select'; import { renderChildren } from '../../lib/utils'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '../../ui/tabs'; -import { ResizablePanelGroup, ResizablePanel, ResizableHandle } from '../../ui/resizable'; +import { ResizablePanelGroup, ResizablePanel, ResizableHandle } from '../../custom/resizable'; import { Alert, AlertDescription } from '../../ui/alert'; import { toast } from '../../ui/sonner'; import { AlertCircle, ChevronDown, ChevronRight, Loader2, Maximize2, Check, X } from 'lucide-react'; diff --git a/packages/components/src/ui/index.ts b/packages/components/src/ui/index.ts index ef900fb3b..e77bc2016 100644 --- a/packages/components/src/ui/index.ts +++ b/packages/components/src/ui/index.ts @@ -36,7 +36,11 @@ export * from './pagination'; export * from './popover'; export * from './progress'; export * from './radio-group'; -export * from './resizable'; +// NOT re-exported: `./resizable` ships v3-era `data-[panel-group-direction]` +// styles that react-resizable-panels v4 never triggers, so its handle has no +// stacked-group appearance. `custom/resizable.tsx` wraps it and owns the public +// `Resizable*` names — exporting both here would also collide (TS2308), since +// `src/index.ts` star-exports `./ui` and `./custom` side by side. export * from './scroll-area'; export * from './select'; export * from './separator';