oficina / slides Public

Keep the editor strip off its own label; square the in-flow highlights

The presence avatars now sit inside the thumbnail's bottom-left corner
([slides-thumb] is positioned) instead of over the item label, and the
strip kebab is a compact 28px icon button in the thumbnail's top-right,
shown on the current item, on hover, on focus-within and while open
(always on devices with no hover). 'Current slide' is screen-reader
only, no longer the app's one uppercase label.

Square rule: the current strip item is a square band (the thumbnail
keeps its own frame), the pressed Blocks row is square, and the
segmented switcher's hard-coded 8px is var(--rst-radius).

TestBrowserStripDoesNotOverlapItself measures label vs avatars vs
trigger at 1440 and 800 with a real second tab's avatar.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 8b5141e8814c409d12f1efc002e7a1c99349d792 parent 4e853dc
2 files changed, +223 −17
  • internal/slides/static/editor.css +48 −17
  • internal/slidestest/browser_editor_strip_test.go +175 −0
diff --git a/internal/slides/static/editor.css b/internal/slides/static/editor.css
index f85b0e8..63d2faf 100644
--- a/internal/slides/static/editor.css
+++ b/internal/slides/static/editor.css
@@ -98,9 +98,13 @@
overflow-x: auto;
padding: 0 0 var(--rst-sp-2);
}
+/* An item is a row of the list, not a card: the thumbnail carries the
+ frame, and the current item is a square band behind it — an in-flow
+ highlight, so square, like every other selected row in the suite.
+ The band is a fill AND a border, and aria-current plus the
+ screen-reader "Current slide" say it in words (never colour alone). */
[slides-strip-item] {
- border: 1px solid var(--rst-line);
- border-radius: var(--rst-radius);
+ border: 1px solid transparent;
box-sizing: border-box;
display: grid;
flex: none;
@@ -127,7 +131,9 @@
aspect-ratio: 16 / 9;
background: var(--rst-surface-2);
border-radius: var(--rst-radius-sm);
+ box-shadow: 0 0 0 1px var(--rst-line);
overflow: hidden;
+ position: relative;
width: 180px;
}
[slides-thumb-img] {
@@ -156,7 +162,6 @@
outline: none;
}
[slides-strip-link]:focus-visible::after {
- border-radius: var(--rst-radius);
outline: 2px solid var(--rst-accent);
outline-offset: 2px;
}
@@ -166,11 +171,15 @@
text-overflow: ellipsis;
white-space: nowrap;
}
+/* "Current slide" is for a screen reader: the band says it on screen.
+ Visually hidden the way rst-sr-only is, kept in the link's name. */
[slides-strip-current] {
- color: var(--rst-accent);
- font-size: var(--rst-fs-xs);
- text-transform: uppercase;
- letter-spacing: 0.04em;
+ block-size: 1px;
+ clip-path: inset(50%);
+ inline-size: 1px;
+ overflow: hidden;
+ position: absolute;
+ white-space: nowrap;
}
[slides-strip-item]:not([data-current="true"]) [slides-strip-current] {
display: none;
@@ -378,6 +387,7 @@
[slides-blocks-buttons] > button[aria-pressed="true"] {
background: var(--rst-accent-soft);
border-color: var(--rst-accent);
+ border-radius: 0;
}
/* ── The stage bar: the switcher and the notes toggle ────────────── */
@@ -395,7 +405,7 @@
[slides-switcher] {
background: var(--rst-accent-soft);
border: 0;
- border-radius: 8px;
+ border-radius: var(--rst-radius);
display: flex;
gap: 2px;
margin: 0;
@@ -543,17 +553,37 @@
/* ── The strip's menu, and reordering ────────────────────────────── */
-/* The item menu sits over the thumbnail's corner, above the link's
- stretched box, so it is pressable. */
+/* The item menu sits inside the thumbnail's top-right corner, above
+ the link's stretched box so it is pressable, and clear of the label
+ under the thumbnail. A compact icon button — 28px, over the 24px
+ target minimum — shown on the current item, on hover, while the
+ item holds focus and while the menu is open; hidden with opacity, not
+ display, so it stays in the tab order and focus is what reveals it.
+ A device with no hover shows it always. */
[slides-strip-menu] {
position: absolute;
- right: var(--rst-sp-2);
- top: var(--rst-sp-2);
+ right: calc(var(--rst-sp-2) + 4px);
+ top: calc(var(--rst-sp-2) + 4px);
z-index: 2;
}
[slides-strip-menu] > summary {
- min-height: 24px;
- min-width: 24px;
+ block-size: 28px;
+ box-sizing: border-box;
+ inline-size: 28px;
+ justify-content: center;
+ opacity: 0;
+ padding: 0;
+}
+[slides-strip-item][data-current="true"] [slides-strip-menu] > summary,
+[slides-strip-item]:hover [slides-strip-menu] > summary,
+[slides-strip-item]:focus-within [slides-strip-menu] > summary,
+[slides-strip-menu][open] > summary {
+ opacity: 1;
+}
+@media (hover: none) {
+ [slides-strip-menu] > summary {
+ opacity: 1;
+ }
}
[slides-strip-item][draggable] {
cursor: grab;
@@ -575,9 +605,10 @@
carries the name too, in `title` and its accessible label — the
colour on screen is decoration on top of that, not the message. */
-/* Up to three faces on the thumbnail of the slide each person is on,
- at its corner free of the item menu (right: var(--rst-sp-2); top:
- var(--rst-sp-2), above). */
+/* Up to three faces inside the thumbnail of the slide each person is
+ on (presence-view.mjs appends them to [slides-thumb], which is
+ positioned), at its bottom-left corner: free of the item menu at
+ the top-right, and never over the label under the thumbnail. */
[slides-presence-avatars] {
bottom: var(--rst-sp-1);
display: flex;
diff --git a/internal/slidestest/browser_editor_strip_test.go b/internal/slidestest/browser_editor_strip_test.go
new file mode 100644
index 0000000..dd05d34
--- /dev/null
+++ b/internal/slidestest/browser_editor_strip_test.go
@@ -0,0 +1,175 @@
+//go:build browser
+
+// The editor's round-2 polish, held in a real Chromium: the strip does
+// not draw over its own words, a reload is not somebody else arriving,
+// and a modal dialog owns the keyboard. Each of these was a defect a
+// person could see (polish audit, slides.md), and none of them is
+// visible to a markup test — they are geometry, a sessionStorage and a
+// key event's route through the page.
+package slidestest
+
+import (
+ "fmt"
+ "testing"
+ "time"
+
+ "amadan.net/rastrillo/rastrillo/harness"
+ "github.com/chromedp/cdproto/input"
+ "github.com/chromedp/chromedp"
+)
+
+const editorBound = `document.querySelector('[slides-stage] [data-block="heading"]')?.getAttribute('contenteditable') === 'true'`
+
+// openEditor signs in and opens the sample deck's editor, bound.
+func openEditor(t *testing.T) (*harness.Rig, *fakeMailer, string) {
+ t.Helper()
+ rig, mailer := newRig(t)
+ browserSignIn(t, rig, mailer)
+ var deck string
+ rig.Run(chromedp.Evaluate(`(document.querySelector('a[href^="/d/"]')?.getAttribute('href') ?? '')`, &deck))
+ if !deckPath.MatchString(deck) {
+ t.Fatalf("the deck list links no deck: %q", deck)
+ }
+ rig.Run(
+ chromedp.Navigate(rig.Origin+deck+"/edit"),
+ chromedp.WaitVisible(`[slides-editor]`, chromedp.ByQuery),
+ waitFor(editorBound),
+ )
+ return rig, mailer, deck
+}
+
+// stripGeometry measures every strip item: its label, its avatars and
+// its menu trigger, and reports each overlap, each trigger under the
+// 24px target minimum, and each avatar box outside its thumbnail. An
+// empty answer is the pass. Items outside the strip's scrollport are
+// measured too — getBoundingClientRect does not care.
+const stripGeometry = `(() => {
+ const r = el => el && el.getBoundingClientRect()
+ const hit = (a, b) => a && b && a.width > 0 && b.width > 0 &&
+ a.left < b.right && b.left < a.right && a.top < b.bottom && b.top < a.bottom
+ const inside = (a, b) => a.left >= b.left - 0.5 && a.right <= b.right + 0.5 && a.top >= b.top - 0.5 && a.bottom <= b.bottom + 0.5
+ const out = []
+ let avatars = 0
+ for (const li of document.querySelectorAll('[slides-strip-item]')) {
+ const id = li.dataset.slideId
+ const label = r(li.querySelector('[slides-strip-label]'))
+ const thumb = r(li.querySelector('[slides-thumb]'))
+ const av = r(li.querySelector('[slides-presence-avatars]'))
+ const menu = r(li.querySelector('[slides-strip-menu] > summary'))
+ if (av) avatars++
+ if (hit(label, av)) out.push(id + ': the label is under the avatars')
+ if (hit(label, menu)) out.push(id + ': the label is under the menu trigger')
+ if (av && !inside(av, thumb)) out.push(id + ': the avatars are outside the thumbnail')
+ if (menu && (menu.width < 24 || menu.height < 24)) out.push(id + ': the menu trigger is ' + menu.width + 'x' + menu.height + ', under 24x24')
+ if (menu && (menu.width > 32 || menu.height > 32)) out.push(id + ': the menu trigger is ' + menu.width + 'x' + menu.height + ', not a compact icon button')
+ if (menu && !inside(menu, thumb)) out.push(id + ': the menu trigger is outside the thumbnail')
+ }
+ if (avatars === 0) out.push('no strip item carries an avatar — nothing was measured')
+ return out.join('\n')
+})()`
+
+// TestBrowserStripDoesNotOverlapItself (polish audit, "PA RRENT
+// SLIDE"): with another tab's avatar on the current slide, the item's
+// label is clear of the avatars and of the menu trigger, the trigger is
+// a compact 24px-or-more target, and the words "Current slide" are for
+// a screen reader only — at 1440, where the strip is a column, and at
+// 800, where it is a row.
+func TestBrowserStripDoesNotOverlapItself(t *testing.T) {
+ rig, _, deck := openEditor(t)
+ var slide string
+ rig.Run(chromedp.Evaluate(`document.querySelector('[slides-editor]').dataset.slide`, &slide))
+
+ // A real second tab, so the avatar is presence-view.mjs's own and
+ // sits wherever the bundle puts it.
+ tab2, cancel := chromedp.NewContext(rig.Context())
+ defer cancel()
+ runOn(t, tab2,
+ chromedp.Navigate(rig.Origin+deck+"/edit"),
+ chromedp.WaitVisible(`[slides-editor]`, chromedp.ByQuery),
+ waitFor(editorBound),
+ )
+ rig.Run(waitFor(`document.querySelector('[slides-strip-item][data-slide-id="` + slide + `"] [slides-presence-avatar]') !== null`))
+
+ for _, width := range []int64{1440, 800} {
+ rig.Run(
+ chromedp.EmulateViewport(width, 900),
+ // Park the pointer away from the strip: hover is one of the
+ // states that shows a menu trigger, and this is the resting
+ // look being measured.
+ chromedp.MouseEvent(input.MouseMoved, float64(width)-5, 895),
+ chromedp.Sleep(300*time.Millisecond),
+ )
+ var problems string
+ rig.Run(chromedp.Evaluate(stripGeometry, &problems))
+ if problems != "" {
+ t.Errorf("at %dpx the strip overlaps itself:\n%s", width, problems)
+ }
+
+ // The resting menu trigger of an item that is neither current
+ // nor hovered nor focused is not drawn over its thumbnail.
+ var restingOpacity string
+ rig.Run(chromedp.Evaluate(`getComputedStyle(document.querySelector('[slides-strip-item]:not([data-current="true"]) [slides-strip-menu] > summary')).opacity`, &restingOpacity))
+ if restingOpacity != "0" {
+ t.Errorf("at %dpx a resting item's menu trigger has opacity %s, want 0 (shown on hover, focus or current only)", width, restingOpacity)
+ }
+ var currentOpacity string
+ rig.Run(chromedp.Evaluate(`getComputedStyle(document.querySelector('[slides-strip-item][data-current="true"] [slides-strip-menu] > summary')).opacity`, &currentOpacity))
+ if currentOpacity != "1" {
+ t.Errorf("at %dpx the current item's menu trigger has opacity %s, want 1", width, currentOpacity)
+ }
+
+ // "Current slide" is in the accessible name and not on screen.
+ var current struct {
+ Text string `json:"text"`
+ Width float64 `json:"width"`
+ Height float64 `json:"height"`
+ Upper string `json:"upper"`
+ }
+ rig.Run(chromedp.Evaluate(`(() => {
+ const el = document.querySelector('[slides-strip-item][data-current="true"] [slides-strip-current]')
+ const b = el.getBoundingClientRect()
+ return { text: el.textContent, width: b.width, height: b.height, upper: getComputedStyle(el).textTransform }
+ })()`, &current))
+ if current.Text == "" {
+ t.Errorf("at %dpx the current item lost its 'Current slide' text — a screen reader still needs it", width)
+ }
+ if current.Width > 1 || current.Height > 1 {
+ t.Errorf("at %dpx 'Current slide' is visible (%gx%g), want it for screen readers only", width, current.Width, current.Height)
+ }
+ if current.Upper == "uppercase" {
+ t.Errorf("at %dpx 'Current slide' is still uppercase", width)
+ }
+
+ // Hover and keyboard focus each show a resting item's trigger.
+ var at struct {
+ X float64 `json:"x"`
+ Y float64 `json:"y"`
+ }
+ rig.Run(chromedp.Evaluate(`(() => {
+ const li = document.querySelector('[slides-strip-item]:not([data-current="true"])')
+ li.scrollIntoView({ block: 'nearest', inline: 'nearest' })
+ const b = li.getBoundingClientRect()
+ return { x: b.left + b.width / 2, y: b.top + b.height / 2 }
+ })()`, &at))
+ var hoverOpacity string
+ rig.Run(
+ chromedp.MouseEvent(input.MouseMoved, at.X, at.Y),
+ chromedp.Sleep(300*time.Millisecond),
+ chromedp.Evaluate(`[...document.querySelectorAll('[slides-strip-item]:not([data-current="true"])')].filter(li => li.matches(':hover')).map(li => getComputedStyle(li.querySelector('[slides-strip-menu] > summary')).opacity).join(',')`, &hoverOpacity),
+ )
+ if hoverOpacity != "1" {
+ t.Errorf("at %dpx a hovered item's menu trigger has opacity %q, want 1", width, hoverOpacity)
+ }
+ var focusOpacity string
+ rig.Run(
+ chromedp.MouseEvent(input.MouseMoved, float64(width)-5, 895),
+ chromedp.Evaluate(`document.querySelectorAll('[slides-strip-item]:not([data-current="true"]) [slides-strip-link]')[1].focus()`, nil),
+ chromedp.Sleep(300*time.Millisecond),
+ chromedp.Evaluate(`getComputedStyle(document.activeElement.closest('[slides-strip-item]').querySelector('[slides-strip-menu] > summary')).opacity`, &focusOpacity),
+ )
+ if focusOpacity != "1" {
+ t.Errorf("at %dpx a focused item's menu trigger has opacity %q, want 1", width, focusOpacity)
+ }
+ rig.Screen(`[slides-strip-nav]`, fmt.Sprintf("the strip at %dpx", width))
+ }
+}