Fix breadcrumb scroll button: one-crumb nudge and opaque hover (#62422)

Author: Ebonsignori

src/fixtures/tests/playwright-rendering.spec.ts

@@ -732,6 +732,81 @@ test.describe('test nav at different viewports', () => {
   })
 })
 
+test.describe('secondary-bar breadcrumb scroller', () => {
+  // The secondary bar (and its breadcrumb scroller) only renders at wide
+  // viewports, and the fixture trail is short enough to fit there, so we cap the
+  // scroller width to force a deterministic overflow independent of title
+  // lengths — then exercise the chevrons.
+  test('chevrons scroll one crumb at a time instead of jumping to the ends', async ({ page }) => {
+    // Smooth-scroll settle waits across several chevron clicks add up past the
+    // default 5s cap.
+    test.setTimeout(20000)
+    page.setViewportSize({ width: 1300, height: 700 })
+    await page.goto('/get-started/foo/bar')
+
+    const bar = page.getByTestId('breadcrumbs-bar')
+    await expect(bar).toBeVisible()
+
+    const scrollArea = page.locator('[data-search="breadcrumbs"]')
+    await expect(scrollArea).toBeVisible()
+
+    // Force a deterministic overflow independent of title lengths: cap the
+    // scroll region, drop the nav's min-width:100% (which otherwise stretches the
+    // short fixture trail to fill the container so it never overflows), and pad
+    // the crumbs so several are hidden at once — enough that a per-crumb nudge is
+    // distinguishable from a jump to the end.
+    await page.addStyleTag({
+      content: `
+        [data-search="breadcrumbs"] { max-width: 360px; }
+        [data-search="breadcrumbs"] nav { min-width: 0 !important; }
+        [data-search="breadcrumbs"] li { padding-right: 60px; }
+      `,
+    })
+
+    const scrollLeftOf = () => scrollArea.evaluate((el) => el.scrollLeft)
+    const maxScrollOf = () => scrollArea.evaluate((el) => el.scrollWidth - el.clientWidth)
+    await expect.poll(maxScrollOf).toBeGreaterThan(0)
+
+    // Anchor to the right end explicitly so we start from a known state: fully
+    // scrolled right (current page visible), only the left chevron active.
+    await scrollArea.evaluate((el) => el.scrollTo({ left: el.scrollWidth, behavior: 'instant' }))
+    const maxScroll = await maxScrollOf()
+    await expect.poll(scrollLeftOf).toBe(maxScroll)
+
+    const leftChevron = page.getByRole('button', { name: 'Scroll breadcrumbs left' })
+    const rightChevron = page.getByRole('button', { name: 'Scroll breadcrumbs right' })
+    // At the right extreme the left chevron is active and the right one is hidden.
+    await expect(leftChevron).toBeVisible()
+    await expect(rightChevron).toBeHidden()
+
+    // One left click nudges toward the start by a single crumb — it must move,
+    // but must NOT jump all the way to 0 (the old behavior) while more than one
+    // crumb is still hidden to the left.
+    await leftChevron.click()
+    await expect.poll(scrollLeftOf).toBeLessThan(maxScroll)
+    const afterOneLeft = await scrollLeftOf()
+    expect(afterOneLeft).toBeGreaterThan(0)
+    // The right chevron appears once we're no longer at the right extreme.
+    await expect(rightChevron).toBeVisible()
+
+    // A right click walks back toward the current page by one crumb, not a full
+    // jump back to the right extreme.
+    await rightChevron.click()
+    await expect.poll(scrollLeftOf).toBeGreaterThan(afterOneLeft)
+
+    // Repeated left clicks eventually reach the start, which hides the left
+    // chevron (canScrollLeft flips false). Drive off the chevron's own visibility
+    // rather than an exact scrollLeft, since smooth scrolling can leave a
+    // sub-pixel remainder.
+    for (let i = 0; i < 6 && (await leftChevron.isVisible()); i++) {
+      await leftChevron.click()
+      await page.waitForTimeout(200)
+    }
+    await expect(leftChevron).toBeHidden()
+    await expect.poll(scrollLeftOf).toBeLessThanOrEqual(1)
+  })
+})
+
 test.describe('survey', () => {
   test.skip(!ANALYTICS_ENABLED, 'Analytics are disabled')
 

src/frame/components/page-header/BreadcrumbsScroller.module.scss

@@ -38,6 +38,24 @@
   background-color: var(--bgColor-default, var(--color-canvas-default));
 }
 
+// On hover, keep the solid canvas background (Primer's invisible IconButton
+// otherwise swaps in a translucent tint, letting crumbs bleed through the
+// button) and instead signal interactivity by bolding the chevron: a
+// currentColor stroke thickens the fill-based octicon glyph.
+.leftChevron:hover,
+.rightChevron:hover {
+  background-color: var(
+    --bgColor-default,
+    var(--color-canvas-default)
+  ) !important;
+  color: var(--fgColor-default, var(--color-fg-default));
+
+  svg {
+    stroke: currentColor;
+    stroke-width: 0.75px;
+  }
+}
+
 // Hidden chevrons are removed from view and interaction (and from the tab
 // order / a11y tree in the component). Because they're absolutely positioned
 // they already occupy no layout space, so the scroll area is unaffected.

src/frame/components/page-header/BreadcrumbsScroller.tsx

@@ -9,6 +9,10 @@ import { Breadcrumbs } from './Breadcrumbs'
 
 import styles from './BreadcrumbsScroller.module.scss'
 
+// Padding past the ~28px chevron so a revealed crumb clears it rather than
+// landing half-hidden underneath.
+const CHEVRON_PAD = 32
+
 // Wraps the secondary-bar breadcrumbs in a horizontal scroll region. When the
 // trail is too long to fit, the region scrolls — anchored to the RIGHT so the
 // current page is shown first — with a left chevron to reveal the leftmost
@@ -67,20 +71,42 @@ export const BreadcrumbsScroller = () => {
     return () => observer.disconnect()
   }, [anchorRight])
 
