fix: two strips that said "there is more" and gave no way to get there
The section bar hid its scrollbar and faded its right edge. A fade is not an affordance: a vertical wheel does not move a horizontal overflow, so with a plain mouse the last links — Courses, Settings — were simply unreachable. ScrollStrip gives it arrows that appear only while there is somewhere to go, maps the wheel onto the axis that actually scrolls, and brings the current page into view on arrival. It renders as whatever it replaces, so the bar stays a <nav> landmark rather than becoming a div. The library's column browser had two faults. Its columns were sized `clamp(220px, 32%, 320px)` — a percentage against the *visible* width, so they shrank as more opened and the strip understated how much there was to scroll. And opening a category added a column off the right-hand edge with nothing scrolling it into view. Fixed width now, the newest column scrolls itself into view, a spacer supplies the end padding that a scroll container does not count, and the scrollbar is visible because here it is the only affordance there is. Guarded scrollIntoView and scrollBy — jsdom has neither, and the first version of this broke seven navbar tests by assuming them. Frontend 316/316. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
4a28b5e0a0
commit
e176068f26
7 changed files with 253 additions and 14 deletions
|
|
@ -20,8 +20,30 @@
|
|||
cursor: pointer;
|
||||
}
|
||||
|
||||
.cc-columns { display: flex; gap: 14px; align-items: flex-start; overflow-x: auto; }
|
||||
.cc-column-slot { flex: 0 0 clamp(220px, 32%, 320px); min-width: 0; }
|
||||
.cc-columns {
|
||||
display: flex; gap: 14px; align-items: flex-start;
|
||||
overflow-x: auto; scroll-behavior: smooth;
|
||||
/* Padding on a scroll container is not part of its scrollable width in every
|
||||
browser, so the last column used to sit flush against the clipped edge
|
||||
with nothing beyond it to scroll to. The spacer element supplies the room
|
||||
instead, which every browser counts. */
|
||||
scroll-padding-inline-end: 14px;
|
||||
padding-bottom: 4px;
|
||||
}
|
||||
/* A fixed width, not a percentage. `32%` resolved against the *visible* width,
|
||||
so the columns shrank as more of them opened and the strip never quite
|
||||
admitted how much there was to scroll. */
|
||||
.cc-column-slot { flex: 0 0 min(300px, 78vw); min-width: 0; }
|
||||
/* Keeps the last column clear of the edge without relying on container padding. */
|
||||
.cc-columns::after { content: ''; flex: 0 0 4px; }
|
||||
|
||||
/* The strip scrolls; the scrollbar is what says so. Hiding it left no way to
|
||||
reach the far column with a mouse, since a vertical wheel does not move a
|
||||
horizontal overflow. */
|
||||
.cc-columns { scrollbar-width: thin; }
|
||||
.cc-columns::-webkit-scrollbar { height: 10px; }
|
||||
.cc-columns::-webkit-scrollbar-thumb { background: var(--border); border-radius: 5px; }
|
||||
.cc-columns::-webkit-scrollbar-thumb:hover { background: var(--text-subtle); }
|
||||
|
||||
.cc-column {
|
||||
background: var(--card-bg);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { useMemo, useState } from 'react'
|
||||
import { useEffect, useMemo, useRef, useState } from 'react'
|
||||
import './CategoryColumns.css'
|
||||
|
||||
/** A folder of further topics. */
|
||||
|
|
@ -126,6 +126,19 @@ export default function CategoryColumns({
|
|||
// Narrow screens show the deepest column only, with a way back up.
|
||||
const deepest = columns.length - 1
|
||||
|
||||
// Opening a category adds a column off the right-hand edge. Without this it
|
||||
// is simply not on screen, and nothing says a strip that already fills the
|
||||
// window has more to the right of it.
|
||||
const strip = useRef(null)
|
||||
useEffect(() => {
|
||||
const box = strip.current
|
||||
if (!box) return
|
||||
const last = box.lastElementChild
|
||||
if (typeof last?.scrollIntoView === 'function') {
|
||||
last.scrollIntoView({ block: 'nearest', inline: 'end', behavior: 'smooth' })
|
||||
}
|
||||
}, [columns.length])
|
||||
|
||||
return (
|
||||
<div className="cc-wrap">
|
||||
{path.length > 0 && (
|
||||
|
|
@ -134,7 +147,7 @@ export default function CategoryColumns({
|
|||
‹ {byId[path[path.length - 2]]?.name || allLabel}
|
||||
</button>
|
||||
)}
|
||||
<div className="cc-columns" data-deepest={deepest}>
|
||||
<div className="cc-columns" data-deepest={deepest} ref={strip}>
|
||||
{columns.map((column, level) => (
|
||||
<div key={`wrap-${column.parentId}-${level}`}
|
||||
className={`cc-column-slot${level === deepest ? ' is-deepest' : ''}`}>
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { useState, useEffect } from 'react'
|
||||
import { Link, useLocation } from 'react-router-dom'
|
||||
import ScrollStrip from './ScrollStrip'
|
||||
import { useAuth } from '../context/AuthContext'
|
||||
import api from '../api/client'
|
||||
import ExamSwitcher from './ExamSwitcher'
|
||||
|
|
@ -178,14 +179,18 @@ export default function Navbar({ onSignIn, onRegister }) {
|
|||
<div className={`navbar-sections${sectionBarHidden ? ' is-hidden' : ''}`}>
|
||||
<div className="container navbar-sections-inner">
|
||||
<ExamSwitcher onChange={() => window.location.reload()} />
|
||||
<nav className="nav-sections" aria-label="Sections">
|
||||
{/* The bar used to hide its scrollbar and fade the right edge,
|
||||
which says "there is more" and offers no way to get there —
|
||||
a vertical wheel does not move a horizontal overflow, so with
|
||||
a mouse the last links were unreachable. */}
|
||||
<ScrollStrip as="nav" className="nav-sections" label="Sections">
|
||||
{navLinks.map(l => (
|
||||
<Link key={l.to} to={l.to} className={location.pathname === l.to ? 'is-current' : undefined}
|
||||
aria-current={location.pathname === l.to ? 'page' : undefined}>
|
||||
{l.label}
|
||||
</Link>
|
||||
))}
|
||||
</nav>
|
||||
</ScrollStrip>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
|
|
|||
37
frontend/src/components/ScrollStrip.css
Normal file
37
frontend/src/components/ScrollStrip.css
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
/* A strip that scrolls sideways, with arrows that say so. */
|
||||
|
||||
.ss-wrap { position: relative; display: flex; flex: 1; min-width: 0; align-items: center; }
|
||||
|
||||
.ss-strip {
|
||||
display: flex; align-items: center; gap: 2px;
|
||||
flex: 1; min-width: 0;
|
||||
overflow-x: auto; overflow-y: hidden;
|
||||
scrollbar-width: none; scroll-behavior: smooth;
|
||||
-webkit-overflow-scrolling: touch;
|
||||
}
|
||||
.ss-strip::-webkit-scrollbar { display: none; }
|
||||
|
||||
/* The arrows are the affordance, so the scrollbar stays hidden here — unlike a
|
||||
content strip, a nav bar with a scrollbar through it looks broken. */
|
||||
.ss-arrow {
|
||||
position: absolute; top: 50%; transform: translateY(-50%); z-index: 2;
|
||||
width: 26px; height: 30px; padding: 0;
|
||||
display: inline-flex; align-items: center; justify-content: center;
|
||||
font: inherit; font-size: 1.1rem; line-height: 1;
|
||||
color: var(--text-muted); cursor: pointer;
|
||||
background: var(--bg); border: 1px solid var(--border); border-radius: 7px;
|
||||
box-shadow: 0 1px 4px rgba(15, 23, 42, 0.12);
|
||||
}
|
||||
.ss-arrow:hover { color: var(--primary); border-color: var(--primary); }
|
||||
.ss-arrow.is-start { left: -2px; }
|
||||
.ss-arrow.is-end { right: -2px; }
|
||||
|
||||
/* Room for the arrow to sit over, so it never covers a whole label. */
|
||||
.ss-wrap:has(.ss-arrow.is-start) .ss-strip { padding-left: 28px; }
|
||||
.ss-wrap:has(.ss-arrow.is-end) .ss-strip { padding-right: 28px; }
|
||||
|
||||
@media (max-width: 720px) {
|
||||
/* Touch scrolls directly; the arrows would only take up room. */
|
||||
.ss-arrow { display: none; }
|
||||
.ss-wrap:has(.ss-arrow) .ss-strip { padding-left: 0; padding-right: 0; }
|
||||
}
|
||||
91
frontend/src/components/ScrollStrip.jsx
Normal file
91
frontend/src/components/ScrollStrip.jsx
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import './ScrollStrip.css'
|
||||
|
||||
/**
|
||||
* A horizontal strip of things that scrolls, with a way to actually scroll it.
|
||||
*
|
||||
* The section bar hid its scrollbar and faded the right-hand edge, which says
|
||||
* "there is more" and gives you nothing to do about it: a vertical wheel does
|
||||
* not move a horizontal overflow, so with a plain mouse the last few links —
|
||||
* Courses, Settings — were simply unreachable.
|
||||
*
|
||||
* So: arrows at each end, shown only while there is somewhere to go, and the
|
||||
* wheel mapped onto the axis that actually scrolls. The current item is
|
||||
* scrolled into view on arrival, because the page you are on should be visible
|
||||
* in the bar that says where you are.
|
||||
*/
|
||||
export default function ScrollStrip({ children, className = '', label, step = 220, as: Tag = 'div', role }) {
|
||||
const box = useRef(null)
|
||||
const [edges, setEdges] = useState({ start: false, end: false })
|
||||
|
||||
const measure = useCallback(() => {
|
||||
const el = box.current
|
||||
if (!el) return
|
||||
const max = el.scrollWidth - el.clientWidth
|
||||
// A pixel of slack: sub-pixel layout leaves scrollLeft a hair short of max
|
||||
// and the end arrow would never switch off.
|
||||
setEdges({ start: el.scrollLeft > 1, end: el.scrollLeft < max - 1 })
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
const el = box.current
|
||||
if (!el) return undefined
|
||||
measure()
|
||||
el.addEventListener('scroll', measure, { passive: true })
|
||||
window.addEventListener('resize', measure)
|
||||
// Links can arrive after the first paint (a role loading, a count filling in).
|
||||
const observer = typeof ResizeObserver !== 'undefined' ? new ResizeObserver(measure) : null
|
||||
observer?.observe(el)
|
||||
return () => {
|
||||
el.removeEventListener('scroll', measure)
|
||||
window.removeEventListener('resize', measure)
|
||||
observer?.disconnect()
|
||||
}
|
||||
}, [measure, children])
|
||||
|
||||
// Bring whatever is marked as current into view. Guarded because not every
|
||||
// environment implements it — jsdom does not, and neither did older Safari.
|
||||
useEffect(() => {
|
||||
const current = box.current?.querySelector('[aria-current]')
|
||||
if (typeof current?.scrollIntoView === 'function') {
|
||||
current.scrollIntoView({ block: 'nearest', inline: 'nearest' })
|
||||
}
|
||||
}, [children])
|
||||
|
||||
const nudge = (direction) => {
|
||||
const el = box.current
|
||||
if (typeof el?.scrollBy === 'function') el.scrollBy({ left: direction * step, behavior: 'smooth' })
|
||||
else if (el) el.scrollLeft += direction * step
|
||||
}
|
||||
|
||||
const onWheel = (event) => {
|
||||
const el = box.current
|
||||
if (!el || event.deltaY === 0 || Math.abs(event.deltaX) > Math.abs(event.deltaY)) return
|
||||
const max = el.scrollWidth - el.clientWidth
|
||||
if (max <= 0) return
|
||||
// Only swallow the page's scroll while this strip can still move, so
|
||||
// reaching the end hands the wheel back to the page.
|
||||
const next = el.scrollLeft + event.deltaY
|
||||
if (next > 0 && next < max) event.preventDefault()
|
||||
el.scrollLeft = next
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="ss-wrap">
|
||||
{edges.start && (
|
||||
<button type="button" className="ss-arrow is-start" aria-label="Scroll left"
|
||||
tabIndex={-1} onClick={() => nudge(-1)}>‹</button>
|
||||
)}
|
||||
{/* Rendered as whatever it is replacing, so a section bar stays a <nav>
|
||||
landmark rather than becoming an anonymous div. */}
|
||||
<Tag ref={box} className={`ss-strip ${className}`} onWheel={onWheel}
|
||||
role={role} aria-label={label}>
|
||||
{children}
|
||||
</Tag>
|
||||
{edges.end && (
|
||||
<button type="button" className="ss-arrow is-end" aria-label="Scroll right"
|
||||
tabIndex={-1} onClick={() => nudge(1)}>›</button>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
73
frontend/src/components/ScrollStrip.test.jsx
Normal file
73
frontend/src/components/ScrollStrip.test.jsx
Normal file
|
|
@ -0,0 +1,73 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { act, render, screen } from '@testing-library/react'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import ScrollStrip from './ScrollStrip'
|
||||
|
||||
/** jsdom gives every element zero size, so the strip is told how big it is. */
|
||||
function sizeStrip({ scrollWidth, clientWidth, scrollLeft = 0 }) {
|
||||
const strip = document.querySelector('.ss-strip')
|
||||
Object.defineProperty(strip, 'scrollWidth', { value: scrollWidth, configurable: true })
|
||||
Object.defineProperty(strip, 'clientWidth', { value: clientWidth, configurable: true })
|
||||
strip.scrollLeft = scrollLeft
|
||||
strip.scrollBy = vi.fn()
|
||||
// The measurement runs off a scroll event, so its state update has to be
|
||||
// flushed before anything is asserted about the arrows.
|
||||
act(() => { strip.dispatchEvent(new Event('scroll')) })
|
||||
return strip
|
||||
}
|
||||
|
||||
const links = () => Array.from({ length: 12 }, (_, i) => <a key={i} href={`/p${i}`}>Page {i}</a>)
|
||||
|
||||
describe('a strip that scrolls sideways', () => {
|
||||
beforeEach(() => { vi.clearAllMocks() })
|
||||
|
||||
it('offers nothing when everything already fits', () => {
|
||||
render(<ScrollStrip label="Sections">{links()}</ScrollStrip>)
|
||||
sizeStrip({ scrollWidth: 400, clientWidth: 400 })
|
||||
expect(screen.queryByLabelText('Scroll right')).not.toBeInTheDocument()
|
||||
expect(screen.queryByLabelText('Scroll left')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('offers a way to the end when there is more than fits', () => {
|
||||
render(<ScrollStrip label="Sections">{links()}</ScrollStrip>)
|
||||
sizeStrip({ scrollWidth: 1200, clientWidth: 600 })
|
||||
// The old bar faded its right edge and hid the scrollbar, which said
|
||||
// "there is more" and gave a mouse no way to get there.
|
||||
expect(screen.getByLabelText('Scroll right')).toBeInTheDocument()
|
||||
expect(screen.queryByLabelText('Scroll left')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('offers the way back once you have moved', () => {
|
||||
render(<ScrollStrip label="Sections">{links()}</ScrollStrip>)
|
||||
sizeStrip({ scrollWidth: 1200, clientWidth: 600, scrollLeft: 300 })
|
||||
expect(screen.getByLabelText('Scroll left')).toBeInTheDocument()
|
||||
expect(screen.getByLabelText('Scroll right')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('stops offering the end once you are at it', () => {
|
||||
render(<ScrollStrip label="Sections">{links()}</ScrollStrip>)
|
||||
sizeStrip({ scrollWidth: 1200, clientWidth: 600, scrollLeft: 600 })
|
||||
expect(screen.queryByLabelText('Scroll right')).not.toBeInTheDocument()
|
||||
expect(screen.getByLabelText('Scroll left')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('is not fooled by a sub-pixel remainder at the end', () => {
|
||||
render(<ScrollStrip label="Sections">{links()}</ScrollStrip>)
|
||||
// scrollLeft lands a hair short of max; the arrow must still switch off.
|
||||
sizeStrip({ scrollWidth: 1200, clientWidth: 600, scrollLeft: 599.6 })
|
||||
expect(screen.queryByLabelText('Scroll right')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('moves the strip when an arrow is used', async () => {
|
||||
render(<ScrollStrip label="Sections" step={220}>{links()}</ScrollStrip>)
|
||||
const strip = sizeStrip({ scrollWidth: 1200, clientWidth: 600 })
|
||||
await userEvent.click(screen.getByLabelText('Scroll right'))
|
||||
expect(strip.scrollBy).toHaveBeenCalledWith({ left: 220, behavior: 'smooth' })
|
||||
})
|
||||
|
||||
it('keeps the arrows out of the tab order — they duplicate the links', () => {
|
||||
render(<ScrollStrip label="Sections">{links()}</ScrollStrip>)
|
||||
sizeStrip({ scrollWidth: 1200, clientWidth: 600 })
|
||||
expect(screen.getByLabelText('Scroll right')).toHaveAttribute('tabindex', '-1')
|
||||
})
|
||||
})
|
||||
|
|
@ -143,13 +143,10 @@ body {
|
|||
/* The strip scrolls when the links outrun the width; the fade on the right is
|
||||
what says so, since a hard cut just looks like a broken layout. */
|
||||
.navbar-sections-inner { position: relative; }
|
||||
.nav-sections {
|
||||
display: flex; align-items: center; gap: 2px; flex: 1; min-width: 0;
|
||||
overflow-x: auto; scrollbar-width: none;
|
||||
-webkit-mask-image: linear-gradient(90deg, #000 calc(100% - 28px), transparent);
|
||||
mask-image: linear-gradient(90deg, #000 calc(100% - 28px), transparent);
|
||||
}
|
||||
.nav-sections::-webkit-scrollbar { display: none; }
|
||||
/* Layout and scrolling belong to ScrollStrip; this only styles the links.
|
||||
The mask fade that used to sit here implied scrollability and gave nothing
|
||||
to act on — the arrows do that job now, and a fade over one of them would
|
||||
only make the arrow look broken. */
|
||||
.navbar .nav-sections a {
|
||||
color: var(--text); font-size: 0.84rem; font-weight: 500; opacity: 0.78;
|
||||
padding: 7px 11px; border-radius: 7px; white-space: nowrap;
|
||||
|
|
@ -706,7 +703,8 @@ body {
|
|||
copy of the same links competing for the same thumb. */
|
||||
.nav-burger { display: flex; }
|
||||
.nav-logout { display: none; }
|
||||
.navbar-sections .nav-sections { display: none; }
|
||||
/* Hide the whole strip, wrapper and arrows, not just the link row. */
|
||||
.navbar-sections .ss-wrap { display: none; }
|
||||
.navbar-sections, .navbar-sections-inner, .navbar-sections:focus-within { height: 42px; }
|
||||
.navbar-sections.is-hidden { height: 0; }
|
||||
/* Mobile quiz: hide sidebar, show toggle */
|
||||
|
|
|
|||
Loading…
Reference in a new issue