Skip to content

Commit 8ea2d4d

Browse files
waleedlatif1waleedlatif1
committed
fix(tabs): preserve readable labels in crowded tab strips
1 parent 240067b commit 8ea2d4d

3 files changed

Lines changed: 105 additions & 66 deletions

File tree

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

Lines changed: 80 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,97 @@ 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) => ({
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) => ({
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+
await page.screenshot({ path: testInfo.outputPath('crowded-tabs-touch.png') })
198+
} finally {
199+
await session.send('Emulation.setTouchEmulationEnabled', { enabled: false })
200+
await session.detach()
201+
}
202+
await page.locator('#eight-tabs').click()
203+
})
204+
await test.step('Short labels stay below the maximum tab width', async () => {
205+
await page.locator('#short-tabs').click()
206+
const widths = await page
207+
.locator('[data-tab-strip-item]')
208+
.evaluateAll((tabs) => tabs.map((tab) => tab.getBoundingClientRect().width))
209+
expect(widths.every((width) => width < 120)).toBe(true)
144210
await page.locator('#eight-tabs').click()
145211
})
146212
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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ function BrowserChromeFixture({ useOcclusion }: BrowserChromeFixtureProps) {
112112
title: shortTitles ? 'A' : `Example resource ${index + 1} with a descriptive title`,
113113
icon: <File className='size-[16px] shrink-0' />,
114114
active: selected === `tab-${index}`,
115+
attention: index === 1,
115116
}))}
116117
variant='floating'
117118
onSelect={setSelected}

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

Lines changed: 24 additions & 52 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,14 @@ 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. Crowded rows then scroll.
6846
*/
6947
const TAB_WIDTH: Record<TabStripVariant, string> = {
7048
attached: 'w-[156px] min-w-[96px] shrink',
71-
floating: 'min-w-[64px] max-w-[var(--tab-strip-max-tab-width,200px)] shrink',
49+
floating: 'min-w-28 max-w-[var(--tab-strip-max-tab-width,200px)] shrink',
7250
}
7351

7452
/** The resting shape of a tab that is not the active one. */
@@ -427,9 +405,11 @@ const Tab = forwardRef<HTMLDivElement, TabProps>(function Tab(
427405
className={cn(
428406
'group relative select-none',
429407
rowActionsGroupClass,
430-
// `shrink` lets a crowded strip squeeze tabs to their floor before it
431-
// starts scrolling.
432408
tab.pinned ? 'w-[34px] min-w-[34px] max-w-[34px] flex-none' : TAB_WIDTH[variant],
409+
variant === 'floating' &&
410+
closeable &&
411+
tab.attention &&
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)