From 6c4ef5671cb7ceef9d4a98a91a5d0e7aaf869750 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 22:25:35 +0000 Subject: [PATCH 1/3] feat(frontend): show where wide tables hide columns and keep the scrollbar on screen Wide tables scroll inside their own box since #703, but nothing showed that columns were cut off on the right. On Opportunities (16 columns at 1280px, the last five clipped) the table's only horizontal scrollbar sat at the bottom of a page-tall table, under the sticky action box, and a row reached with Tab could sit hidden under that box. lib/table-scroll.ts, wired from index.ts: - Any table scroller with hidden columns fades that edge out (.scroll-hint-left/right, a CSS mask, so row and cell backgrounds cannot cover it; the mask leaves scrollbars at full opacity). - The Opportunities table gets a scrollbar strip that sticks just above the action box and mirrors the table's scroll position both ways. The table's own bar is hidden so only one shows; touch, trackpad and Shift+wheel still scroll the table directly. - --action-box-offset tracks the action box's real height (it wraps to 136px at 390px), used by the list padding and the strip. - Keyboard focus inside the list scrolls the page so the focused row is clear of the topbar, strip and action box. Browsers do not do this themselves because a row under a sticky bar counts as on screen. Refs #185 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/index.test.ts | 11 ++ frontend/src/index.ts | 4 + frontend/src/lib/table-scroll.ts | 197 +++++++++++++++++++++++++++ frontend/src/styles/components.css | 2 +- frontend/src/styles/responsive.css | 70 ++++++++++ 5 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 frontend/src/lib/table-scroll.ts diff --git a/frontend/src/__tests__/index.test.ts b/frontend/src/__tests__/index.test.ts index 015724152..202081a99 100644 --- a/frontend/src/__tests__/index.test.ts +++ b/frontend/src/__tests__/index.test.ts @@ -31,12 +31,17 @@ jest.mock('../auth', () => ({ logout: jest.fn() })); +jest.mock('../lib/table-scroll', () => ({ + initTableScroll: jest.fn() +})); + import * as app from '../app'; import * as plans from '../plans'; import * as recommendations from '../recommendations'; import * as history from '../history'; import * as settings from '../settings'; import * as auth from '../auth'; +import * as tableScroll from '../lib/table-scroll'; describe('Index Module', () => { beforeEach(() => { @@ -118,4 +123,10 @@ describe('Index Module', () => { resolve(undefined); }, 10)); }); + + test('wires the wide-table scroll affordances on DOMContentLoaded (#185)', () => { + require('../index'); + document.dispatchEvent(new Event('DOMContentLoaded')); + expect(tableScroll.initTableScroll).toHaveBeenCalled(); + }); }); diff --git a/frontend/src/index.ts b/frontend/src/index.ts index 962d0ba26..d38a63dca 100644 --- a/frontend/src/index.ts +++ b/frontend/src/index.ts @@ -15,6 +15,7 @@ import { logout } from './auth'; import { openCreateUserModal, closeUserModal } from './users/userModals'; import { openCreateGroupModal, closeGroupModal, addPermission, closeDuplicateGroupModal, saveDuplicateGroup } from './groups/groupModals'; import { initTopbarFilters } from './topbar-filters'; +import { initTableScroll } from './lib/table-scroll'; // Re-export for external use export { api, utils }; @@ -71,6 +72,9 @@ document.addEventListener('DOMContentLoaded', () => { // used to carry; sections subscribe to state.subscribeProvider / // subscribeAccount and reload themselves when the filter changes. initTopbarFilters(); + + // Edge fades and the Opportunities scrollbar strip for wide tables (#185). + initTableScroll(); }); // Initialize on page load diff --git a/frontend/src/lib/table-scroll.ts b/frontend/src/lib/table-scroll.ts new file mode 100644 index 000000000..d2121b1bb --- /dev/null +++ b/frontend/src/lib/table-scroll.ts @@ -0,0 +1,197 @@ +/** + * Wide-table scroll affordances (issue #185). + * + * Wide tables scroll sideways inside their own box (issue #703), but nothing + * told the user that columns were cut off on the right, and the Opportunities + * table's only horizontal scrollbar sat at the bottom of a page-tall table, + * under the sticky action box. This module adds three things: + * + * - A fade on whichever edge of a table scroller has hidden columns + * (`.scroll-hint-left` / `.scroll-hint-right`), for every table on every + * page, so a clipped column reads as "more this way". + * - For the Opportunities table, a scrollbar strip that sticks just above + * the action box while the table is on screen and mirrors the table's + * horizontal scroll position both ways. + * - `--action-box-offset`, the action box's real rendered height (it wraps + * to two or three lines on narrow screens), which the list padding and + * the strip use, plus a focus handler that scrolls a keyboard-focused + * Opportunities row out from under the sticky topbar or action box + * (browsers only scroll focus into view when it is off-screen, and a + * row under a sticky bar counts as on-screen). + * + * jsdom has no layout, so the jest tests stub the size properties; the + * Playwright spec tests-e2e/table-scroll-affordance.spec.ts covers the real + * rendering. + */ + +export const HINT_LEFT = 'scroll-hint-left'; +export const HINT_RIGHT = 'scroll-hint-right'; +export const STRIP_CLASS = 'table-scroll-strip'; +const STRIPPED_TABLE = '#recommendations-list > table'; +const ACTION_BOX_ID = 'recommendations-action-box'; + +function px(value: string): number { + return parseFloat(value) || 0; +} + +function setVar(el: HTMLElement, name: string, value: string): void { + if (el.style.getPropertyValue(name) !== value) el.style.setProperty(name, value); +} + +/** The element that scrolls a table sideways: the table itself when it is a + * block scroller, else its direct container; null when neither scrolls. */ +export function scrollerFor(table: HTMLTableElement): HTMLElement | null { + for (const el of [table, table.parentElement]) { + if (!el) continue; + const overflowX = getComputedStyle(el).overflowX; + if (overflowX === 'auto' || overflowX === 'scroll') return el; + } + return null; +} + +/** Marks the edges of `el` that have content scrolled out of view. The + * scrollbar sizes are exported so the fade's CSS mask can leave the + * scrollbars themselves at full opacity. */ +export function updateScrollHint(el: HTMLElement): void { + const max = el.scrollWidth - el.clientWidth; + const left = max > 1 && el.scrollLeft > 1; + const right = max > 1 && el.scrollLeft < max - 1; + el.classList.toggle(HINT_LEFT, left); + el.classList.toggle(HINT_RIGHT, right); + if (!left && !right) return; + const cs = getComputedStyle(el); + const sbWidth = el.offsetWidth - el.clientWidth - px(cs.borderLeftWidth) - px(cs.borderRightWidth); + const sbHeight = el.offsetHeight - el.clientHeight - px(cs.borderTopWidth) - px(cs.borderBottomWidth); + setVar(el, '--scroll-hint-sb-w', `${Math.max(0, sbWidth)}px`); + setVar(el, '--scroll-hint-sb-h', `${Math.max(0, sbHeight)}px`); +} + +function stripAfter(table: HTMLElement): HTMLElement | null { + const next = table.nextElementSibling; + return next instanceof HTMLElement && next.classList.contains(STRIP_CLASS) ? next : null; +} + +/** Ensures the mirrored scrollbar strip right after `table` exists, spans the + * same scroll range and sits at the same position. Hidden while the table + * fits. The strip is aria-hidden: it duplicates the table's own scrolling, + * which keyboard and assistive-technology users already reach. */ +export function syncScrollStrip(table: HTMLElement): HTMLElement { + let strip = stripAfter(table); + if (!strip) { + strip = document.createElement('div'); + strip.className = STRIP_CLASS; + strip.setAttribute('aria-hidden', 'true'); + strip.tabIndex = -1; + strip.appendChild(document.createElement('div')); + table.after(strip); + } + const overflow = table.scrollWidth - table.clientWidth; + const hidden = overflow <= 1; + if (strip.hidden !== hidden) strip.hidden = hidden; + if (hidden) return strip; + const spacer = strip.firstElementChild as HTMLElement; + // Same scroll range as the table, whatever the strip's own width. + const width = `${strip.clientWidth + overflow}px`; + if (spacer.style.width !== width) spacer.style.width = width; + if (strip.scrollLeft !== table.scrollLeft) strip.scrollLeft = table.scrollLeft; + return strip; +} + +/** Re-measures every table scroller, the Opportunities strip and the action + * box offset. Cheap enough to run once per animation frame after a change. */ +export function refreshTableScroll(root: ParentNode = document): void { + root.querySelectorAll('table').forEach((table) => { + const scroller = scrollerFor(table); + if (scroller) updateScrollHint(scroller); + }); + root.querySelectorAll(STRIPPED_TABLE).forEach((table) => { + syncScrollStrip(table); + }); + const box = document.getElementById(ACTION_BOX_ID); + if (box && box.offsetHeight > 0) { + setVar(document.documentElement, '--action-box-offset', `${box.offsetHeight}px`); + } +} + +/** Keeps a strip and its table scrolled to the same position, and refreshes + * the edge fade of whatever scrolled. Exported for the unit tests. */ +export function handleScroll(target: EventTarget | null): void { + if (!(target instanceof HTMLElement)) return; + if (target.classList.contains(STRIP_CLASS)) { + const table = target.previousElementSibling; + if (table instanceof HTMLElement && table.scrollLeft !== target.scrollLeft) { + table.scrollLeft = target.scrollLeft; + } + return; + } + if (target.matches(STRIPPED_TABLE)) { + const strip = stripAfter(target); + if (strip && strip.scrollLeft !== target.scrollLeft) strip.scrollLeft = target.scrollLeft; + } + if (target.classList.contains(HINT_LEFT) || target.classList.contains(HINT_RIGHT)) { + updateScrollHint(target); + } +} + +const FOCUS_MARGIN = 16; + +/** A mouse click on a half-hidden checkbox should not jolt the page, so only + * keyboard focus (`:focus-visible`) is scrolled clear. */ +function isKeyboardFocus(el: HTMLElement): boolean { + try { + return el.matches(':focus-visible'); + } catch { + return true; // engines without :focus-visible support + } +} + +/** Scrolls the page so a focused control inside the Opportunities list is + * not hidden under the sticky topbar, the strip or the action box. + * Exported for the unit tests. */ +export function keepFocusClear(target: EventTarget | null): void { + if (!(target instanceof HTMLElement) || !target.closest('#recommendations-list')) return; + if (!isKeyboardFocus(target)) return; + const rect = target.getBoundingClientRect(); + const topbar = document.querySelector('.app-topbar'); + const top = topbar ? topbar.getBoundingClientRect().bottom : 0; + let bottom = window.innerHeight; + const box = document.getElementById(ACTION_BOX_ID); + if (box && box.offsetHeight > 0) bottom = Math.min(bottom, box.getBoundingClientRect().top); + const strip = document.querySelector(`${STRIPPED_TABLE} + .${STRIP_CLASS}`); + if (strip && !strip.hidden) bottom = Math.min(bottom, strip.getBoundingClientRect().top); + if (rect.bottom > bottom) { + window.scrollBy(0, rect.bottom - bottom + FOCUS_MARGIN); + } else if (rect.top < top) { + window.scrollBy(0, rect.top - top - FOCUS_MARGIN); + } +} + +let initialized = false; + +/** Wires the affordances for the app's lifetime: refresh after DOM changes + * (tables are re-rendered wholesale), on resize, and on every scroll. */ +export function initTableScroll(): void { + if (initialized) return; + initialized = true; + let queued = false; + const schedule = (): void => { + if (queued) return; + queued = true; + requestAnimationFrame(() => { + queued = false; + refreshTableScroll(); + }); + }; + document.addEventListener('scroll', (e) => handleScroll(e.target), { capture: true, passive: true }); + document.addEventListener('focusin', (e) => keepFocusClear(e.target)); + window.addEventListener('resize', schedule); + // class/hidden catch tab switches and the sidebar collapsing, both of + // which change a table's width without touching its children. + new MutationObserver(schedule).observe(document.body, { + childList: true, + subtree: true, + attributes: true, + attributeFilter: ['class', 'hidden'], + }); + schedule(); +} diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 5d76ec88e..745bb38d2 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -863,7 +863,7 @@ th .column-filter-btn.active { gap: 0.4rem; font-size: 0.9em; } -#recommendations-list { padding-bottom: var(--action-box-height); } +#recommendations-list { padding-bottom: var(--action-box-offset, var(--action-box-height)); } tr.recommendation-row { cursor: pointer; diff --git a/frontend/src/styles/responsive.css b/frontend/src/styles/responsive.css index 0fbe0ee62..68b94b88a 100644 --- a/frontend/src/styles/responsive.css +++ b/frontend/src/styles/responsive.css @@ -362,3 +362,73 @@ body.sidebar-open { max-width: 100%; overflow-x: auto; } + +/* Wide-table scroll affordances (issue #185); lib/table-scroll.ts sets the + * classes and variables. A scroller with columns cut off on one side fades + * that edge out, so the clipped column reads as "more this way". The fade is + * a mask on the scroller's own box, so cell and row backgrounds cannot cover + * it; the extra mask layers keep any scrollbar at full opacity. */ +.scroll-hint-left, +.scroll-hint-right { + --scroll-hint-l: #000; + --scroll-hint-r: #000; + --scroll-hint-fade: linear-gradient(to right, var(--scroll-hint-l), #000 4rem, #000 calc(100% - 4rem), var(--scroll-hint-r)); + -webkit-mask-image: var(--scroll-hint-fade), linear-gradient(#000, #000), linear-gradient(#000, #000); + mask-image: var(--scroll-hint-fade), linear-gradient(#000, #000), linear-gradient(#000, #000); + -webkit-mask-size: calc(100% - var(--scroll-hint-sb-w, 0px)) calc(100% - var(--scroll-hint-sb-h, 0px)), var(--scroll-hint-sb-w, 0px) 100%, 100% var(--scroll-hint-sb-h, 0px); + mask-size: calc(100% - var(--scroll-hint-sb-w, 0px)) calc(100% - var(--scroll-hint-sb-h, 0px)), var(--scroll-hint-sb-w, 0px) 100%, 100% var(--scroll-hint-sb-h, 0px); + -webkit-mask-position: 0 0, 100% 0, 0 100%; + mask-position: 0 0, 100% 0, 0 100%; + -webkit-mask-repeat: no-repeat; + mask-repeat: no-repeat; +} +.scroll-hint-left { --scroll-hint-l: rgb(0 0 0 / 0.12); } +.scroll-hint-right { --scroll-hint-r: rgb(0 0 0 / 0.12); } + +/* The Opportunities table is page-tall, so its own horizontal scrollbar sat + * off-screen under the action box. The strip after it mirrors that scrollbar + * and sticks just above the action box while the table is on screen; the + * table's own bar is hidden so the two never show at once (touch, trackpad + * and Shift+wheel still scroll the table directly). The ::-webkit-scrollbar + * rules keep the strip's bar visible where the OS uses overlay scrollbars; + * Chromium ignores them once scrollbar-color is set, hence the @supports. */ +#recommendations-list > table { + scrollbar-width: none; +} +#recommendations-list > table::-webkit-scrollbar { + display: none; +} +.table-scroll-strip { + position: sticky; + bottom: var(--action-box-offset, var(--action-box-height)); + z-index: 9; + overflow-x: scroll; + overflow-y: hidden; + background: var(--cudly-surface); + border-top: 1px solid var(--cudly-border); +} +.table-scroll-strip[hidden] { + display: none; +} +.table-scroll-strip > div { + height: 1px; +} +.table-scroll-strip::-webkit-scrollbar { + height: 12px; +} +.table-scroll-strip::-webkit-scrollbar-track { + background: var(--cudly-surface-muted); +} +.table-scroll-strip::-webkit-scrollbar-thumb { + background: var(--cudly-border-strong); + border: 2px solid var(--cudly-surface-muted); + border-radius: var(--cudly-r-full); +} +.table-scroll-strip::-webkit-scrollbar-thumb:hover { + background: var(--cudly-text-subtle); +} +@supports not selector(::-webkit-scrollbar) { + .table-scroll-strip { + scrollbar-color: var(--cudly-border-strong) var(--cudly-surface-muted); + } +} From dff58b00df561d25ec34f70e692dc988722aaa7a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 22:25:35 +0000 Subject: [PATCH 2/3] test(frontend): cover the wide-table scroll affordances Jest covers the scroller detection, edge hints, strip sync and focus scrolling with stubbed layout (jsdom has none). The Playwright spec checks the real rendering at 1280px and 390px with 40 rows: the clipped edge fades, the strip sits on screen right above the action box and stays in step with the table both ways, a table that fits gets neither, and Tab onto a row under the action box scrolls it clear. Refs #185 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/table-scroll.test.ts | 248 ++++++++++++++++++ .../tests-e2e/table-scroll-affordance.spec.ts | 115 ++++++++ 2 files changed, 363 insertions(+) create mode 100644 frontend/src/__tests__/table-scroll.test.ts create mode 100644 frontend/tests-e2e/table-scroll-affordance.spec.ts diff --git a/frontend/src/__tests__/table-scroll.test.ts b/frontend/src/__tests__/table-scroll.test.ts new file mode 100644 index 000000000..81c65ec0a --- /dev/null +++ b/frontend/src/__tests__/table-scroll.test.ts @@ -0,0 +1,248 @@ +/** + * Wide-table scroll affordance tests (issue #185). + * + * jsdom has no layout engine, so each test stubs the size and scroll + * properties the module reads. The real rendering (fade, sticky strip above + * the action box) is covered by tests-e2e/table-scroll-affordance.spec.ts. + */ + +import { + HINT_LEFT, + HINT_RIGHT, + STRIP_CLASS, + handleScroll, + keepFocusClear, + refreshTableScroll, + scrollerFor, + syncScrollStrip, + updateScrollHint, +} from '../lib/table-scroll'; + +interface Size { + scrollWidth?: number; + clientWidth?: number; + offsetWidth?: number; + offsetHeight?: number; + clientHeight?: number; +} + +function stubSize(el: HTMLElement, size: Size): void { + for (const [key, value] of Object.entries(size)) { + Object.defineProperty(el, key, { configurable: true, value }); + } +} + +function hints(el: HTMLElement): string[] { + return [HINT_LEFT, HINT_RIGHT].filter((c) => el.classList.contains(c)); +} + +function mountRecommendationsTable(): HTMLTableElement { + document.body.innerHTML = '
x
'; + const table = document.querySelector('table')!; + table.style.overflowX = 'auto'; + return table; +} + +describe('scrollerFor', () => { + afterEach(() => { document.body.innerHTML = ''; }); + + test('returns the table when the table itself scrolls', () => { + document.body.innerHTML = '
'; + const table = document.querySelector('table')!; + table.style.overflowX = 'auto'; + expect(scrollerFor(table)).toBe(table); + }); + + test('returns the direct container when it scrolls', () => { + document.body.innerHTML = '
'; + const table = document.querySelector('table')!; + expect(scrollerFor(table)).toBe(table.parentElement); + }); + + test('returns null when neither scrolls', () => { + document.body.innerHTML = '
'; + expect(scrollerFor(document.querySelector('table')!)).toBeNull(); + }); +}); + +describe('updateScrollHint', () => { + test('a table that fits gets no hint', () => { + const el = document.createElement('div'); + stubSize(el, { scrollWidth: 500, clientWidth: 500 }); + updateScrollHint(el); + expect(hints(el)).toEqual([]); + }); + + test('at the left edge only the right side is hinted', () => { + const el = document.createElement('div'); + stubSize(el, { scrollWidth: 1400, clientWidth: 1000, offsetWidth: 1000, offsetHeight: 300, clientHeight: 288 }); + updateScrollHint(el); + expect(hints(el)).toEqual([HINT_RIGHT]); + // A horizontal scrollbar's height is exported so the fade skips it. + expect(el.style.getPropertyValue('--scroll-hint-sb-h')).toBe('12px'); + expect(el.style.getPropertyValue('--scroll-hint-sb-w')).toBe('0px'); + }); + + test('in the middle both sides are hinted, at the end only the left', () => { + const el = document.createElement('div'); + stubSize(el, { scrollWidth: 1400, clientWidth: 1000 }); + el.scrollLeft = 200; + updateScrollHint(el); + expect(hints(el)).toEqual([HINT_LEFT, HINT_RIGHT]); + el.scrollLeft = 400; + updateScrollHint(el); + expect(hints(el)).toEqual([HINT_LEFT]); + }); + + test('hints are dropped once the table fits again', () => { + const el = document.createElement('div'); + stubSize(el, { scrollWidth: 1400, clientWidth: 1000 }); + updateScrollHint(el); + stubSize(el, { scrollWidth: 1400, clientWidth: 1400 }); + updateScrollHint(el); + expect(hints(el)).toEqual([]); + }); +}); + +describe('syncScrollStrip', () => { + afterEach(() => { document.body.innerHTML = ''; }); + + test('adds one aria-hidden strip right after the table, spanning the same scroll range', () => { + const table = mountRecommendationsTable(); + stubSize(table, { scrollWidth: 1400, clientWidth: 1000 }); + const strip = syncScrollStrip(table); + stubSize(strip, { clientWidth: 1000 }); + syncScrollStrip(table); + + expect(table.nextElementSibling).toBe(strip); + expect(document.querySelectorAll(`.${STRIP_CLASS}`)).toHaveLength(1); + expect(strip.getAttribute('aria-hidden')).toBe('true'); + expect(strip.tabIndex).toBe(-1); + expect(strip.hidden).toBe(false); + expect((strip.firstElementChild as HTMLElement).style.width).toBe('1400px'); + }); + + test('hides the strip while the table fits', () => { + const table = mountRecommendationsTable(); + stubSize(table, { scrollWidth: 1000, clientWidth: 1000 }); + expect(syncScrollStrip(table).hidden).toBe(true); + }); + + test('scrolling the strip scrolls the table and the other way round', () => { + const table = mountRecommendationsTable(); + stubSize(table, { scrollWidth: 1400, clientWidth: 1000 }); + const strip = syncScrollStrip(table); + + strip.scrollLeft = 150; + handleScroll(strip); + expect(table.scrollLeft).toBe(150); + + table.scrollLeft = 320; + handleScroll(table); + expect(strip.scrollLeft).toBe(320); + }); + + test('a scroll on a hinted table refreshes its edge hints', () => { + const table = mountRecommendationsTable(); + stubSize(table, { scrollWidth: 1400, clientWidth: 1000 }); + updateScrollHint(table); + table.scrollLeft = 400; + handleScroll(table); + expect(hints(table)).toEqual([HINT_LEFT]); + }); +}); + +describe('refreshTableScroll', () => { + afterEach(() => { + document.body.innerHTML = ''; + document.documentElement.style.removeProperty('--action-box-offset'); + }); + + test('hints scrollers, adds the Opportunities strip and records the action box height', () => { + const table = mountRecommendationsTable(); + stubSize(table, { scrollWidth: 1400, clientWidth: 1000 }); + const box = document.createElement('div'); + box.id = 'recommendations-action-box'; + stubSize(box, { offsetHeight: 136 }); + document.body.appendChild(box); + + refreshTableScroll(); + + expect(hints(table)).toEqual([HINT_RIGHT]); + expect(table.nextElementSibling?.classList.contains(STRIP_CLASS)).toBe(true); + expect(document.documentElement.style.getPropertyValue('--action-box-offset')).toBe('136px'); + }); + + test('other tables get the hint but no strip', () => { + document.body.innerHTML = '
'; + const wrap = document.getElementById('history-list')!; + stubSize(wrap, { scrollWidth: 900, clientWidth: 400 }); + refreshTableScroll(); + expect(hints(wrap)).toEqual([HINT_RIGHT]); + expect(document.querySelector(`.${STRIP_CLASS}`)).toBeNull(); + }); + + test('a hidden action box leaves the last measured offset alone', () => { + document.documentElement.style.setProperty('--action-box-offset', '93px'); + const box = document.createElement('div'); + box.id = 'recommendations-action-box'; + stubSize(box, { offsetHeight: 0 }); + document.body.appendChild(box); + refreshTableScroll(); + expect(document.documentElement.style.getPropertyValue('--action-box-offset')).toBe('93px'); + }); +}); + +describe('keepFocusClear', () => { + let scrollBy: jest.SpyInstance; + + function rect(top: number, bottom: number): DOMRect { + return { top, bottom, left: 0, right: 0, width: 0, height: bottom - top, x: 0, y: top, toJSON: () => ({}) } as DOMRect; + } + + function mount(rowTop: number, keyboard = true): HTMLInputElement { + document.body.innerHTML = ` +
+
+
+
`; + const input = document.querySelector('input')!; + jest.spyOn(input, 'getBoundingClientRect').mockReturnValue(rect(rowTop, rowTop + 20)); + jest.spyOn(input, 'matches').mockImplementation((sel: string) => (sel === ':focus-visible' ? keyboard : false)); + jest.spyOn(document.querySelector('.app-topbar')!, 'getBoundingClientRect').mockReturnValue(rect(0, 56)); + jest.spyOn(document.querySelector('.table-scroll-strip')!, 'getBoundingClientRect').mockReturnValue(rect(706, 720)); + const box = document.getElementById('recommendations-action-box')!; + stubSize(box, { offsetHeight: 80 }); + jest.spyOn(box, 'getBoundingClientRect').mockReturnValue(rect(720, 800)); + return input; + } + + beforeEach(() => { + Object.defineProperty(window, 'innerHeight', { configurable: true, value: 800 }); + scrollBy = jest.spyOn(window, 'scrollBy').mockImplementation(() => undefined); + }); + afterEach(() => { + scrollBy.mockRestore(); + document.body.innerHTML = ''; + }); + + test('a row focused under the strip and action box is scrolled above them', () => { + keepFocusClear(mount(700)); + // Row bottom 720 vs strip top 706: 14px plus the 16px margin. + expect(scrollBy).toHaveBeenCalledWith(0, 30); + }); + + test('a row focused under the topbar is scrolled below it', () => { + keepFocusClear(mount(40)); + expect(scrollBy).toHaveBeenCalledWith(0, -32); + }); + + test('a row in the clear, a mouse click, or focus outside the list does nothing', () => { + keepFocusClear(mount(300)); + keepFocusClear(mount(700, false)); + const outside = document.createElement('button'); + document.body.appendChild(outside); + keepFocusClear(outside); + expect(scrollBy).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/tests-e2e/table-scroll-affordance.spec.ts b/frontend/tests-e2e/table-scroll-affordance.spec.ts new file mode 100644 index 000000000..52e59cb4b --- /dev/null +++ b/frontend/tests-e2e/table-scroll-affordance.spec.ts @@ -0,0 +1,115 @@ +/** + * Wide-table scroll affordances (issue #185). + * + * The Opportunities table is wider than the content area at common laptop + * widths, and it is page-tall, so its own horizontal scrollbar used to sit + * off-screen under the sticky action box with nothing showing that columns + * were cut off. This checks, in a real browser, that the clipped edge fades, + * that a scrollbar strip sits on screen just above the action box and stays + * in step with the table, and that keyboard focus keeps rows clear of the + * action box. + */ + +import { test, expect, type Page } from '@playwright/test'; +import { mockApi, seedAuth, RECS } from './fixtures/recs'; + +// 40 rows so the table runs well past the bottom of the viewport. +const MANY = Array.from({ length: 40 }, (_, i) => ({ + ...RECS[i % RECS.length]!, + id: `wide-${i}`, + region: i % 2 ? 'ap-southeast-2' : RECS[i % RECS.length]!.region, + count: i + 1, +})); + +async function openOpportunities(page: Page, width: number): Promise { + await page.setViewportSize({ width, height: 900 }); + await seedAuth(page); + await mockApi(page); + await page.route( + (url) => url.pathname === '/api/recommendations', + (route) => (route.request().method() === 'GET' + ? route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + recommendations: MANY, + summary: { total_recommendations: MANY.length, total_upfront_cost: 0, potential_monthly_savings: 0, avg_payback_months: 6 }, + }), + }) + : route.fallback()), + ); + await page.goto('/opportunities'); + await page.waitForLoadState('networkidle'); +} + +const table = '#recommendations-list > table'; +const strip = '#recommendations-list > .table-scroll-strip'; + +for (const width of [1280, 390]) { + test(`opportunities table shows where its hidden columns are at ${width}px`, async ({ page }) => { + await openOpportunities(page, width); + + const tbl = page.locator(table); + await expect(tbl).toHaveClass(/scroll-hint-right/); + await expect(tbl).not.toHaveClass(/scroll-hint-left/); + + // The strip is on screen at the top of the page, right above the action box. + const bar = page.locator(strip); + await expect(bar).toBeVisible(); + const barBox = (await bar.boundingBox())!; + const actionBox = (await page.locator('#recommendations-action-box').boundingBox())!; + expect(barBox.y + barBox.height).toBeLessThanOrEqual(actionBox.y + 1); + expect(barBox.y + barBox.height).toBeGreaterThan(actionBox.y - 2); + expect(barBox.y).toBeGreaterThan(0); + + // Scrolling the strip scrolls the table, and both edges fade mid-way. + await bar.evaluate((el) => { el.scrollLeft = 120; }); + await expect.poll(() => tbl.evaluate((el) => el.scrollLeft)).toBe(120); + await expect(tbl).toHaveClass(/scroll-hint-left/); + await expect(tbl).toHaveClass(/scroll-hint-right/); + + // Scrolling the table (trackpad, Shift+wheel) moves the strip too; at the + // far end only the left edge fades. + await tbl.evaluate((el) => { el.scrollLeft = el.scrollWidth; }); + const end = await tbl.evaluate((el) => el.scrollLeft); + await expect.poll(() => bar.evaluate((el) => el.scrollLeft)).toBe(end); + await expect(tbl).not.toHaveClass(/scroll-hint-right/); + + // Still no page-level horizontal scroll (#703). + expect(await page.evaluate(() => document.documentElement.scrollWidth - document.documentElement.clientWidth)).toBeLessThanOrEqual(0); + }); +} + +test('keyboard focus scrolls a row clear of the action box', async ({ page }) => { + await openOpportunities(page, 1280); + const actionTop = async (): Promise => (await page.locator('#recommendations-action-box').boundingBox())!.y; + + // A row checkbox that starts underneath the action box. + const boxes = page.locator(`${table} tbody input[type="checkbox"]`); + const top = await actionTop(); + let target = -1; + for (let i = 0; i < await boxes.count(); i++) { + const b = await boxes.nth(i).boundingBox(); + if (b && b.y > top) { target = i; break; } + } + expect(target).toBeGreaterThan(0); + + await boxes.nth(target - 1).focus(); + await page.keyboard.press('Tab'); + // Tab may land on the row's Plan button first; either way the focused + // control must end up above the strip and the action box (the page + // scrolls smoothly, hence the poll). + await expect.poll(async () => { + const focusedBottom = await page.evaluate(() => document.activeElement!.getBoundingClientRect().bottom); + const stripTop = (await page.locator(strip).boundingBox())!.y; + return focusedBottom <= Math.min(stripTop, await actionTop()); + }).toBe(true); +}); + +test('a table that fits gets no fade and no strip', async ({ page }) => { + await openOpportunities(page, 1920); + const tbl = page.locator(table); + expect(await tbl.evaluate((el) => el.scrollWidth <= el.clientWidth)).toBe(true); + await expect(page.locator('.scroll-hint-left, .scroll-hint-right')).toHaveCount(0); + await expect(page.locator(strip)).toBeHidden(); +}); From 6f8dd286e7b705ab87928641f411cd6d528cad15 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 13:31:41 +0000 Subject: [PATCH 3/3] fix(frontend): give the JS-set scroll-hint and action-box properties CSS defaults The stylesheets read --scroll-hint-sb-w, --scroll-hint-sb-h and --action-box-offset, but only table-scroll.ts ever set them, so no stylesheet defined them and #745's custom-property lint flags the reads as undefined. Declare :root defaults that equal the old var() fallbacks (0px, and --action-box-height). The inline values the module sets on the scroller and on still win over :root, so rendering is unchanged. Refs #185 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/table-scroll.test.ts | 17 +++++++++++++++++ frontend/src/styles/components.css | 2 +- frontend/src/styles/responsive.css | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/table-scroll.test.ts b/frontend/src/__tests__/table-scroll.test.ts index 81c65ec0a..c03faf006 100644 --- a/frontend/src/__tests__/table-scroll.test.ts +++ b/frontend/src/__tests__/table-scroll.test.ts @@ -6,6 +6,8 @@ * the action box) is covered by tests-e2e/table-scroll-affordance.spec.ts. */ +import fs from 'fs'; +import path from 'path'; import { HINT_LEFT, HINT_RIGHT, @@ -246,3 +248,18 @@ describe('keepFocusClear', () => { expect(scrollBy).not.toHaveBeenCalled(); }); }); + +test('every custom property the module sets has a stylesheet default', () => { + // The stylesheets read these with var(); a CSS default keeps them defined + // before the first measurement and for the custom-property lint (#745). + const src = path.join(__dirname, '..'); + const source = fs.readFileSync(path.join(src, 'lib', 'table-scroll.ts'), 'utf8'); + const set = [...source.matchAll(/setVar\([^,]+,\s*'(--[\w-]+)'/g)].map((m) => m[1]!); + expect(set.sort()).toEqual(['--action-box-offset', '--scroll-hint-sb-h', '--scroll-hint-sb-w']); + const stylesDir = path.join(src, 'styles'); + const css = fs.readdirSync(stylesDir) + .filter((f) => f.endsWith('.css')) + .map((f) => fs.readFileSync(path.join(stylesDir, f), 'utf8')) + .join('\n'); + expect(set.filter((name) => !new RegExp(`:root\\s*\\{[^}]*${name}\\s*:`).test(css))).toEqual([]); +}); diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 745bb38d2..c9f13c2c1 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -810,7 +810,7 @@ th .column-filter-btn.active { * up the design tokens so colors + spacing + radii sit in the same * visual family as the rest of the action-center reskin. Drawer is * tracked as a deferred sub-task in #340. */ -:root { --action-box-height: 80px; } +:root { --action-box-height: 80px; --action-box-offset: var(--action-box-height); } .recommendations-action-box { position: sticky; bottom: 0; diff --git a/frontend/src/styles/responsive.css b/frontend/src/styles/responsive.css index 68b94b88a..544fe44bf 100644 --- a/frontend/src/styles/responsive.css +++ b/frontend/src/styles/responsive.css @@ -368,6 +368,7 @@ body.sidebar-open { * that edge out, so the clipped column reads as "more this way". The fade is * a mask on the scroller's own box, so cell and row backgrounds cannot cover * it; the extra mask layers keep any scrollbar at full opacity. */ +:root { --scroll-hint-sb-w: 0px; --scroll-hint-sb-h: 0px; } .scroll-hint-left, .scroll-hint-right { --scroll-hint-l: #000;