-  // Reveal the start of the trail (Home) in one click. Breadcrumb trails are
-  // short, so scrolling fully to the left is clearer than a partial nudge that
-  // can leave the leading crumb half-hidden behind the chevron.
+  // Reveal the previous crumb: align the crumb straddling the left edge to that
+  // edge. Targeting the boundary crumb (not stepping back from the first fully
+  // visible one) also covers a current crumb wider than the viewport. Falls back
+  // to the far left only when nothing is clipped left.
   const scrollLeftClick = useCallback(() => {
     const el = scrollRef.current
     if (!el) return
-    el.scrollTo({ left: 0 })
+    const containerLeft = el.getBoundingClientRect().left + CHEVRON_PAD
+    const crumbs = Array.from(el.querySelectorAll('li'))
+    // The rightmost crumb still clipped/off to the left: the one to reveal.
+    let target: Element | undefined
+    for (const crumb of crumbs) {
+      if (crumb.getBoundingClientRect().left < containerLeft - 1) target = crumb
+    }
+    if (!target) {
+      el.scrollTo({ left: 0 })
+      return
+    }
+    el.scrollBy({ left: target.getBoundingClientRect().left - containerLeft })
   }, [])
 
-  // Return to the current page (the end of the trail) in one click.
+  // Reveal the next crumb toward the current page. Mirrors scrollLeftClick,
+  // including the oversized-crumb case. Falls back to the far right only when
+  // nothing is clipped right.
   const scrollRightClick = useCallback(() => {
     const el = scrollRef.current
     if (!el) return
-    el.scrollTo({ left: el.scrollWidth })
+    const containerRight = el.getBoundingClientRect().right - CHEVRON_PAD
+    const crumbs = Array.from(el.querySelectorAll('li'))
+    // The leftmost crumb still clipped/off to the right: the one to reveal.
+    const target = crumbs.find((crumb) => crumb.getBoundingClientRect().right > containerRight + 1)
+    if (!target) {
+      el.scrollTo({ left: el.scrollWidth })
+      return
+    }
+    el.scrollBy({ left: target.getBoundingClientRect().right - containerRight })
   }, [])
 
   // When a crumb link receives focus via keyboard (Tab), the browser does not