From c37a6d02997f5c8dfd992b33391a572e4e303a4a Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 12 Jun 2026 10:18:12 +0100 Subject: [PATCH 1/2] fix(pad): show saved-revision markers in in-pad history mode (#7946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When #7659 moved the timeslider into the pad as an embedded iframe, the user-facing control became the outer #history-slider-input range input. The saved-revision stars are still drawn by broadcast_slider.ts into the iframe's #ui-slider-bar, but that DOM is hidden in embed mode, and pad_mode.ts bridged rev/max/value/timer/authors to the outer slider but never the saved revisions. Result: clicking "Save Revision" appeared to work but no markers showed in the timeslider (regression in 3.3.x). Bridge the embedded timeslider's clientVars.savedRevisions onto the outer slider as percentage-positioned star markers, rendered on first sync and re-rendered when the slider max changes. Markers are a purely visual, aria-hidden overlay (keyboard/SR users already reach any revision via the slider + step buttons) with a click-to-seek convenience for mouse users. Adds a frontend-new Playwright spec exercising the real user flow (save a revision in the pad, enter in-pad history mode, assert a visible marker on the outer slider). The only prior coverage lived in the legacy mocha suite (src/tests/frontend/specs/timeslider_revisions.js) which no CI workflow runs, and the modern timeslider specs drive the ?embed=1 iframe directly and never exercise the outer history UI — so this regression was invisible to CI. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/css/pad.css | 44 +++++++++++++- src/static/js/pad_mode.ts | 58 +++++++++++++++++++ src/templates/pad.html | 11 +++- .../specs/timeslider_saved_revisions.spec.ts | 58 +++++++++++++++++++ 4 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 src/tests/frontend-new/specs/timeslider_saved_revisions.spec.ts diff --git a/src/static/css/pad.css b/src/static/css/pad.css index e2bc4c89e68..fb43a115455 100644 --- a/src/static/css/pad.css +++ b/src/static/css/pad.css @@ -214,12 +214,52 @@ body.history-mode #history-controls { display: flex; } .history-controls button.buttonicon.buttonicon-play.pause::before { content: "\e829"; } -.history-slider-input { +.history-slider-wrap { + position: relative; flex: 1 1 auto; min-width: 80px; margin: 0 6px; + display: flex; + align-items: center; +} +.history-slider-input { + flex: 1 1 auto; + min-width: 0; + width: 100%; cursor: pointer; } +/* Saved-revision markers overlaid on the slider track (issue #7946). Inset + * left/right by ~half the native range thumb so a marker lines up with the + * thumb centre at the track extremes. pointer-events:none on the layer keeps + * slider dragging unobstructed; the stars themselves re-enable clicks to seek. */ +.history-slider-stars { + position: absolute; + left: 8px; + right: 8px; + top: 0; + bottom: 0; + pointer-events: none; +} +.history-slider-stars .history-star { + position: absolute; + top: 50%; + transform: translate(-50%, -50%); + width: 16px; + height: 16px; + padding: 0; + margin: 0; + border: 0; + background: none; + line-height: 1; + cursor: pointer; + pointer-events: auto; +} +.history-slider-stars .history-star::before { + font-family: fontawesome-etherpad; + content: "\e856"; + color: #da9700; + font-size: 14px; +} .history-timer { flex: 0 0 auto; font-size: 12px; @@ -282,7 +322,7 @@ body.history-mode #history-controls { display: flex; } @media (max-width: 800px) { .history-controls { padding: 0 6px; gap: 4px; min-height: 36px; } .history-controls button.buttonicon { padding: 4px 6px; min-width: 32px; } - .history-slider-input { min-width: 60px; margin: 0 2px; } + .history-slider-wrap { min-width: 60px; margin: 0 2px; } } @media (max-width: 480px) { .history-controls #history-leftstep, diff --git a/src/static/js/pad_mode.ts b/src/static/js/pad_mode.ts index 499d9832b80..93ba68c9ac6 100644 --- a/src/static/js/pad_mode.ts +++ b/src/static/js/pad_mode.ts @@ -194,6 +194,11 @@ class PadModeController { } this.revLabel.textContent = ''; this.dateLabel.textContent = ''; + const stars = document.getElementById('history-slider-stars'); + if (stars) { + stars.replaceChildren(); + stars.dataset.sig = ''; + } } // Restore everything entry-time we stashed: chat message visibility, the @@ -374,6 +379,10 @@ class PadModeController { playBtn.classList.toggle('pause', playing); playBtn.setAttribute('aria-pressed', playing ? 'true' : 'false'); } + // Saved-revision markers depend on the slider max, which is only known + // once the inner slider has reported its length — render them here so we + // pick up the correct positions on first sync and on any max change. + this.renderSavedRevisionStars(innerWin); }; // The hook registered earlier in attachInnerBridges already calls // onRevChange — piggyback on it for slider input/timer updates by @@ -390,6 +399,55 @@ class PadModeController { registerSync(); } + // Mirror the embedded timeslider's saved revisions onto the outer slider as + // clickable star markers (issue #7946). The inner slider draws its own stars + // on #ui-slider-bar, but that DOM is hidden in embed mode, so users only see + // the outer #history-slider-input — which had no markers. We read the + // authoritative list from the iframe's clientVars and position each star by + // revNum/max. A signature guard keeps this cheap when sync() fires on every + // scrub; positions are percentage-based so they reflow on resize for free. + private renderSavedRevisionStars(innerWin: Window): void { + const inner: any = innerWin as any; + const layer = document.getElementById('history-slider-stars'); + const sliderInput = document.getElementById('history-slider-input') as HTMLInputElement | null; + if (!layer || !sliderInput) return; + + const saved = inner.clientVars?.savedRevisions; + const max = Number(sliderInput.max) || 0; + if (!Array.isArray(saved) || saved.length === 0 || max <= 0) { + if (layer.childElementCount) layer.replaceChildren(); + layer.dataset.sig = ''; + return; + } + + const inRange = saved + .map((r: any) => Number(r && r.revNum)) + .filter((n: number) => Number.isFinite(n) && n >= 0 && n <= max); + const sig = `${max}:${[...inRange].sort((a, b) => a - b).join(',')}`; + if (layer.dataset.sig === sig) return; + layer.dataset.sig = sig; + layer.replaceChildren(); + + for (const rev of saved) { + const revNum = Number(rev && rev.revNum); + if (!Number.isFinite(revNum) || revNum < 0 || revNum > max) continue; + const frac = max === 0 ? 0 : revNum / max; + // A purely visual marker (the layer is aria-hidden): keyboard/screen + // reader users already reach any revision via the slider and step + // buttons, so we mirror the legacy timeslider's mouse-only stars rather + // than inject extra tab stops. The hover title aids mouse users; the + // click is a convenience to jump straight to the saved point. + const star = document.createElement('span'); + star.className = 'history-star'; + star.style.left = `${(frac * 100).toFixed(4)}%`; + star.title = (rev && typeof rev.label === 'string' && rev.label) || `Revision ${revNum}`; + star.addEventListener('click', () => { + try { inner.BroadcastSlider?.setSliderPosition?.(revNum); } catch (_e) { /* inner gone */ } + }); + layer.appendChild(star); + } + } + // Capture the live state we'll restore on exit: live chat message // visibility (just the timestamps — actual messages stay), live users // panel HTML, and current Export hrefs. diff --git a/src/templates/pad.html b/src/templates/pad.html index 6bbc359c6ef..ce176a8e33e 100644 --- a/src/templates/pad.html +++ b/src/templates/pad.html @@ -159,8 +159,15 @@ - + + + + +