fix(text): keep motion-ticker in place across a hover pause - #3
fix(text): keep motion-ticker in place across a hover pause#3felixchen-wordup wants to merge 4 commits into
Conversation
pause-on-hover made the marquee snap back to its starting position on mouse leave, and stopped it dead instead of decelerating (tgomilar#2). onEnter paused the animation before lerpRate(0) ran, so the ramp rendered nothing and the deceleration was never visible. The ramp then drove ctrls.speed down to ~0.003 against the already-paused animation. On resume, MainThreadAnimation.play() rebases startTime as `now - holdTime` while tick() reads back `(timestamp - startTime) * speed`, so the elapsed time is multiplied by the current speed — at 0.003 that collapses ~0.7s of progress to ~0.002s, restarting the loop. Let the ramp own the stop: onEnter only calls lerpRate(0), which pauses through the playback controller once the rate reaches MIN_RATE, so the animation stays live while it decelerates and its time stays meaningful. Resuming now restores ctrls.time explicitly after play(), the same way onResize() already does, and the rate never goes below MIN_RATE. The keyboard path had the inverted order too — there pause() cancelled the rate ramp outright, so Space stopped the ticker dead. It now shares the same path as hover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tgomilar
left a comment
There was a problem hiding this comment.
Necessary improvement: attributeChangedCallback must respect the paused state
When the ticker is stopped on hover and an attribute such as speed changes, attributeChangedCallback (lines 153-165) builds a new animation that runs at full speed. The ticker scrolls again while the component still reports paused. This is the same hover path this PR fixes, so I think it belongs in this PR.
Fix: after rebuilding, set the speed to currentRate and call pause() when the rate is zero, in the same way as onResize (line 288), which already sets the speed.
| * Long enough for the rate ramp to bottom out, so a resume happens from a | ||
| * fully stopped ticker rather than from one still coasting near full speed. | ||
| */ | ||
| const HOVER_DWELL = 900 |
There was a problem hiding this comment.
The fixed 900ms wait for the speed ramp to finish can fail on a slow machine. Use the until helper (lines 30-36) to wait until playState becomes paused instead of a fixed time.
A live attribute change or resize rebuilt the ticker animation running at full speed, so a hover-paused ticker started scrolling while still reporting 'paused'. Rebuilds from both paths now go through one rebuildMarquee() that floors ctrls.speed at MIN_RATE, holds the fresh animation when the controller is paused, flushes motion's async keyframe resolver so the carried-over time survives the first frame, and derives progress from the outgoing animation's own duration so the position no longer scales with a speed change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The gap styles were written once in build(), so changing the gap attribute only altered the animation math while the rendered columnGap and marginRight kept their original values — and the wrap width was measured against the new gap, leaving the loop seam off by the delta. Gap styling now lives in one applyGap() that build, startMarquee and rebuildMarquee all run before measuring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d fill intact Four defects on the same rebuild path, found by auditing every observed attribute against every playback state: - The wave loop captured speed, direction, stride and item positions at start and was never refreshed, so any live attribute change or resize desynced the wave from the scroll. Geometry now lives in refreshWave(), run on every rebuild; while paused it only re-measures and the resume path restarts the loop. - A direction flip mapped the progress fraction onto mirrored keyframes and teleported the track. The rebuild now derives its time from the rendered offset, which holds the position under any change of duration or direction. - A pointer or focus leaving resumed a ticker the user had paused with Space, and left the flag inverted so the next press did nothing. onLeave now respects the keyboard pause. - fillSet() only ran at build, so a container that grew later was left with a gap after the duplicated sets. Rebuilds now top the track up, and fillSet skips the setB rebuild when nothing changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Done — and you were right that it belongs in this PR, thanks. The fix itself is in a40a93d, but the branch has grown by three commits since your review, so here is an honest account of why each change is there. The fix you asked for (a40a93d)
Why two more commits followedYour comment demonstrated that the rebuild path had never been audited as a whole — you found a second consumer of the same flaw one day after I fixed the first. Rather than wait for the third report, I swept every observed attribute against every playback state (running, decelerating, hover-paused, keyboard-paused, externally paused, detached) and fixed what fell out. Everything below reproduces on 245b17c — 71b6ceb — four more on the same path:
VerificationSeven more tests added in these commits, all driving the public surface and asserting on the rendered Two smaller findings from the same audit were left out as they change documented behaviour rather than fix broken behaviour: the four read-once attributes sit in |
Fixes #2.
<motion-ticker>withpause-on-hover(the default) snapped the track back tox: 0on mouse leave, and stopped dead on hover instead of decelerating. Both come from the same place.Root cause
onEnterpaused before ramping:No deceleration.
this.pause()→handle.pause()→ctrls.pause()happens first, so thelerpRate(0)ramp that follows runs against an already-paused animation and renders nothing.The jump. The ramp still keeps assigning
ctrls.speed = currentRatedown to the< 0.003threshold. Resuming from that speed is what loses the position —MainThreadAnimation.play()rebases aswhile
tick()reads it back asso the resumed time comes out multiplied by the current speed. At
speed ≈ 0.003that turns ~0.7 s of progress into ~0.002 s, i.e. the first frame ofx: [0, -w].Measured on the fixture before the fix — the track is frozen through the whole hover, then snaps:
The hover fix (ca20f57)
onEnterno longer pauses up front. It just callslerpRate(0), and the ramp pauses through the playback controller once the rate reachesMIN_RATE. The animation stays live while it decelerates, so the ramp is visible andtimekeeps meaning something.playStatestill ends up'paused', just at the end of the ramp rather than at the start — which also seems more truthful, since it is genuinely still running while it slows down.resumeCtrls()restorestimearoundplay(), the same way the resize path does. This is what actually guarantees the position survives, independent of whatplay()does tostartTime.ctrls.speedis floored atMIN_RATE(0.05) rather than being driven toward 0, so the degenerate range is never entered in the first place.pausedfield is gone —playStatewas already tracking the same thing, and keeping both in sync was what made the two handlers hard to follow.The keyboard path (Space / Enter) had the inverted order too, with a different symptom: there
lerpRate(0)ran first andthis.pause()then cancelledrateRafoutright, so the ramp never got a frame and Space stopped the ticker dead. It now goes through the same path as hover.Review follow-up: the rebuild path (a40a93d, 245b17c, 71b6ceb)
The review pointed out that
attributeChangedCallbackrebuilds the animation at full speed while the component still reportspaused. Auditing that path as a whole (every observed attribute against every playback state) surfaced a family of defects, all reproducible onmain:onResizehad the same flaw asattributeChangedCallbackin a different costume (it wrotectrls.speed = 0while hover-paused, which makestimeread back 0 and loses the position on resume). Both callbacks now share onerebuildMarquee()that floors the speed atMIN_RATE, holds the fresh animation directly viactrls.pause()(the controller's ownpause()is a no-op when already paused), and readsctrls.durationonce before assigningtimeso motion's async keyframe resolver is flushed — without that, the next frame'splay()restarts the rebuilt animation from 0.gapwasn't actually live: its styles were written once inbuild(), so a live change altered the animation maths but not the rendering, and the loop seam drifted by the delta. OneapplyGap()now runs before every measurement.[0,-w]vs[-w,0]. Rebuilds now derive their time from the rendered offset, which holds the on-screen position under any change of duration or direction.startWave()captured speed, direction, stride and item positions in its closure and nothing refreshed them. Geometry now lives inrefreshWave(), run on every rebuild; while paused it only re-measures and the resume path picks the fresh values up.onLeaveresumed unconditionally and left thekeyboardPausedflag inverted, so Space appeared dead on its next press.onLeavenow respects the keyboard pause, keeping the aria-label's "Press Space to pause" promise.fillSet()only ran at build, so a container that grew later was left with a hole after the duplicated sets. Rebuilds now top the track up.Tests
Twelve added to
motion-ticker.test.ts, all driving the public surface and asserting on the renderedtranslateX/translateY. Each was mutation-checked — reverting the source while keeping the tests makes exactly the corresponding tests fail:keeps scrolling while it decelerates on hoverholds its position for as long as the pointer staysresumes from where it stopped when the pointer leavesdecelerates and resumes in place when toggled by keyboarddoes not pause on hover when pause-on-hover is falsestays parked in place when a live attribute changes while hover-pausedscrolls on without a jump across a live attribute change while runningre-applies the gap to the track when the attribute changeskeeps its rendered position when direction flipskeeps a keyboard pause across a pointer visittops the track back up when the container growsre-times the wave when speed changesFull gate is green:
typecheck,lint,format:check,check:preload,test(198 passed / 27 files),build,check:exports,size(motion-ticker 2.52 kB of a 3 kB budget).Known leftovers, deliberately out of scope
Two smaller findings from the same audit change documented behaviour rather than fix broken behaviour, so they are not in this PR: the four read-once attributes (
pause-on-hover,wave,wave-amplitude,wave-length) sit inobservedAttributesand trigger pointless rebuilds, andstartMarqueeretries on rAF unboundedly while the element has no width. Happy to file issues for either.🤖 Generated with Claude Code