From 270669cdf4ede1bc96936cede864490a027eff19 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 00:29:14 +0000 Subject: [PATCH 1/2] fix(app-shell): AppSidebar/UnifiedSidebar area switchers adopt derived area visibility (#3319) Both inline area switchers now reuse @object-ui/layout's hasVisibleNavigationItems predicate (#3311): an area is offered iff at least one of its navigation items survives the item-level guards, and the active area is elected among the VISIBLE areas only (first visible by default; re-elected when the active area is gated away; a mere reveal never steals the user's selection). Also tightens areas: any[] to NavigationArea[]. No authorable area-level key is introduced. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa --- packages/app-shell/src/layout/AppSidebar.tsx | 117 +++++--- .../app-shell/src/layout/UnifiedSidebar.tsx | 152 ++++++---- .../AppSidebar.derivedAreaVisibility.test.tsx | 245 ++++++++++++++++ ...fiedSidebar.derivedAreaVisibility.test.tsx | 268 ++++++++++++++++++ 4 files changed, 675 insertions(+), 107 deletions(-) create mode 100644 packages/app-shell/src/layout/__tests__/AppSidebar.derivedAreaVisibility.test.tsx create mode 100644 packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx diff --git a/packages/app-shell/src/layout/AppSidebar.tsx b/packages/app-shell/src/layout/AppSidebar.tsx index 4effbc2e7e..23c137fe9c 100644 --- a/packages/app-shell/src/layout/AppSidebar.tsx +++ b/packages/app-shell/src/layout/AppSidebar.tsx @@ -53,8 +53,8 @@ import { Home, ListTree, } from 'lucide-react'; -import { NavigationRenderer, resolveHref, resolveActiveNavItem } from '@object-ui/layout'; -import type { NavigationItem } from '@object-ui/types'; +import { NavigationRenderer, resolveHref, resolveActiveNavItem, hasVisibleNavigationItems } from '@object-ui/layout'; +import type { NavigationArea, NavigationItem } from '@object-ui/types'; import { useMetadata } from '../providers/MetadataProvider'; import { useExpressionContext, evaluateVisibility } from '../providers/ExpressionProvider'; import { useAuth, useIsWorkspaceAdmin, getUserInitials } from '@object-ui/auth'; @@ -191,45 +191,6 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri // Navigation pin persistence via localStorage const { togglePin, applyPins } = useNavPins(); - // Area management — track selected area when app defines areas - const areas: any[] = activeApp?.areas || []; - const [activeAreaId, setActiveAreaId] = React.useState( - () => areas.length > 0 ? areas[0].id : null, - ); - - // Reset area when app changes or areas become available - React.useEffect(() => { - if (areas.length > 0) { - setActiveAreaId(prev => areas.some((a: any) => a.id === prev) ? prev : areas[0].id); - } else { - setActiveAreaId(null); - } - }, [activeAppName, areas.length]); - - // Resolve navigation items: area navigation > flat navigation > empty - const activeArea = areas.find((a: any) => a.id === activeAreaId); - const resolvedNavigation: NavigationItem[] = activeArea?.navigation || activeApp?.navigation || []; - - // App-level context selectors (e.g. Studio's package scope). Their - // values are injected into nav items as `{}` template vars. - const { contextValues, element: contextSelectorsUI } = useAppContextSelectors( - activeAppName, - activeApp?.contextSelectors, - t, - ); - - // Apply saved order and pin state to navigation items - const processedNavigation = React.useMemo(() => { - const ordered = applyOrder(resolvedNavigation); - return applyPins(ordered); - }, [resolvedNavigation, applyOrder, applyPins]); - - // Search filter state for sidebar navigation - const [navSearchQuery, setNavSearchQuery] = React.useState(''); - - // Recent section collapsed by default - const [recentExpanded, setRecentExpanded] = React.useState(false); - // Visibility evaluation from Console expression context const { evaluator } = useExpressionContext(); const evalVis = React.useCallback( @@ -277,6 +238,71 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri [registeredObjectNames], ); + // Area management — track selected area when app defines areas. + // + // Area visibility is DERIVED from the items inside (objectui#3311 / + // objectui#3319): the switcher offers an area iff at least one of its + // navigation items survives the same item-level guards NavigationRenderer + // applies (`visible`, `requiredPermissions`, runtime capabilities, + // action-dispatcher presence). The active area is elected among the + // VISIBLE areas only, so the user is never landed in — or stranded on — + // an area that renders nothing. Same derivation as `AppSchemaRenderer` + // (@object-ui/layout); the predicate is shared, not re-implemented. + const areas: NavigationArea[] = activeApp?.areas || []; + const visibleAreas = areas.filter((area) => + hasVisibleNavigationItems(area.navigation, { + evaluateVisibility: evalVis, + checkPermission: checkPerm, + checkCapability: checkCap, + // This sidebar wires no `onAction` on its NavigationRenderer, so + // `action` items never render here and cannot carry an area's + // visibility either (framework#4509). + hasActionHandler: false, + }), + ); + const [activeAreaId, setActiveAreaId] = React.useState( + () => visibleAreas.length > 0 ? visibleAreas[0].id : null, + ); + + const visibleAreaIds = visibleAreas.map((a) => a.id).join(','); + + // Re-elect when the app changes or the visible-area set changes. Keeping + // `prev` whenever it is still visible means merely REVEALING a new area + // never steals the user's current selection. + React.useEffect(() => { + if (visibleAreas.length > 0) { + setActiveAreaId(prev => visibleAreas.some((a) => a.id === prev) ? prev : visibleAreas[0].id); + } else { + setActiveAreaId(null); + } + }, [activeAppName, visibleAreaIds]); + + // Resolve navigation items: area navigation > flat navigation > empty. + // The render-time `?? visibleAreas[0]` fallback covers the frame between + // a gating change hiding the active area and the effect above re-electing. + const activeArea = visibleAreas.find((a) => a.id === activeAreaId) ?? visibleAreas[0]; + const resolvedNavigation: NavigationItem[] = activeArea?.navigation || activeApp?.navigation || []; + + // App-level context selectors (e.g. Studio's package scope). Their + // values are injected into nav items as `{}` template vars. + const { contextValues, element: contextSelectorsUI } = useAppContextSelectors( + activeAppName, + activeApp?.contextSelectors, + t, + ); + + // Apply saved order and pin state to navigation items + const processedNavigation = React.useMemo(() => { + const ordered = applyOrder(resolvedNavigation); + return applyPins(ordered); + }, [resolvedNavigation, applyOrder, applyPins]); + + // Search filter state for sidebar navigation + const [navSearchQuery, setNavSearchQuery] = React.useState(''); + + // Recent section collapsed by default + const [recentExpanded, setRecentExpanded] = React.useState(false); + const basePath = activeApp ? `/apps/${appRouteSegment(activeApp) ?? activeAppName}` : ''; // Fallback system navigation when no active app exists — routes into the Setup app. @@ -435,8 +461,9 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri {activeApp ? ( <> - {/* Area Switcher — shown when app defines areas */} - {areas.length > 1 && ( + {/* Area Switcher — offered only when MULTIPLE areas are visible; + area visibility is derived from the items inside (#3319) */} + {visibleAreas.length > 1 && ( @@ -444,9 +471,9 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri - {areas.map((area: any) => { + {visibleAreas.map((area) => { const AreaIcon = getIcon(area.icon); - const isActiveArea = area.id === activeAreaId; + const isActiveArea = area.id === activeArea?.id; return ( evaluateVisibility(expr, evaluator), + [evaluator], + ); + + // Permission check for nav `requiredPermissions` entries. + // + // Two authored forms: + // - `object:action` → object CRUD gate. + // - bare name → an ADR-0066 system capability, checked against the union of + // the user's permission-set `systemPermissions` (from /me/permissions) — + // the SAME subset rule the server applies to `AppSchema.requiredPermissions`. + // This used to be misread as `can(, 'read')` only, so a nav item + // requiring a capability was hidden even from users whose permission set + // granted it (admins included) — `requiredPermissions` degenerated into a + // "hide from everyone" switch. The object-read fallback stays for nav + // items that gate on a plain object name. + const { can, hasCapabilities } = usePermissions(); + const checkPerm = React.useCallback( + (permissions: string[]) => permissions.every((perm: string) => { + const parts = perm.split(':'); + if (parts.length >= 2) { + return can(parts[0], parts[1] as any); + } + return hasCapabilities([perm]) || can(perm, 'read'); + }), + [can, hasCapabilities], + ); + + // Runtime capability gate: hide nav items targeting objects/services + // not registered in this runtime (e.g. cloud-only `sys_app`). + const registeredObjectNames = React.useMemo( + () => new Set((metadataObjects || []).map((o: any) => o?.name).filter(Boolean)), + [metadataObjects], + ); + const checkCap = React.useCallback( + (kind: 'object' | 'service', name: string): boolean => { + if (kind === 'object') { + if (registeredObjectNames.size === 0) return true; + return registeredObjectNames.has(name); + } + return true; + }, + [registeredObjectNames], + ); + + // Area management. + // + // Area visibility is DERIVED from the items inside (objectui#3311 / + // objectui#3319): the switcher offers an area iff at least one of its + // navigation items survives the same item-level guards NavigationRenderer + // applies (`visible`, `requiredPermissions`, runtime capabilities, + // action-dispatcher presence). The active area is elected among the + // VISIBLE areas only, so the user is never landed in — or stranded on — + // an area that renders nothing. Same derivation as `AppSchemaRenderer` + // (@object-ui/layout); the predicate is shared, not re-implemented. + const areas: NavigationArea[] = activeApp?.areas || []; + const visibleAreas = areas.filter((area) => + hasVisibleNavigationItems(area.navigation, { + evaluateVisibility: evalVis, + checkPermission: checkPerm, + checkCapability: checkCap, + // This sidebar always wires `onAction={dispatchNavAction}` on its + // NavigationRenderer (framework#4509), so `action` items render and + // count as area content. + hasActionHandler: !!dispatchNavAction, + }), + ); const [activeAreaId, setActiveAreaId] = React.useState( - () => areas.length > 0 ? areas[0].id : null, + () => visibleAreas.length > 0 ? visibleAreas[0].id : null, ); + const visibleAreaIds = visibleAreas.map((a) => a.id).join(','); + + // Re-elect when the app changes or the visible-area set changes. Keeping + // `prev` whenever it is still visible means merely REVEALING a new area + // never steals the user's current selection. React.useEffect(() => { - if (areas.length > 0) { - setActiveAreaId(prev => areas.some((a: any) => a.id === prev) ? prev : areas[0].id); + if (visibleAreas.length > 0) { + setActiveAreaId(prev => visibleAreas.some((a) => a.id === prev) ? prev : visibleAreas[0].id); } else { setActiveAreaId(null); } - }, [activeApp?.name, areas.length]); + }, [activeApp?.name, visibleAreaIds]); - // Resolve navigation items - const activeArea = areas.find((a: any) => a.id === activeAreaId); + // Resolve navigation items. The render-time `?? visibleAreas[0]` fallback + // covers the frame between a gating change hiding the active area and the + // effect above re-electing. + const activeArea = visibleAreas.find((a) => a.id === activeAreaId) ?? visibleAreas[0]; const appNavigation: NavigationItem[] = activeArea?.navigation || activeApp?.navigation || []; // App-level context selectors (e.g. Studio's package scope). Their @@ -310,54 +385,6 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { // Recent section collapsed by default const [recentExpanded, setRecentExpanded] = React.useState(false); - // Visibility evaluation - const { evaluator } = useExpressionContext(); - const evalVis = React.useCallback( - (expr: string | boolean | undefined) => evaluateVisibility(expr, evaluator), - [evaluator], - ); - - // Permission check for nav `requiredPermissions` entries. - // - // Two authored forms: - // - `object:action` → object CRUD gate. - // - bare name → an ADR-0066 system capability, checked against the union of - // the user's permission-set `systemPermissions` (from /me/permissions) — - // the SAME subset rule the server applies to `AppSchema.requiredPermissions`. - // This used to be misread as `can(, 'read')` only, so a nav item - // requiring a capability was hidden even from users whose permission set - // granted it (admins included) — `requiredPermissions` degenerated into a - // "hide from everyone" switch. The object-read fallback stays for nav - // items that gate on a plain object name. - const { can, hasCapabilities } = usePermissions(); - const checkPerm = React.useCallback( - (permissions: string[]) => permissions.every((perm: string) => { - const parts = perm.split(':'); - if (parts.length >= 2) { - return can(parts[0], parts[1] as any); - } - return hasCapabilities([perm]) || can(perm, 'read'); - }), - [can, hasCapabilities], - ); - - // Runtime capability gate: hide nav items targeting objects/services - // not registered in this runtime (e.g. cloud-only `sys_app`). - const registeredObjectNames = React.useMemo( - () => new Set((metadataObjects || []).map((o: any) => o?.name).filter(Boolean)), - [metadataObjects], - ); - const checkCap = React.useCallback( - (kind: 'object' | 'service', name: string): boolean => { - if (kind === 'object') { - if (registeredObjectNames.size === 0) return true; - return registeredObjectNames.has(name); - } - return true; - }, - [registeredObjectNames], - ); - const isStudioHomeActive = isStudioApp && location.pathname.replace(/\/+$/, '') === basePath; return ( @@ -420,8 +447,9 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { )} - {/* Area Switcher */} - {areas.length > 1 && ( + {/* Area Switcher — offered only when MULTIPLE areas are visible; + area visibility is derived from the items inside (#3319) */} + {visibleAreas.length > 1 && ( @@ -429,9 +457,9 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { - {areas.map((area: any) => { + {visibleAreas.map((area) => { const AreaIcon = getIcon(area.icon); - const isActiveArea = area.id === activeAreaId; + const isActiveArea = area.id === activeArea?.id; return ( ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + navGroupLabel: (_a: string, _g: string, fallback?: string) => fallback, + }), +})); + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: null, signOut: vi.fn(), isAuthEnabled: false, activeOrganization: null }), + useIsWorkspaceAdmin: () => false, + getUserInitials: () => 'U', +})); + +// Mutable permission state — swapped per test, re-read on every render. +let permissionsState: { + can: (objectName: string, action: string) => boolean; + hasCapabilities: (caps: string[]) => boolean; +}; +vi.mock('@object-ui/permissions', () => ({ + usePermissions: () => permissionsState, +})); + +let metadataState: { apps: unknown[]; objects: unknown[] }; +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => metadataState, +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + // Mirrors the real evaluateVisibility's literal handling — enough for + // fixtures gating with `visible: false`. + evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', +})); + +vi.mock('../../utils', () => ({ + resolveI18nLabel: (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, +})); + +// Lazy lucide DynamicIcon would suspend mid-test; a null icon is enough here. +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('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), +})); + +import { SidebarProvider } from '@object-ui/components'; +import { AppSidebar } from '../AppSidebar'; + +// --------------------------------------------------------------------------- +// Fixtures — labels are pairwise distinct so a hit is unambiguous. +// --------------------------------------------------------------------------- + +const salesArea: NavigationArea = { + id: 'area-sales', + label: 'Sales', + navigation: [{ id: 'a1', type: 'object', label: 'Opportunities', objectName: 'opportunity' }], +}; + +const serviceArea: NavigationArea = { + id: 'area-service', + label: 'Service', + navigation: [{ id: 'a2', type: 'object', label: 'Cases', objectName: 'case' }], +}; + +const marketingArea: NavigationArea = { + id: 'area-marketing', + label: 'Marketing', + navigation: [{ id: 'a3', type: 'object', label: 'Campaigns', objectName: 'campaign' }], +}; + +const gatedSales: NavigationArea = { + ...salesArea, + navigation: [{ ...salesArea.navigation[0], visible: false }], +}; + +function sidebarUi(areas: NavigationArea[]) { + metadataState = { + apps: [{ name: 'crm', label: 'CRM', active: true, areas }], + objects: [], + }; + return ( + + + {}} /> + + + ); +} + +beforeEach(() => { + permissionsState = { can: () => true, hasCapabilities: () => true }; + localStorage.clear(); +}); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('AppSidebar derived area visibility (#3319)', () => { + it('keeps every area in the switcher when every area has a visible item', () => { + render(sidebarUi([salesArea, serviceArea, marketingArea])); + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Service')).toBeInTheDocument(); + expect(screen.getByText('Marketing')).toBeInTheDocument(); + // First area is active — only its navigation renders. + expect(screen.getByText('Opportunities')).toBeInTheDocument(); + expect(screen.queryByText('Cases')).not.toBeInTheDocument(); + + // Switching among visible areas still works. + fireEvent.click(screen.getByText('Marketing')); + expect(screen.getByText('Campaigns')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + + it('keeps a PARTIALLY gated area visible and active, hiding only the gated item', () => { + const partialSales: NavigationArea = { + ...salesArea, + navigation: [ + { ...salesArea.navigation[0], visible: false }, + { id: 'a1b', type: 'object', label: 'Quotes', objectName: 'quote' }, + ], + }; + render(sidebarUi([partialSales, serviceArea])); + // The area survives (it still has a visible item) and stays active… + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Quotes')).toBeInTheDocument(); + // …but the gated ITEM does not render (real NavigationRenderer guard). + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + + it('hides an area whose items are ALL gated and elects the first VISIBLE area', () => { + render(sidebarUi([gatedSales, serviceArea, marketingArea])); + // The fully gated area is not offered in the switcher… + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.getByText('Service')).toBeInTheDocument(); + expect(screen.getByText('Marketing')).toBeInTheDocument(); + // …and never auto-activated: the first VISIBLE area's navigation renders. + expect(screen.getByText('Cases')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + + it('hides the switcher entirely when only one area remains visible', () => { + render(sidebarUi([gatedSales, serviceArea])); + // One visible area = nothing to switch between: no switcher rows at all, + // but the visible area's navigation still renders. + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.queryByText('Service')).not.toBeInTheDocument(); + expect(screen.getByText('Cases')).toBeInTheDocument(); + }); + + it('renders no switcher and no area navigation when every area is fully gated', () => { + render( + sidebarUi([ + gatedSales, + { ...serviceArea, navigation: [{ ...serviceArea.navigation[0], visible: false }] }, + ]), + ); + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.queryByText('Service')).not.toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + expect(screen.queryByText('Cases')).not.toBeInTheDocument(); + }); + + it('re-elects when the ACTIVE area is gated away, and a mere reveal does not steal the selection', () => { + const adminSales: NavigationArea = { + ...salesArea, + navigation: [{ ...salesArea.navigation[0], requiredPermissions: ['sales:admin'] }], + }; + const areas = [adminSales, serviceArea, marketingArea]; + + const view = render(sidebarUi(areas)); + // Permission granted: Sales is visible and active. + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Opportunities')).toBeInTheDocument(); + + // Permission revoked (`object:action` form → can('sales','admin') false): + // Sales derives hidden, drops out of the switcher, and the sidebar + // re-elects the first visible area. + permissionsState = { + can: (objectName, action) => !(objectName === 'sales' && action === 'admin'), + hasCapabilities: () => true, + }; + view.rerender(sidebarUi(areas)); + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + expect(screen.getByText('Cases')).toBeInTheDocument(); + + // Permission granted again: Sales REAPPEARS in the switcher, but the + // user's current selection is not yanked away — Service stays active. + permissionsState = { can: () => true, hasCapabilities: () => true }; + view.rerender(sidebarUi(areas)); + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Cases')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx new file mode 100644 index 0000000000..e43bbdbf0e --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx @@ -0,0 +1,268 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * UnifiedSidebar — derived area visibility (objectui#3319). + * + * Same contract as the AppSidebar suite: the inline area switcher adopts the + * #3311 derivation via the SHARED `hasVisibleNavigationItems` predicate from + * `@object-ui/layout` — an area is offered iff something inside it renders, + * and the active area is elected among the visible areas only. + * + * One behavior is specific to this sidebar: it wires + * `onAction={dispatchNavAction}` (framework#4509), so an `action` nav item IS + * content and can carry its area's visibility (`hasActionHandler: true`). + * + * NB (lesson from #3322): a gated fixture is only load-bearing when it sits + * in the area that would otherwise be ACTIVE. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationArea } from '@object-ui/types'; + +// --------------------------------------------------------------------------- +// Mocks — providers and console-only chrome. @object-ui/components and +// @object-ui/layout stay REAL so the shared predicate and the item-level +// guards inside NavigationRenderer are actually exercised. +// --------------------------------------------------------------------------- + +// Partial mock: @object-ui/components also imports from @object-ui/i18n +// (createSafeTranslation), so the rest of the module must stay real. +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + navGroupLabel: (_a: string, _g: string, fallback?: string) => fallback, + }), +})); + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: null, activeOrganization: null }), + useIsWorkspaceAdmin: () => false, +})); + +// Mutable permission state — swapped per test, re-read on every render. +let permissionsState: { + can: (objectName: string, action: string) => boolean; + hasCapabilities: (caps: string[]) => boolean; +}; +vi.mock('@object-ui/permissions', () => ({ + usePermissions: () => permissionsState, +})); + +let metadataState: { apps: unknown[]; objects: unknown[] }; +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => metadataState, +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + // Mirrors the real evaluateVisibility's literal handling — enough for + // fixtures gating with `visible: false`. + evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', +})); + +vi.mock('../../utils', () => ({ + resolveI18nLabel: (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, +})); + +// Lazy lucide DynamicIcon would suspend mid-test; a null icon is enough here. +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 }), +})); +const dispatchNavAction = vi.fn(); +vi.mock('../../hooks/useNavActionDispatch', () => ({ + useNavActionDispatch: () => dispatchNavAction, +})); +vi.mock('../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ context: 'app', currentAppName: 'crm' }), +})); +vi.mock('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), +})); +vi.mock('../LocalizedSidebarTrigger', () => ({ + LocalizedSidebarTrigger: () => null, +})); + +import { SidebarProvider } from '@object-ui/components'; +import { UnifiedSidebar } from '../UnifiedSidebar'; + +// --------------------------------------------------------------------------- +// Fixtures — labels are pairwise distinct so a hit is unambiguous. +// --------------------------------------------------------------------------- + +const salesArea: NavigationArea = { + id: 'area-sales', + label: 'Sales', + navigation: [{ id: 'a1', type: 'object', label: 'Opportunities', objectName: 'opportunity' }], +}; + +const serviceArea: NavigationArea = { + id: 'area-service', + label: 'Service', + navigation: [{ id: 'a2', type: 'object', label: 'Cases', objectName: 'case' }], +}; + +const marketingArea: NavigationArea = { + id: 'area-marketing', + label: 'Marketing', + navigation: [{ id: 'a3', type: 'object', label: 'Campaigns', objectName: 'campaign' }], +}; + +const gatedSales: NavigationArea = { + ...salesArea, + navigation: [{ ...salesArea.navigation[0], visible: false }], +}; + +function sidebarUi(areas: NavigationArea[]) { + metadataState = { + apps: [{ name: 'crm', label: 'CRM', active: true, areas }], + objects: [], + }; + return ( + + + + + + ); +} + +beforeEach(() => { + permissionsState = { can: () => true, hasCapabilities: () => true }; + localStorage.clear(); +}); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('UnifiedSidebar derived area visibility (#3319)', () => { + it('keeps every area in the switcher when every area has a visible item', () => { + render(sidebarUi([salesArea, serviceArea, marketingArea])); + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Service')).toBeInTheDocument(); + expect(screen.getByText('Marketing')).toBeInTheDocument(); + // First area is active — only its navigation renders. + expect(screen.getByText('Opportunities')).toBeInTheDocument(); + expect(screen.queryByText('Cases')).not.toBeInTheDocument(); + + // Switching among visible areas still works. + fireEvent.click(screen.getByText('Marketing')); + expect(screen.getByText('Campaigns')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + + it('keeps a PARTIALLY gated area visible and active, hiding only the gated item', () => { + const partialSales: NavigationArea = { + ...salesArea, + navigation: [ + { ...salesArea.navigation[0], visible: false }, + { id: 'a1b', type: 'object', label: 'Quotes', objectName: 'quote' }, + ], + }; + render(sidebarUi([partialSales, serviceArea])); + // The area survives (it still has a visible item) and stays active… + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Quotes')).toBeInTheDocument(); + // …but the gated ITEM does not render (real NavigationRenderer guard). + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + + it('hides an area whose items are ALL gated and elects the first VISIBLE area', () => { + render(sidebarUi([gatedSales, serviceArea, marketingArea])); + // The fully gated area is not offered in the switcher… + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.getByText('Service')).toBeInTheDocument(); + expect(screen.getByText('Marketing')).toBeInTheDocument(); + // …and never auto-activated: the first VISIBLE area's navigation renders. + expect(screen.getByText('Cases')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); + + it('hides the switcher entirely when only one area remains visible', () => { + render(sidebarUi([gatedSales, serviceArea])); + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.queryByText('Service')).not.toBeInTheDocument(); + expect(screen.getByText('Cases')).toBeInTheDocument(); + }); + + it('renders no switcher and no area navigation when every area is fully gated', () => { + render( + sidebarUi([ + gatedSales, + { ...serviceArea, navigation: [{ ...serviceArea.navigation[0], visible: false }] }, + ]), + ); + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.queryByText('Service')).not.toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + expect(screen.queryByText('Cases')).not.toBeInTheDocument(); + }); + + it('counts an action item as area content — this sidebar wires a dispatcher (framework#4509)', () => { + const actionsArea: NavigationArea = { + id: 'area-actions', + label: 'Actions', + navigation: [ + { id: 'act1', type: 'action', label: 'Run Sync', actionDef: { actionName: 'sync' } } as never, + ], + }; + render(sidebarUi([actionsArea, serviceArea])); + // The action-only area is visible AND active (it is first): the switcher + // offers both areas. With no dispatcher this area would derive hidden. + expect(screen.getByText('Actions')).toBeInTheDocument(); + expect(screen.getByText('Service')).toBeInTheDocument(); + expect(screen.getByText('Run Sync')).toBeInTheDocument(); + }); + + it('re-elects when the ACTIVE area is gated away, and a mere reveal does not steal the selection', () => { + const adminSales: NavigationArea = { + ...salesArea, + navigation: [{ ...salesArea.navigation[0], requiredPermissions: ['sales:admin'] }], + }; + const areas = [adminSales, serviceArea, marketingArea]; + + const view = render(sidebarUi(areas)); + // Permission granted: Sales is visible and active. + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Opportunities')).toBeInTheDocument(); + + // Permission revoked (`object:action` form → can('sales','admin') false): + // Sales derives hidden, drops out of the switcher, and the sidebar + // re-elects the first visible area. + permissionsState = { + can: (objectName, action) => !(objectName === 'sales' && action === 'admin'), + hasCapabilities: () => true, + }; + view.rerender(sidebarUi(areas)); + expect(screen.queryByText('Sales')).not.toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + expect(screen.getByText('Cases')).toBeInTheDocument(); + + // Permission granted again: Sales REAPPEARS in the switcher, but the + // user's current selection is not yanked away — Service stays active. + permissionsState = { can: () => true, hasCapabilities: () => true }; + view.rerender(sidebarUi(areas)); + expect(screen.getByText('Sales')).toBeInTheDocument(); + expect(screen.getByText('Cases')).toBeInTheDocument(); + expect(screen.queryByText('Opportunities')).not.toBeInTheDocument(); + }); +}); From 428db06d910e58b1168a329c55df30388106b1b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 00:31:29 +0000 Subject: [PATCH 2/2] chore: changeset for #3319 sidebar derived area visibility Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa --- ...-shell-sidebars-derived-area-visibility.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .changeset/app-shell-sidebars-derived-area-visibility.md diff --git a/.changeset/app-shell-sidebars-derived-area-visibility.md b/.changeset/app-shell-sidebars-derived-area-visibility.md new file mode 100644 index 0000000000..97d8736adb --- /dev/null +++ b/.changeset/app-shell-sidebars-derived-area-visibility.md @@ -0,0 +1,30 @@ +--- +"@object-ui/app-shell": patch +--- + +`AppSidebar` and `UnifiedSidebar` area switchers now adopt the derived area +visibility introduced for `AppSchemaRenderer` in objectui#3311, closing the +same visible-but-empty gap in the console shells (objectui#3319). + +Both sidebars inlined their own area switcher without any area-level +filtering, so an area whose navigation items were **all** gated away +(`visible` expression, `requiredPermissions`, `requiresObject` / +`requiresService` capability gates) still appeared in the switcher and +rendered an empty navigation — and a fully gated *first* area was even +auto-activated, landing the user on an empty sidebar. + +## What changed + +- **Shared predicate, not a second implementation.** Both switchers now call + `hasVisibleNavigationItems` from `@object-ui/layout` — the exact guards + `NavigationRenderer` applies per item — so the switcher can never disagree + with the rendered navigation. In `UnifiedSidebar`, `action` items count as + content (it wires `onAction`, framework#4509); in `AppSidebar` they do not + (it wires none). +- **The active area is elected among visible areas only**: first visible by + default, re-elected when the active area is gated away, and a gating change + that merely *reveals* an area never steals the user's current selection. +- `areas: any[]` tightened to `NavigationArea[]` in both components. + +No authorable area-level key is introduced — visibility stays derived, per +the objectui#3311 ruling.