From b1c05f1945e5fb6dea89d15ae923aa23c9a44b5e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 00:06:23 +0000 Subject: [PATCH] fix(app-shell): let the icon-mode sidebar rail scroll vertically In the collapsed (icon-mode) sidebar the SidebarContent primitive clips with a variant-scoped overflow-hidden, so rows below the fold could not be reached until the sidebar expanded (objectui#12794). UnifiedSidebar passes the same variant with overflow-auto (which tailwind-merge lets replace the primitive's shorthand) plus overflow-x-hidden, and hides the rail's scrollbar so a classic bar does not take its width out of the 3rem rail. Off icon mode the class list is unchanged. Claude-Session: https://claude.ai/code/session_0129XjRmiNNiNRzQ5twZ75Vr Co-authored-by: Claude --- .changeset/12794-icon-rail-scroll.md | 9 + .../app-shell/src/layout/UnifiedSidebar.tsx | 26 ++- ...ifiedSidebar.iconRailScroll-12794.test.tsx | 181 ++++++++++++++++++ 3 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 .changeset/12794-icon-rail-scroll.md create mode 100644 packages/app-shell/src/layout/__tests__/UnifiedSidebar.iconRailScroll-12794.test.tsx diff --git a/.changeset/12794-icon-rail-scroll.md b/.changeset/12794-icon-rail-scroll.md new file mode 100644 index 0000000000..dec8d2b820 --- /dev/null +++ b/.changeset/12794-icon-rail-scroll.md @@ -0,0 +1,9 @@ +--- +'@object-ui/app-shell': patch +--- + +In the collapsed (icon-mode) console sidebar, the rail now scrolls vertically when its rows overflow, as the expanded sidebar does. Before, the rows below the fold could not be reached until the sidebar was expanded again (objectui#12794). + +The `SidebarContent` primitive clips its content in icon mode with `overflow-hidden`. `UnifiedSidebar` now passes the same icon-mode variant with `overflow-auto` and `overflow-x-hidden`, so the rail scrolls up and down but never sideways. It draws no scrollbar, because a scrollbar would take its width out of the 3rem rail and push the icons off-centre. The mouse wheel, touch and keyboard focus still scroll it. The rail's width and the icons' centring are unchanged. + +Off icon mode nothing changes: the expanded sidebar and the mobile sheet scroll as before, with the host's scrollbar. The primitive itself is a Shadcn-synced file and is not edited, and no export, prop or type member is added. diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index c1d484813a..6ce907421c 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -85,6 +85,30 @@ import { SidebarItemMenu, sidebarItemDesignPath, favoriteDesignPath } from './Si const HIDDEN_ICON_LABEL_NO_POINTER = 'group-data-[state=collapsed]:group-data-[collapsible=icon]:pointer-events-none'; +/** + * The classes the sidebar content carries so that, once the sidebar collapses + * to icons, the rail scrolls vertically when its rows overflow (objectui#12794). + * The `SidebarContent` primitive sets `overflow-hidden` under this variant, so + * the rows below the fold could not be reached until the sidebar expanded. + * + * `cn` (tailwind-merge) drops the primitive's same-variant `overflow-hidden` + * only for a later same-variant `overflow-*` shorthand; an `overflow-y-*` alone + * leaves it in place and lets stylesheet order decide. So the shorthand here + * is `overflow-auto`, and `overflow-x-hidden` keeps the horizontal axis + * clipped: the rail's rows are wider than the 3rem rail, and it never scrolls + * sideways. The rail draws no scrollbar: a classic bar takes its width out of + * the 3rem rail, so the icon buttons would run under it and sit off-centre. + * The wheel, touch and keyboard focus still scroll it. Marked important + * because a host may theme every scrollbar with an unlayered `*` rule (the + * console does), which outranks any layered utility. Off icon mode nothing + * changes: the expanded sidebar and the mobile sheet keep the primitive's own + * `overflow-auto` and the host's scrollbar. + */ +const ICON_RAIL_SCROLLS_VERTICALLY = + 'group-data-[state=collapsed]:group-data-[collapsible=icon]:overflow-auto ' + + 'group-data-[state=collapsed]:group-data-[collapsible=icon]:overflow-x-hidden ' + + 'group-data-[state=collapsed]:group-data-[collapsible=icon]:[scrollbar-width:none]!'; + /** * Lazy-resolved Lucide icon — see ../utils/getIcon for impl. * The local symbol is kept for backwards compat with existing call sites @@ -478,7 +502,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { )} - +
{context === 'app' && activeApp ? ( <> diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.iconRailScroll-12794.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.iconRailScroll-12794.test.tsx new file mode 100644 index 0000000000..bd70ee2f64 --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.iconRailScroll-12794.test.tsx @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * UnifiedSidebar — in the collapsed (icon-mode) sidebar the rail scrolls + * vertically when its rows overflow (objectui#12794). + * + * The `SidebarContent` primitive sets `overflow-hidden` under its icon-mode + * variant, so the rows below the fold could not be reached until the sidebar + * expanded (measured in a browser; the readings are on the pull request). The + * call site now passes a same-variant `overflow-auto` with `overflow-x-hidden` + * and no scrollbar. The pins DERIVE the variant from the bare primitive's own + * `…:overflow-hidden` token, so a primitive that changes its icon-mode + * condition turns this red instead of leaving a variant that no longer + * matches. The test DOM computes no Tailwind, so the pins read the class list. + * + * Harness: the provider / chrome mocks of + * `UnifiedSidebar.iconModeRecentLabel-12744.test.tsx`; `@object-ui/layout` and + * `@object-ui/components` (the real `SidebarProvider`, `Sidebar` and + * `SidebarContent`) stay REAL. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + language: 'en', + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + appLabel: ({ label }: { label?: string }) => label, + }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: null, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), +})); + +vi.mock('@object-ui/permissions', async (importOriginal) => ({ + ...(await importOriginal()), + usePermissions: () => ({ can: () => true, hasCapabilities: () => true }), +})); + +let metadataState: { apps: unknown[]; objects: unknown[] }; +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => metadataState, +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', + evaluateNavigationVisibility: (expr: unknown) => expr !== false && expr !== 'false', +})); + +vi.mock('../../utils', () => ({ + resolveKeyedI18nLabel: (label: unknown) => (typeof label === 'string' ? label : ''), + matchAppBySegment: (apps: Array<{ name?: string }>, segment?: string) => + apps.find((a) => a?.name === segment), + appRouteSegment: (app: { name?: string }) => app?.name, +})); + +vi.mock('../../utils/getIcon', () => ({ getIcon: () => () => null })); +vi.mock('../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) })); +vi.mock('../../hooks/useFavorites', () => ({ + useFavorites: () => ({ favorites: [], removeFavorite: vi.fn() }), +})); +vi.mock('../../hooks/useNavPins', () => ({ + useNavPins: () => ({ togglePin: vi.fn(), applyPins: (items: unknown) => items }), +})); +vi.mock('../../hooks/useNavActionDispatch', () => ({ + useNavActionDispatch: () => vi.fn(), +})); +vi.mock('../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ context: 'app', currentAppName: 'crm' }), +})); +vi.mock('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), + contextSelectorQueryKey: (id: string) => (id === 'active_package' ? 'package' : id), + STUDIO_PACKAGE_SELECTOR_ID: 'active_package', +})); +vi.mock('../LocalizedSidebarTrigger', () => ({ + LocalizedSidebarTrigger: () => null, +})); + +import { Sidebar, SidebarContent, SidebarProvider } from '@object-ui/components'; +import { UnifiedSidebar } from '../UnifiedSidebar'; + +function sidebarUi(open: boolean) { + metadataState = { + apps: [ + { + name: 'crm', + label: 'CRM', + active: true, + navigation: [{ id: 'nav_accounts', type: 'object', objectName: 'account', label: 'Accounts' }], + }, + ], + objects: [], + }; + return ( + + + + + + ); +} + +const tokens = (el: Element) => el.className.split(/\s+/).filter(Boolean); +const contentOf = (container: HTMLElement) => + container.querySelector('[data-sidebar="content"]') as HTMLElement; + +/** + * The bare primitive's class list, with the call site's old `pt-2`: what the + * content element carried before this fix, and the source of the icon-mode + * variant the fix has to override. + */ +function primitiveTokens(open: boolean): string[] { + const { container, unmount } = render( + + + + + , + ); + const list = tokens(contentOf(container)); + unmount(); + return list; +} + +/** The variant prefix the primitive clips the rail under, read off its `…:overflow-hidden` token. */ +function iconModePrefix(): string { + const clip = primitiveTokens(false).find((t) => t.endsWith(':overflow-hidden')); + expect(clip, 'the primitive clips the icon rail with a variant-scoped overflow-hidden').toBeDefined(); + return clip!.slice(0, -'overflow-hidden'.length); +} + +beforeEach(() => { + localStorage.clear(); + document.cookie = 'sidebar_state=; path=/; max-age=0'; +}); + +describe('UnifiedSidebar — the icon rail scrolls vertically when its rows overflow (objectui#12794)', () => { + it('collapsed to icons: the rail scrolls vertically, stays clipped sideways and draws no scrollbar', () => { + const prefix = iconModePrefix(); + const { container } = render(sidebarUi(false)); + const peer = container.querySelector('.group.peer[data-state]'); + expect(peer).toHaveAttribute('data-state', 'collapsed'); + expect(peer).toHaveAttribute('data-collapsible', 'icon'); + + const list = tokens(contentOf(container)); + expect(list).toContain(`${prefix}overflow-auto`); + expect(list).toContain(`${prefix}overflow-x-hidden`); + expect(list).toContain(`${prefix}[scrollbar-width:none]!`); + // No icon-mode clip survives the merge to compete with the shorthand. + expect(list).not.toContain(`${prefix}overflow-hidden`); + expect(list).not.toContain(`${prefix}overflow-y-hidden`); + expect(list).not.toContain(`${prefix}overflow-y-clip`); + expect(list).not.toContain(`${prefix}overflow-clip`); + }); + + it('expanded: every class that applies off icon mode is what it was before (CONTROL)', () => { + const prefix = iconModePrefix(); + const before = primitiveTokens(true).filter((t) => !t.startsWith(prefix)); + expect(before).toContain('overflow-auto'); + + const { container } = render(sidebarUi(true)); + expect(container.querySelector('.group.peer[data-state]')).toHaveAttribute('data-state', 'expanded'); + const now = tokens(contentOf(container)).filter((t) => !t.startsWith(prefix)); + expect(now).toEqual(before); + }); +});