Skip to content

Commit 5126dc6

Browse files
authored
fix(tabs): preserve readable labels in crowded tab strips (#8319)
* fix(tabs): preserve readable labels in crowded tab strips * fix(tabs): type layout checks and verify touch selection stability * fix(tabs): keep touch sizing stable across activity changes
1 parent 240067b commit 5126dc6

3 files changed

Lines changed: 182 additions & 72 deletions

File tree

‎apps/desktop/e2e/browser-chrome.spec.ts‎

Lines changed: 133 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ mountBrowserChromeFixture(useBrowserPanelOcclusion);`,
5555
response.end(
5656
path.endsWith('.js')
5757
? bundle.outputFiles.find((file) => file.path.endsWith('.js'))?.text
58-
: css.css
58+
: `${css.css}\n${bundle.outputFiles.find((file) => file.path.endsWith('.css'))?.text ?? ''}`
5959
)
6060
return
6161
}
@@ -75,7 +75,7 @@ mountBrowserChromeFixture(useBrowserPanelOcclusion);`,
7575
response.end(
7676
path === '/page'
7777
? '<!doctype html><html><body style="background:#192b40;color:white;font:24px system-ui;padding:25px"><h1>Browser fixture</h1><p>A live page behind the application chrome.</p><button>Page action</button></body></html>'
78-
: '<!doctype html><html class="dark"><head><link rel="stylesheet" href="/fixture.css"></head><body style="margin:0;background:#191919;color:#eee"><div id="root"></div><script src="/fixture.js"></script></body></html>'
78+
: '<!doctype html><html class="dark"><head><link rel="stylesheet" href="/fixture.css"></head><body style="margin:0;background:var(--bg);color:var(--text-primary)"><div id="root"></div><script src="/fixture.js"></script></body></html>'
7979
)
8080
})
8181
await new Promise<void>((resolve) => server?.listen(0, resolve))
@@ -116,31 +116,150 @@ mountBrowserChromeFixture(useBrowserPanelOcclusion);`,
116116
return { width: bounds.width, right: bounds.right }
117117
})
118118
)
119-
expect(geometry.every((tab) => tab.width >= 64 && tab.width < 160)).toBe(true)
119+
expect(geometry.every((tab) => tab.width < 160)).toBe(true)
120120
expect(geometry.at(-1)?.right).toBeLessThan(1070)
121121
await page.screenshot({ path: testInfo.outputPath('tabs.png') })
122122
})
123-
await test.step('Short labels keep their compact intrinsic width', async () => {
124-
await page.locator('#short-tabs').click()
125-
const widths = await page
126-
.locator('[data-tab-strip-item]')
127-
.evaluateAll((tabs) => tabs.map((tab) => tab.getBoundingClientRect().width))
128-
expect(widths.every((width) => width >= 64 && width < 96)).toBe(true)
129-
await page.locator('#eight-tabs').click()
130-
})
131-
await test.step('Crowded tabs preserve controls and scroll', async () => {
123+
await test.step('Crowded tabs keep readable labels when selected, hovered, and focused', async () => {
132124
await page.locator('#many-tabs').click()
133125
await expect(page.locator('[data-tab-strip-item]')).toHaveCount(18)
134126
const overflow = await page
135127
.locator('[data-tab-strip-item]')
136128
.first()
137129
.evaluate((tab) => ({
138-
width: tab.getBoundingClientRect().width,
139130
scrollWidth: tab.parentElement?.scrollWidth ?? 0,
140131
clientWidth: tab.parentElement?.clientWidth ?? 0,
141132
}))
142-
expect(overflow.width).toBeGreaterThanOrEqual(64)
143133
expect(overflow.scrollWidth).toBeGreaterThan(overflow.clientWidth)
134+
const active = page.getByRole('tab', { selected: true })
135+
const label = active.locator('[data-overflow-text]')
136+
await expect
137+
.poll(() => label.evaluate((element) => element.getBoundingClientRect().width))
138+
.toBeGreaterThanOrEqual(48)
139+
const neighbor = page.getByRole('tab').nth(2)
140+
const beforeHover = await neighbor.evaluate((element: HTMLElement) => ({
141+
left: element.offsetLeft,
142+
width: element.offsetWidth,
143+
}))
144+
await neighbor.hover()
145+
await expect
146+
.poll(() =>
147+
neighbor
148+
.locator('[data-overflow-text]')
149+
.evaluate((element) => element.getBoundingClientRect().width)
150+
)
151+
.toBeGreaterThanOrEqual(48)
152+
expect(
153+
await neighbor.evaluate((element: HTMLElement) => ({
154+
left: element.offsetLeft,
155+
width: element.offsetWidth,
156+
}))
157+
).toEqual(beforeHover)
158+
await neighbor.click()
159+
await expect(neighbor).toHaveAttribute('aria-selected', 'true')
160+
await page.keyboard.press('End')
161+
const last = page.getByRole('tab').last()
162+
await expect(last).toBeFocused()
163+
await expect(last).toHaveAttribute('aria-selected', 'true')
164+
await expect(last).toBeInViewport({ ratio: 1 })
165+
await expect
166+
.poll(() => label.evaluate((element) => element.getBoundingClientRect().width))
167+
.toBeGreaterThanOrEqual(48)
168+
await page.mouse.move(200, 180)
169+
await page.screenshot({
170+
path: testInfo.outputPath('crowded-tabs.png'),
171+
animations: 'disabled',
172+
})
173+
await page.evaluate(() => document.documentElement.classList.remove('dark'))
174+
await page.screenshot({
175+
path: testInfo.outputPath('crowded-tabs-light.png'),
176+
animations: 'disabled',
177+
})
178+
await page.evaluate(() => document.documentElement.classList.add('dark'))
179+
await page.keyboard.press('Home')
180+
await expect(page.getByRole('tab').first()).toBeInViewport({ ratio: 1 })
181+
await page.locator('#eight-tabs').click()
182+
})
183+
await test.step('Touch tabs leave room for both attention and close controls', async () => {
184+
const session = await page.context().newCDPSession(page)
185+
try {
186+
await session.send('Emulation.setTouchEmulationEnabled', { enabled: true })
187+
expect(await page.evaluate(() => matchMedia('(any-pointer: coarse)').matches)).toBe(true)
188+
await page.locator('#many-tabs').click()
189+
const attention = page.getByRole('tab').nth(1)
190+
await expect
191+
.poll(() =>
192+
attention
193+
.locator('[data-overflow-text]')
194+
.evaluate((element) => element.getBoundingClientRect().width)
195+
)
196+
.toBeGreaterThanOrEqual(48)
197+
const attentionItem = page.locator('[data-tab-strip-item="tab-1"]')
198+
const indicator = attentionItem.locator('[data-row-action-indicator]')
199+
const controls = attentionItem.locator('[data-row-action-controls]')
200+
await expect(indicator).toBeVisible()
201+
await expect(indicator).toHaveCSS('opacity', '1')
202+
await expect(controls).toHaveCSS('opacity', '1')
203+
await attentionItem.getByRole('button', { name: /^Close / }).click({ trial: true })
204+
expect(
205+
await attentionItem.evaluate((element) => {
206+
const title = element.querySelector('[data-overflow-text]')?.getBoundingClientRect()
207+
const indicator = element
208+
.querySelector('[data-row-action-indicator]')
209+
?.getBoundingClientRect()
210+
const close = element.querySelector('[aria-label^="Close "]')?.getBoundingClientRect()
211+
return (
212+
title &&
213+
indicator &&
214+
close &&
215+
title.right <= indicator.left &&
216+
indicator.right <= close.left &&
217+
close.right <= element.getBoundingClientRect().right
218+
)
219+
})
220+
).toBe(true)
221+
await page.screenshot({ path: testInfo.outputPath('crowded-tabs-touch.png') })
222+
const beforeSelection = await attention.evaluate(
223+
(element: HTMLElement) => element.offsetWidth
224+
)
225+
await attention.click()
226+
await expect(attention).toHaveAttribute('aria-selected', 'true')
227+
expect(await attention.evaluate((element: HTMLElement) => element.offsetWidth)).toBe(
228+
beforeSelection
229+
)
230+
await page.locator('#toggle-activity').click()
231+
expect(await attention.evaluate((element: HTMLElement) => element.offsetWidth)).toBe(
232+
beforeSelection
233+
)
234+
await page.locator('#toggle-activity').click()
235+
await page.locator('#medium-tabs').click()
236+
await page.getByRole('tab').first().click()
237+
const intrinsicWidth = await attention.evaluate(
238+
(element: HTMLElement) => element.offsetWidth
239+
)
240+
expect(intrinsicWidth).toBeGreaterThan(144)
241+
expect(intrinsicWidth).toBeLessThan(200)
242+
await attention.click()
243+
expect(await attention.evaluate((element: HTMLElement) => element.offsetWidth)).toBe(
244+
intrinsicWidth
245+
)
246+
await page.locator('#toggle-activity').click()
247+
expect(await attention.evaluate((element: HTMLElement) => element.offsetWidth)).toBe(
248+
intrinsicWidth
249+
)
250+
await page.locator('#toggle-activity').click()
251+
} finally {
252+
await session.send('Emulation.setTouchEmulationEnabled', { enabled: false })
253+
await session.detach()
254+
}
255+
await page.locator('#eight-tabs').click()
256+
})
257+
await test.step('Short labels stay below the maximum tab width', async () => {
258+
await page.locator('#short-tabs').click()
259+
const widths = await page
260+
.locator('[data-tab-strip-item]')
261+
.evaluateAll((tabs) => tabs.map((tab) => tab.getBoundingClientRect().width))
262+
expect(widths.every((width) => width < 120)).toBe(true)
144263
await page.locator('#eight-tabs').click()
145264
})
146265
await test.step('An open menu recovers when no native page was available for its initial capture', async () => {

‎apps/desktop/e2e/fixtures/browser-chrome.tsx‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ function BrowserChromeFixture({ useOcclusion }: BrowserChromeFixtureProps) {
4646
const [activeTabId, setActiveTabId] = useState<string | null>(null)
4747
const [selected, setSelected] = useState('tab-0')
4848
const [tabCount, setTabCount] = useState(8)
49-
const [shortTitles, setShortTitles] = useState(false)
49+
const [titleLength, setTitleLength] = useState<'short' | 'medium' | 'long'>('long')
50+
const [attention, setAttention] = useState(true)
5051
const [error, setError] = useState<string | null>(null)
5152
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
5253
const { snapshot, snapshotLayer, onSnapshotError } = useOcclusion(
@@ -93,25 +94,43 @@ function BrowserChromeFixture({ useOcclusion }: BrowserChromeFixtureProps) {
9394
id='eight-tabs'
9495
onClick={() => {
9596
setTabCount(8)
96-
setShortTitles(false)
97+
setTitleLength('long')
9798
}}
9899
>
99100
Eight tabs
100101
</Button>
101102
<Button id='many-tabs' onClick={() => setTabCount(18)}>
102103
Many tabs
103104
</Button>
104-
<Button id='short-tabs' onClick={() => setShortTitles(true)}>
105+
<Button id='short-tabs' onClick={() => setTitleLength('short')}>
105106
Short titles
106107
</Button>
108+
<Button
109+
id='medium-tabs'
110+
onClick={() => {
111+
setTitleLength('medium')
112+
setTabCount(4)
113+
}}
114+
>
115+
Medium titles
116+
</Button>
117+
<Button id='toggle-activity' onClick={() => setAttention((value) => !value)}>
118+
Toggle activity
119+
</Button>
107120
</div>
108121
{error && <p role='alert'>{error}</p>}
109122
<TabStrip
110123
tabs={Array.from({ length: tabCount }, (_, index) => ({
111124
id: `tab-${index}`,
112-
title: shortTitles ? 'A' : `Example resource ${index + 1} with a descriptive title`,
125+
title:
126+
titleLength === 'short'
127+
? 'A'
128+
: titleLength === 'medium'
129+
? 'Medium title'
130+
: `Example resource ${index + 1} with a descriptive title`,
113131
icon: <File className='size-[16px] shrink-0' />,
114132
active: selected === `tab-${index}`,
133+
attention: attention && index === 1,
115134
}))}
116135
variant='floating'
117136
onSelect={setSelected}

‎packages/emcn/src/components/tab-strip/tab-strip.tsx‎

Lines changed: 26 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,14 @@ import {
1414
useRef,
1515
useState,
1616
} from 'react'
17-
import { OverflowText, RowActions, rowActionsGroupClass } from '@sim/emcn'
17+
import {
18+
OverflowText,
19+
RowActions,
20+
rowActionsGroupClass,
21+
SCROLL_FADE_BAND_PX,
22+
scrollFadeAttributes,
23+
scrollFadeXClass,
24+
} from '@sim/emcn'
1825
import { AnimatePresence, motion, useReducedMotion } from 'framer-motion'
1926
import { Plus, X } from '../../icons'
2027
import { cn } from '../../lib/cn'
@@ -24,36 +31,6 @@ import { TabStripAction } from './tab-strip-action'
2431

2532
const DRAG_EDGE_ZONE = 40
2633
const DRAG_SCROLL_SPEED = 8
27-
/**
28-
* Width of the scroll-edge fades, and so the margin a tab has to clear to be
29-
* genuinely visible. Keep in step with the `w-4` on the gradients below: a tab
30-
* revealed flush against the container edge lands under its gradient and reads
31-
* as half-faded, which is indistinguishable from "there is more to scroll".
32-
*/
33-
const EDGE_FADE_PX = 24
34-
35-
/**
36-
* Edge fades, as a mask rather than a tinted gradient laid over the tabs.
37-
*
38-
* Tabs paint their own fills, and an overlay tinted with the surface colour
39-
* washes a pill's edge toward that colour instead of dissolving it — and it is
40-
* only correct while whatever sits behind the strip is exactly that colour. A
41-
* mask fades pill and label together to real transparency, over any background.
42-
* This is how the command palette fades its results, and how every other
43-
* horizontal fade in the app is drawn.
44-
*
45-
* The four combinations are spelled out because Tailwind scans for literal class
46-
* strings; a template built at runtime would never be generated. Keep the 24px
47-
* stops in step with {@link EDGE_FADE_PX}, which is how far `revealActiveTab`
48-
* insets a tab so it lands clear of the fade rather than under it.
49-
*/
50-
const SCROLL_FADE = {
51-
none: '',
52-
start:
53-
'[-webkit-mask-image:linear-gradient(to_right,transparent_0px,black_24px)] [mask-image:linear-gradient(to_right,transparent_0px,black_24px)]',
54-
end: '[-webkit-mask-image:linear-gradient(to_right,black_calc(100%_-_24px),transparent_100%)] [mask-image:linear-gradient(to_right,black_calc(100%_-_24px),transparent_100%)]',
55-
both: '[-webkit-mask-image:linear-gradient(to_right,transparent_0px,black_24px,black_calc(100%_-_24px),transparent_100%)] [mask-image:linear-gradient(to_right,transparent_0px,black_24px,black_calc(100%_-_24px),transparent_100%)]',
56-
} as const
5734
const TAB_TRANSITION = { duration: 0.1, ease: [0.2, 0, 0, 1] as const }
5835

5936
/**
@@ -62,13 +39,16 @@ const TAB_TRANSITION = { duration: 0.1, ease: [0.2, 0, 0, 1] as const }
6239
* the basis and left every tab sized by its own title.
6340
*
6441
* Floating tabs start at their content width, capped at 200px, then shrink with
65-
* the available space. Floating tabs stop at a 64px control footprint; attached
66-
* tabs retain their 96px label minimum. Crowded rows then scroll, and clipped
67-
* titles remain available through tooltips.
42+
* the available space. Their 112px minimum leaves 50px for the title beside a
43+
* 16px icon and visible close button, including OverflowText's fade. Keep the
44+
* same minimum in every interaction state so revealing actions never shifts
45+
* tabs beneath the pointer. Touch layouts reserve both action slots even when
46+
* no activity indicator is present, so activity changes cannot resize tabs.
47+
* Crowded rows then scroll.
6848
*/
6949
const TAB_WIDTH: Record<TabStripVariant, string> = {
7050
attached: 'w-[156px] min-w-[96px] shrink',
71-
floating: 'min-w-[64px] max-w-[var(--tab-strip-max-tab-width,200px)] shrink',
51+
floating: 'min-w-28 max-w-[var(--tab-strip-max-tab-width,200px)] shrink',
7252
}
7353

7454
/** The resting shape of a tab that is not the active one. */
@@ -378,8 +358,7 @@ const Tab = forwardRef<HTMLDivElement, TabProps>(function Tab(
378358
tab.pinned ? 'justify-center px-0' : 'justify-start gap-1.5 px-2',
379359
closeable && 'pr-8',
380360
closeable &&
381-
tab.attention &&
382-
!tab.active &&
361+
(variant === 'floating' || (tab.attention && !tab.active)) &&
383362
'[@media(any-pointer:coarse)]:pr-[62px] [@media(hover:none)]:pr-[62px]',
384363
TAB_SHAPE[variant],
385364
tab.selected && !tab.active && TAB_SELECTED[variant],
@@ -427,9 +406,10 @@ const Tab = forwardRef<HTMLDivElement, TabProps>(function Tab(
427406
className={cn(
428407
'group relative select-none',
429408
rowActionsGroupClass,
430-
// `shrink` lets a crowded strip squeeze tabs to their floor before it
431-
// starts scrolling.
432409
tab.pinned ? 'w-[34px] min-w-[34px] max-w-[34px] flex-none' : TAB_WIDTH[variant],
410+
variant === 'floating' &&
411+
closeable &&
412+
'[@media(any-pointer:coarse)]:min-w-36 [@media(hover:none)]:min-w-36',
433413
dragging && 'opacity-30'
434414
)}
435415
data-tab-strip-item={tab.id}
@@ -584,16 +564,15 @@ export function TabStrip({
584564
const nodeRect = node.getBoundingClientRect()
585565
const tabLeft = tabRect.left - nodeRect.left + node.scrollLeft
586566
const tabRight = tabLeft + tabRect.width
587-
// Inset by the fade on both sides so the tab comes to rest clear of the
588-
// gradient rather than beneath it.
589-
const viewLeft = node.scrollLeft + EDGE_FADE_PX
590-
const viewRight = node.scrollLeft + node.clientWidth - EDGE_FADE_PX
567+
/** Keep the active tab clear of the canonical scroll fade. */
568+
const viewLeft = node.scrollLeft + SCROLL_FADE_BAND_PX
569+
const viewRight = node.scrollLeft + node.clientWidth - SCROLL_FADE_BAND_PX
591570
const maxScrollLeft = Math.max(0, node.scrollWidth - node.clientWidth)
592571
const target =
593572
tabLeft < viewLeft
594-
? tabLeft - EDGE_FADE_PX
573+
? tabLeft - SCROLL_FADE_BAND_PX
595574
: tabRight > viewRight
596-
? tabRight - node.clientWidth + EDGE_FADE_PX
575+
? tabRight - node.clientWidth + SCROLL_FADE_BAND_PX
597576
: null
598577
if (target === null) return
599578
// The clamp is what lets the first and last tabs sit flush: there is no
@@ -926,18 +905,11 @@ export function TabStrip({
926905
<div className='flex min-w-0 shrink'>
927906
<div
928907
ref={scrollNodeRef}
908+
{...scrollFadeAttributes({ left: canScrollLeft, right: canScrollRight })}
929909
className={cn(
930910
'flex min-w-0 shrink select-none gap-0.5 overflow-x-auto [scrollbar-width:none] [&::-webkit-scrollbar]:hidden',
931911
variant === 'attached' ? 'items-end' : 'items-center gap-2',
932-
SCROLL_FADE[
933-
canScrollLeft
934-
? canScrollRight
935-
? 'both'
936-
: 'start'
937-
: canScrollRight
938-
? 'end'
939-
: 'none'
940-
]
912+
scrollFadeXClass
941913
)}
942914
>
943915
<AnimatePresence initial={false} mode='popLayout'>

0 commit comments

Comments
 (0)