diff --git a/packages/components/button-toggle/__screenshots__/02-dark.png b/packages/components/button-toggle/__screenshots__/02-dark.png index 5e72f60e3..78af39242 100644 Binary files a/packages/components/button-toggle/__screenshots__/02-dark.png and b/packages/components/button-toggle/__screenshots__/02-dark.png differ diff --git a/packages/components/button-toggle/__screenshots__/03-dark.png b/packages/components/button-toggle/__screenshots__/03-dark.png index 41f5a1e24..fa5abb068 100644 Binary files a/packages/components/button-toggle/__screenshots__/03-dark.png and b/packages/components/button-toggle/__screenshots__/03-dark.png differ diff --git a/packages/components/button-toggle/__screenshots__/04-light.png b/packages/components/button-toggle/__screenshots__/04-light.png index c3b69614e..b3b84f296 100644 Binary files a/packages/components/button-toggle/__screenshots__/04-light.png and b/packages/components/button-toggle/__screenshots__/04-light.png differ diff --git a/packages/components/button-toggle/button-toggle-tokens.scss b/packages/components/button-toggle/button-toggle-tokens.scss index d6a190ee5..12b472882 100644 --- a/packages/components/button-toggle/button-toggle-tokens.scss +++ b/packages/components/button-toggle/button-toggle-tokens.scss @@ -11,6 +11,20 @@ --kbq-button-toggle-size-item-padding-vertical: var(--kbq-size-3xs); --kbq-button-toggle-size-item-content-gap-horizontal: var(--kbq-size-xxs); --kbq-button-toggle-size-item-focus-outline-width: var(--kbq-size-3xs); + // How far an item's hit area reaches past the pill it paints. Measured from the inner button's + // border box, one item border inside the item's own — hence the border-width term in each. `-gap` is + // the half of the inter-item gap an item claims; the other two cover the group's padding. The + // pseudo-element they offset is in button-toggle.scss, next to the note on the two other places + // this repo extends a hit area the same way. + --kbq-button-toggle-size-item-hit-area-gap: calc( + var(--kbq-size-border-width) + var(--kbq-button-toggle-size-container-content-gap-horizontal) / 2 + ); + --kbq-button-toggle-size-item-hit-area-horizontal: calc( + var(--kbq-size-border-width) + var(--kbq-button-toggle-size-container-padding-horizontal) + ); + --kbq-button-toggle-size-item-hit-area-vertical: calc( + var(--kbq-size-border-width) + var(--kbq-button-toggle-size-container-padding-vertical) + ); /* THEME TOKENS */ --kbq-button-toggle-container-background: var(--kbq-background-contrast-fade); --kbq-button-toggle-item-default-background: var(--kbq-background-transparent); diff --git a/packages/components/button-toggle/button-toggle.scss b/packages/components/button-toggle/button-toggle.scss index 3cbb092b5..872cd7d6d 100644 --- a/packages/components/button-toggle/button-toggle.scss +++ b/packages/components/button-toggle/button-toggle.scss @@ -19,17 +19,31 @@ .kbq-button-toggle { display: flex; - overflow: hidden; justify-content: flex-start; align-items: center; + // Must not clip: the button's hit area below reaches outside this box. Not `overflow: clip` with + // an `overflow-clip-margin` — Chromium rejects math functions there, so the margin cannot follow + // the tokens the offsets come from and computes to `0px`, clipping the hit area away. + + // What the item stops containing, and why nothing here takes it back: a toggle projects its + // content into the button's own `.kbq-button-text`, which keeps the `overflow: hidden` of + // `kbq-truncate-line()`, so content still cannot spill sideways past the button's content box. + // Content *taller* than the fixed item height no longer is — and cannot be re-clipped from + // inside the button, since the only boxes there with a definite height are the button itself, + // whose `::before` this exists for, and the label box, which would clip its own descenders. + overflow: visible; + + // What that clip used to give for free: a flex item's automatic minimum size is zero only while + // its `overflow` is non-`visible`, and that is what lets the label truncate. + min-width: 0; + // Content-box on purpose, and load-bearing. `--kbq-button-toggle-size-item-height` is 22px, // not the 24px of the design's `--kbq-size-xxl`: DS-3136 subtracted the border it added here // from the token rather than from the rendered box, so the item only reaches its designed // 24px while the border sits outside the declared height. A global `border-box` reset in a // consuming application would pull the border back inside and leave the item 2px short — - // and the inner button, which is sized from the same token, would overflow the shrunken - // content box and be clipped by `overflow: hidden` above. + // and the inner button, which is sized from the same token, would then spill past it. box-sizing: content-box; height: var(--kbq-button-toggle-size-item-height); @@ -60,6 +74,21 @@ button-toggle-size-item-focus-outline-width] ); + // Hit area over the gutters the group's `gap` and `padding` leave around the pill this button + // paints, so the highlight does not drop out between segments. On the button, not the item + // host, so pointer events resolve to it and the gutter is clickable too. `::before` because + // `::after` is the `.kbq-progress` shimmer. Offsets are per orientation, below. + + // The third place this repo extends a hit area with a transparent pseudo-element, after + // `.kbq-navbar-item`'s `:before` (DS-2558) and `select.item-active-area()`. Neither is + // reusable here: the mixin claims a fixed strip on one side of a plain vertical list, with + // no gap to tile between two neighbours, no group padding at the ends, and no orientation + // to swap the axes for. + &:before { + content: ''; + position: absolute; + } + & .kbq-button-wrapper { width: 100%; } @@ -117,6 +146,16 @@ } } + // Hit-area offsets, one orientation each — the vertical block below is this one with the axes + // swapped. The gap is split at its midline so two neighbours tile it rather than overlap, which would + // hand the whole gutter to whichever paints last; the outermost sides have no neighbour to share with + // and take the group's padding instead, leaving no point inside the group unclaimed. + + // `:first-of-type`/`:last-of-type`, not `:first-child`/`:last-child`: the group is a bare directive + // with no template, so a consumer's own element — a divider, a leading icon — is rendered as a real + // child among the toggles and would take the positional match away from the toggle that actually sits + // at the edge, putting the dead zone back. The class only ever lands on a `kbq-button-toggle` element, + // so matching by type reads the toggles alone. &:not(.kbq-button-toggle_vertical) { .kbq-button-toggle { > .kbq-button, @@ -124,6 +163,39 @@ max-width: 100%; height: var(--kbq-button-toggle-size-item-height); + + &:before { + inset-block: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-vertical)); + inset-inline: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-gap)); + } + } + + &:first-of-type > :is(.kbq-button, .kbq-button-icon):before { + inset-inline-start: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-horizontal)); + } + + &:last-of-type > :is(.kbq-button, .kbq-button-icon):before { + inset-inline-end: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-horizontal)); + } + } + } + + &.kbq-button-toggle_vertical { + .kbq-button-toggle { + > .kbq-button, + > .kbq-button-icon { + &:before { + inset-inline: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-horizontal)); + inset-block: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-gap)); + } + } + + &:first-of-type > :is(.kbq-button, .kbq-button-icon):before { + inset-block-start: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-vertical)); + } + + &:last-of-type > :is(.kbq-button, .kbq-button-icon):before { + inset-block-end: calc(-1 * var(--kbq-button-toggle-size-item-hit-area-vertical)); } } } diff --git a/packages/components/button-toggle/e2e.playwright-spec.ts b/packages/components/button-toggle/e2e.playwright-spec.ts index 66d639f52..666975807 100644 --- a/packages/components/button-toggle/e2e.playwright-spec.ts +++ b/packages/components/button-toggle/e2e.playwright-spec.ts @@ -81,6 +81,43 @@ test.describe('KbqButtonToggleModule', () => { const getLabelWidths = (toggle: Locator) => getLabel(toggle).evaluate((element) => ({ scroll: element.scrollWidth, client: element.clientWidth })); + type Box = { x: number; y: number; width: number; height: number }; + + /** + * An item paints a pill smaller than the space the group gives it, and the `gap` and `padding` + * around it answer the pointer too — see the `::before` in button-toggle.scss. Nothing about the + * rendered pixels changes, so a baseline cannot cover any of this. The two helpers below are shared + * by the harnesses that exercise it, which size their pills differently. + */ + + /** Border boxes of the buttons that paint the pills, in document order. */ + const getPillBoxes = (group: Locator): Promise => + group.evaluate((element) => + Array.from(element.querySelectorAll(':scope > kbq-button-toggle > button')).map((button) => { + const { x, y, width, height } = button.getBoundingClientRect(); + + return { x, y, width, height }; + }) + ); + + /** + * Which item owns a viewport point, as the browser's own hit testing resolves it. The tag must + * come back `button` rather than the item host: the button is what carries the click handler. + */ + const getOwnerAt = (page: Page, x: number, y: number) => + page.evaluate( + ([pointX, pointY]) => { + const element = document.elementFromPoint(pointX, pointY); + const toggle = element?.closest('kbq-button-toggle'); + + return { + tag: element ? element.tagName.toLowerCase() : null, + index: toggle ? Array.from(toggle.parentElement!.children).indexOf(toggle) : -1 + }; + }, + [x, y] + ); + test.describe('E2eButtonToggleStates', () => { const getComponent = (page: Page) => page.getByTestId('e2eButtonToggleStates'); const togglePrefix = (locator: Locator) => locator.getByTestId('e2eShowPrefixIcon').click(); @@ -179,6 +216,189 @@ test.describe('KbqButtonToggleModule', () => { await expectIconsCentred(locator); }); + + test.describe('hit area', () => { + /** + * One column of one orientation, `role` telling the exclusive rows from the `multiple` ones + * and the first item's `aria-label` naming the state the column renders — `normal` is the + * only one whose items carry neither a faked state class nor `disabled`. `iconOnly` drops + * that first item's label, which is what makes `KbqButtonCssStyler` swap its class over to + * `.kbq-button-icon`; `default 2`/`default 3` keep their labels either way. Scrolled into + * view because `elementFromPoint` answers `null` outside the viewport, and this harness lays + * every state of every orientation out in one oversized table. + */ + const openGroup = async (page: Page, { vertical = false, state = 'normal', iconOnly = false } = {}) => { + await page.goto('/E2eButtonToggleStates'); + + const locator = getComponent(page); + + if (iconOnly) { + await togglePrefix(locator); + await toggleTitle(locator); + } + + const group = getScreenshotTarget(locator) + .locator( + `kbq-button-toggle-group[role="radiogroup"][aria-orientation="${ + vertical ? 'vertical' : 'horizontal' + }"]` + ) + .filter({ has: page.locator(`button[aria-label="${state}"]`) }); + + await group.scrollIntoViewIfNeeded(); + + return group; + }; + + /** `along` is the axis the items run on, `across` the other, so both orientations read alike. */ + const getAxes = (page: Page, vertical: boolean) => ({ + at: (along: number, across: number) => + vertical ? getOwnerAt(page, across, along) : getOwnerAt(page, along, across), + along: (box: Box) => (vertical ? [box.y, box.y + box.height] : [box.x, box.x + box.width]), + across: (box: Box) => (vertical ? [box.x, box.x + box.width] : [box.y, box.y + box.height]), + alongCentre: (box: Box) => (vertical ? box.y + box.height / 2 : box.x + box.width / 2), + acrossCentre: (box: Box) => (vertical ? box.x + box.width / 2 : box.y + box.height / 2), + alongEnd: (box: Box) => (vertical ? box.y + box.height : box.x + box.width) + }); + + for (const vertical of [false, true]) { + const orientation = vertical ? 'vertical' : 'horizontal'; + + test(`hands the gap between two items over at its midline, ${orientation}`, async ({ page }) => { + const axes = getAxes(page, vertical); + const [first, second] = await getPillBoxes(await openGroup(page, { vertical })); + const gapMid = (axes.alongEnd(first) + axes.along(second)[0]) / 2; + const across = axes.acrossCentre(first); + + // no dead strip in between, and no overlap either + expect(await axes.at(gapMid - 1, across)).toEqual({ tag: 'button', index: 0 }); + expect(await axes.at(gapMid + 1, across)).toEqual({ tag: 'button', index: 1 }); + }); + + test(`reaches into the padding the group frames its items with, ${orientation}`, async ({ page }) => { + const axes = getAxes(page, vertical); + const group = await openGroup(page, { vertical }); + const box = (await group.boundingBox())!; + const pills = await getPillBoxes(group); + const last = pills.length - 1; + const [alongStart, alongEnd] = axes.along(box); + const [acrossStart, acrossEnd] = axes.across(box); + + // the outermost items have no neighbour to share with, so they take the padding whole + expect(await axes.at(alongStart + 1, axes.acrossCentre(pills[0]))).toEqual({ + tag: 'button', + index: 0 + }); + expect(await axes.at(alongEnd - 1, axes.acrossCentre(pills[last]))).toEqual({ + tag: 'button', + index: last + }); + + // and on the cross axis every item does, on both sides — the offset comes from the + // rule none of the positional selectors touch, so a middle item is as much of a + // case as the two ends + for (let index = 0; index < pills.length; index++) { + const alongCentre = axes.alongCentre(pills[index]); + + expect(await axes.at(alongCentre, acrossStart + 1)).toEqual({ tag: 'button', index }); + expect(await axes.at(alongCentre, acrossEnd - 1)).toEqual({ tag: 'button', index }); + } + }); + } + + test('keeps a disabled item in charge of its own half of the gap', async ({ page }) => { + const group = await openGroup(page, { state: 'disabled' }); + const buttons = group.locator('kbq-button-toggle > button'); + const [first, second] = await getPillBoxes(group); + const gapMid = (first.x + first.width + second.x) / 2; + const across = first.y + first.height / 2; + + await expect(buttons.first()).toBeDisabled(); + + // the split is drawn the same either way: a disabled item's share of the gutter is + // still its own, rather than a wider catchment for the enabled neighbour + expect(await getOwnerAt(page, gapMid - 1, across)).toEqual({ tag: 'button', index: 0 }); + expect(await getOwnerAt(page, gapMid + 1, across)).toEqual({ tag: 'button', index: 1 }); + + await page.mouse.click(gapMid - 1, across); + + // and what lands on that share is swallowed, not handed to either neighbour + await expect(buttons.nth(0)).toHaveAttribute('aria-checked', 'false'); + await expect(buttons.nth(1)).toHaveAttribute('aria-checked', 'false'); + }); + + test('claims the same gutters for an icon-only item', async ({ page }) => { + const group = await openGroup(page, { iconOnly: true }); + const box = (await group.boundingBox())!; + const [first, second] = await getPillBoxes(group); + const across = first.y + first.height / 2; + + // An item with nothing but an icon in it, which lays its content out differently and + // is the narrowest pill a group can hand a gutter to. It stays a `.kbq-button`: the + // second class the hit-area selectors name, `.kbq-button-icon`, is unreachable inside + // a toggle, because the icon is projected through the toggle's own wrapper and so is + // never content of the button `KbqButtonCssStyler` counts. `.kbq-button_no-label` is + // what marks the case here. + await expect(group.locator('kbq-button-toggle').first().locator('button')).toHaveClass( + /kbq-button_no-label/ + ); + + expect(await getOwnerAt(page, box.x + 1, across)).toEqual({ tag: 'button', index: 0 }); + expect(await getOwnerAt(page, (first.x + first.width + second.x) / 2 - 1, across)).toEqual({ + tag: 'button', + index: 0 + }); + expect(await getOwnerAt(page, (first.x + first.width + second.x) / 2 + 1, across)).toEqual({ + tag: 'button', + index: 1 + }); + }); + + test('paints the hover state from a pointer in the gap', async ({ page }) => { + const group = await openGroup(page); + const second = group.locator('kbq-button-toggle').nth(1).locator('button'); + const [first, secondBox] = await getPillBoxes(group); + const getBackground = () => + second.evaluate((element: HTMLElement) => getComputedStyle(element).backgroundColor); + + const before = await getBackground(); + + // 1px past the midline, i.e. still outside the pill this must light up + await page.mouse.move((first.x + first.width + secondBox.x) / 2 + 1, first.y + first.height / 2); + + expect(await getBackground()).not.toBe(before); + }); + + test('activates a toggle from a click in the gap', async ({ page }) => { + const group = await openGroup(page); + const second = group.locator('kbq-button-toggle').nth(1).locator('button'); + const [first, secondBox] = await getPillBoxes(group); + + await expect(second).toHaveAttribute('aria-checked', 'false'); + + await page.mouse.click((first.x + first.width + secondBox.x) / 2 + 1, first.y + first.height / 2); + + await expect(second).toHaveAttribute('aria-checked', 'true'); + }); + + test('mirrors the outermost offsets under dir="rtl"', async ({ page }) => { + const group = await openGroup(page); + + await page.evaluate(() => document.documentElement.setAttribute('dir', 'rtl')); + + const box = (await group.boundingBox())!; + const pills = await getPillBoxes(group); + const across = box.y + box.height / 2; + + // the first item is laid out at the trailing edge now, so it claims the padding there + expect(pills[0].x).toBeGreaterThan(pills[pills.length - 1].x); + expect(await getOwnerAt(page, box.x + box.width - 1, across)).toEqual({ tag: 'button', index: 0 }); + expect(await getOwnerAt(page, box.x + 1, across)).toEqual({ + tag: 'button', + index: pills.length - 1 + }); + }); + }); }); test.describe('E2eButtonToggleStatesStretched', () => { @@ -200,6 +420,37 @@ test.describe('KbqButtonToggleModule', () => { await expect(getScreenshotTarget(locator)).toHaveScreenshot('04-dark.png'); }); + /** + * The offsets the hit area is built from are the group's own `gap` and `padding`, which this + * variant does not change — but it is the only one where the pills are sized by `flex: 1` + * rather than by their content, so it is the only one where a gutter could come out of a + * division rather than out of the tokens. + */ + test('hands the gap over at its midline and reaches into the padding', async ({ page }) => { + await page.goto('/E2eButtonToggleStatesStretched'); + + const group = getScreenshotTarget(getComponent(page)) + .locator('kbq-button-toggle-group') + .filter({ has: page.locator('button[aria-label="normal"]') }); + + await group.scrollIntoViewIfNeeded(); + + const box = (await group.boundingBox())!; + const pills = await getPillBoxes(group); + const [first, second] = pills; + const gapMid = (first.x + first.width + second.x) / 2; + const across = first.y + first.height / 2; + + expect(await getOwnerAt(page, gapMid - 1, across)).toEqual({ tag: 'button', index: 0 }); + expect(await getOwnerAt(page, gapMid + 1, across)).toEqual({ tag: 'button', index: 1 }); + + expect(await getOwnerAt(page, box.x + 1, across)).toEqual({ tag: 'button', index: 0 }); + expect(await getOwnerAt(page, box.x + box.width - 1, across)).toEqual({ + tag: 'button', + index: pills.length - 1 + }); + }); + test('centres icons vertically', async ({ page }) => { await page.goto('/E2eButtonToggleStatesStretched'); const locator = getComponent(page); diff --git a/packages/components/core/styles/common/_select.scss b/packages/components/core/styles/common/_select.scss index ca213a9eb..0c176c701 100644 --- a/packages/components/core/styles/common/_select.scss +++ b/packages/components/core/styles/common/_select.scss @@ -249,6 +249,14 @@ padding: var(--kbq-size-xxs) var(--kbq-size-l); } +// Extends an item's hit area past the box it paints, with a transparent pseudo-element on the element +// that owns the click handler — a pseudo-element is hit-tested as part of that element, so hover and +// click both reach the gutter with no TS and no `z-index`. This one claims a fixed strip either side of +// a plain vertical list. The same technique, sized differently, is hand-rolled where the geometry does +// not fit: `.kbq-navbar-item`'s `:before` cancels the item's own margin (DS-2558), and +// `--kbq-button-toggle-size-item-hit-area-*` tiles a group's `gap` between two neighbours and hands the +// group's padding to the outermost items, per orientation (DS-4164). Any clipping ancestor eats all of +// them — see the note on `.kbq-button-toggle`'s `overflow`. @mixin item-active-area($outline) { position: relative; diff --git a/packages/components/navbar/_navbar-item_horizontal.scss b/packages/components/navbar/_navbar-item_horizontal.scss index b3e8c1cb5..f748f33ba 100644 --- a/packages/components/navbar/_navbar-item_horizontal.scss +++ b/packages/components/navbar/_navbar-item_horizontal.scss @@ -19,6 +19,11 @@ justify-content: center; + // Hit area over the margin this item is spaced with, so the highlight does not drop out between + // items (DS-2558) — the transparent pseudo-element is hit-tested as part of the item, so hover and + // click both reach it. `:after` below paints the highlight, at the item's own box. The same + // technique is written twice more: `select.item-active-area()` for a plain vertical list, and + // `--kbq-button-toggle-size-item-hit-area-*` for a group whose `gap` two neighbours have to tile. &:before { top: calc(-1 * #{var(--kbq-size-s)}); left: calc(-1 * #{var(--kbq-size-s)});