revert(nav): keep company switching in the bottom user block only (#1775)
Reverts #1765, which mounted the CompanySwitcher at the top of the
expanded desktop sidebar. Seen live, the top slot is the wrong home for
it: the sidebar head stays brand + collapse control, and the nav starts
directly below it. Switching keeps its single home in the bottom user
block (UserMenu flyout), which is also what the collapsed 64px rail and
every existing muscle memory already use. The mobile sheet's switcher is
untouched.
The logo title tooltip and the source-shape regression test go back with
it: both shipped inside the same commit and both exist only to pin the
top placement.
DECISIONS.md records that #1664's "one-click from the top" framing is
declined rather than merely unimplemented, so the issue does not get
re-opened into the same PR.
Reverts 72181e49bd
Claude-Session: https://claude.ai/code/session_01LcwpT82qwHhtTAFeU47Ay1
Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Opus 5
parent
577552ca94
commit
c5b7716f74
@@ -706,13 +706,9 @@ export default function DashboardNav({ companyName: _companyName, entityType, pa
|
||||
: 'justify-between pl-5 pr-3',
|
||||
)}
|
||||
>
|
||||
{/* Native title tooltip (same convention as the collapsed rail
|
||||
items) so the logo square is labeled on hover, not a mystery
|
||||
mark (#1664). */}
|
||||
<Link
|
||||
href="/"
|
||||
aria-label={getBranding().appName}
|
||||
title={getBranding().appName}
|
||||
className="flex items-center rounded-lg"
|
||||
>
|
||||
<Image
|
||||
@@ -737,19 +733,6 @@ export default function DashboardNav({ companyName: _companyName, entityType, pa
|
||||
</button>
|
||||
</div>
|
||||
|
||||
{/* One-click company switch pinned at the top of the sidebar
|
||||
(#1664): consultants hop between companies constantly, and the
|
||||
user-menu flyout costs three clicks. That flyout stays as the
|
||||
secondary path. Hidden on the collapsed rail (no room at 64px);
|
||||
the UserMenu avatar still reaches the same switch there. Kept
|
||||
outside the data-ph-unmask navs so the company name stays
|
||||
masked in replays, like the mobile sheet's switcher. */}
|
||||
{!collapsed && (
|
||||
<div className="flex-shrink-0 px-4 pb-1">
|
||||
<CompanySwitcher />
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Nav items in their own scroll container so the user block
|
||||
below stays sticky (concept PR 2). */}
|
||||
<div className="relative flex-1 min-h-0 overflow-y-auto overflow-x-hidden pt-1 pb-2">
|
||||
|
||||
@@ -1,69 +0,0 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import fs from 'node:fs'
|
||||
import path from 'node:path'
|
||||
|
||||
const NAV_SRC = fs.readFileSync(
|
||||
path.resolve(__dirname, '../DashboardNav.tsx'),
|
||||
'utf8',
|
||||
)
|
||||
const USER_MENU_SRC = fs.readFileSync(
|
||||
path.resolve(__dirname, '../UserMenu.tsx'),
|
||||
'utf8',
|
||||
)
|
||||
|
||||
/**
|
||||
* Regression pin for one-click company switching in the sidebar (#1664).
|
||||
*
|
||||
* The switch was folded into the bottom-of-sidebar user popover as a nested
|
||||
* flyout (avatar, then "Byt foretag", then the company): three clicks per
|
||||
* switch, which consultants who hop between companies do constantly. The
|
||||
* one-click CompanySwitcher must stay mounted at the top of the desktop
|
||||
* sidebar, with the user-menu flyout as the secondary path. The repo does
|
||||
* not render components in tests, so pin the source shape instead, the same
|
||||
* way JournalEntryList's copy affordance is pinned.
|
||||
*/
|
||||
describe('DashboardNav sidebar company switcher (#1664)', () => {
|
||||
const asideStart = NAV_SRC.indexOf('<aside')
|
||||
const mobileNavStart = NAV_SRC.indexOf('Mobile bottom navigation')
|
||||
// Unique to the desktop nav scroll container; the mobile sheet uses
|
||||
// overscroll-contain instead.
|
||||
const desktopScrollStart = NAV_SRC.indexOf('overflow-y-auto overflow-x-hidden')
|
||||
|
||||
it('has the anchors this pin relies on', () => {
|
||||
expect(asideStart).toBeGreaterThan(-1)
|
||||
expect(mobileNavStart).toBeGreaterThan(asideStart)
|
||||
expect(desktopScrollStart).toBeGreaterThan(asideStart)
|
||||
expect(desktopScrollStart).toBeLessThan(mobileNavStart)
|
||||
})
|
||||
|
||||
it('renders CompanySwitcher inside the desktop sidebar, not only the mobile sheet', () => {
|
||||
const desktopSwitcher = NAV_SRC.indexOf('<CompanySwitcher />', asideStart)
|
||||
expect(desktopSwitcher).toBeGreaterThan(asideStart)
|
||||
expect(desktopSwitcher).toBeLessThan(mobileNavStart)
|
||||
})
|
||||
|
||||
it('pins the desktop switcher to the top of the sidebar, above the nav scroll container', () => {
|
||||
const desktopSwitcher = NAV_SRC.indexOf('<CompanySwitcher />', asideStart)
|
||||
expect(desktopSwitcher).toBeLessThan(desktopScrollStart)
|
||||
})
|
||||
|
||||
it('keeps the mobile sheet switcher as well', () => {
|
||||
const desktopSwitcher = NAV_SRC.indexOf('<CompanySwitcher />', asideStart)
|
||||
const mobileSwitcher = NAV_SRC.indexOf('<CompanySwitcher />', mobileNavStart)
|
||||
expect(mobileSwitcher).toBeGreaterThan(desktopSwitcher)
|
||||
})
|
||||
|
||||
it('labels the brand logo link with a visible title tooltip and aria-label', () => {
|
||||
const logoLink = NAV_SRC.slice(
|
||||
NAV_SRC.indexOf('<Link', asideStart),
|
||||
NAV_SRC.indexOf('</Link>', asideStart),
|
||||
)
|
||||
expect(logoLink).toContain('aria-label={getBranding().appName}')
|
||||
expect(logoLink).toContain('title={getBranding().appName}')
|
||||
})
|
||||
|
||||
it('keeps the user-menu company flyout as the secondary switch path', () => {
|
||||
expect(USER_MENU_SRC).toContain('performCompanySwitch')
|
||||
expect(USER_MENU_SRC).toContain('setCompaniesOpen')
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user