Skip to content

fix(workout): rating a superset set scrolls to the partner, like a tick - #347

Merged
DuarteSantos8 merged 1 commit into
DuarteSantos8:mainfrom
kurktchiev:gh/superset-rating-scroll
Oct 7, 2026
Merged

DuarteSantos8 merged 1 commit into
DuarteSantos8:mainfrom
kurktchiev:gh/superset-rating-scroll

Conversation

@kurktchiev

Copy link
Copy Markdown
Contributor

In the Cards layout a superset is one card. Ticking a set that hands the marker to the partner centres the partner's next row. Rating the set instead, through the RIR or RPE picker, ticks it and moves the marker the same way, but the screen stays where it was and the partner's row can sit below the fold.

The pick closes the sheet in the same tap. Modals.jsx un-pins the body and puts the page back where it stood, once in the cleanup and again on the next frame. Workout.jsx's el.scrollIntoView(...) runs between those two, and the requestAnimationFrame(() => window.scrollTo(0, y)) undoes it. RIR and RPE share the picker, so both scales behave the same.

Modals.jsx now tracks how long its restore sequence runs and exports afterScrollRestore(fn). With no sheet closing, fn runs at once, so a plain tick behaves as before. Otherwise the last re-assert calls fn itself, right after its own scrollTo. It has no timer of its own, because two ~350 ms timers armed a few milliseconds apart can fire in either order. That re-assert ends the window when it belongs to the latest close, counted rather than read off the clock. With privacy.resistFingerprinting rounding Date.now() to 100 ms, a deadline can still look a few ms away in that callback, and a waiter would be stranded. Only a real close arms the window, so StrictMode's effect, cleanup, effect on mount does not delay anything in dev. The card's scroll goes through it and reads the row off the latest render when it fires, since every store write clones the state and the row refs are keyed by entry object.

Deferring the scroll changes the other runs of the same effect, the ones the marker did not cause: mount, pairing two exercises, List to Cards, and moving a unit up or down. They asked for the scroll too, but each goes through a sheet (the weigh-in, the exercise menu, the Layout menu) whose restore undid it, so they looked like no-ops. Left to land after the wait, a session started through the weigh-in sheet would open with its header scrolled away. So now:

  • a run the marker did not cause is not asked for at all while a restore is pending (scrollRestorePending());
  • otherwise it scrolls only a row that is off screen, judged against the tab bar and the rest or hold bar at the bottom, and the status bar inset and the connection bar at the top. Coming back to a session with the row below the fold still brings it back;
  • a move is judged by index, and moveUnitAt records the marker's new index itself, so Move up is not a move, while the tick between two rows of the same exercise in one superset still is.

The top is measured off an element sized by calc(var(--sat) + var(--conn)). Reading --sat itself gives text. With --native-sat: 47px, getPropertyValue('--sat') returns "max(0px,47px)" in both WebKit and Chromium (checked with Playwright), and parseFloat makes that NaN.

Checked in Playwright's WebKit 26.6 with iPhone emulation, guest session, old and new build served by the web image's nginx:

old new
RIR or RPE pick on the first set, iPhone 17 Pro Max (763 px tall) partner row top at 831, below the fold 352, centred
same, iPhone 16e (651 px) / iPhone 17 (681 px) 775 / 790 296 / 311
round finished by a rating, marker back to the first member row at -49, above the screen 352
per-side set rated L then R 886 352
a second sheet opened 120 ms after the pick 831 352
picker dismissed by the backdrop after nudging the exact value 831 352
same-exercise superset rated 831 352

Starting a routine through the weigh-in sheet and without it, pairing from the ⋯ menu, and Move up left the row where it was on both builds, and the new build asked for no row scroll in any of them. Tab return, a fresh open and a swipe landed the same on both.

Tests:

  • Workout.superset-scroll.test.jsx, new: 15 tests. Rating on both scales with a store write during the wait, the plain tick, unpairing during the wait, the mount/pairing/layout/move cases, the same-exercise superset, and the three bars. 11 fail on the old code.
  • Workout.sheet-scroll.test.jsx, new: the real Workout, Modals, store and effort picker together, with fake timers. On the old code the card's scroll lands between the two restores. With the fix the order is restore, restore, then the scroll to the partner's row, and nothing undoes it. The plain tick case passes on both.
  • Modals.test.jsx: six new tests for the hand-over order (with and without the keyboard), a second sheet closing inside the window, a clock reading 100 ms behind, the immediate path, and StrictMode. All six fail on the old code.

Those three files plus Workout.test.jsx: 157 tests pass. The other nine Workout.*.test.jsx files, Stats.recovery.test.jsx and sheets.effort-picker.test.jsx: 89 pass.

#241 rewrites the same un-pin cleanup in Modals.jsx. It takes the restore position from body.style.top and cancels the 350 ms re-assert on a workout-scroll-anchor event. It already conflicts with main there, and whichever lands second has to keep the hand-over in that timer's callback. #222 edits moveUnitAt a few lines from the one line this adds and merges cleanly with it.

🤖 Generated with Claude Code

In the Cards layout a superset is one card, and ticking a set that moves
the marker to the partner centres the partner's next row. Rating the set
instead (the RIR/RPE picker) ticks it and moves the marker all the same,
but the screen stays put and the partner's row can sit below the fold.
The pick closes the sheet in the same tap, and the sheet's un-pin puts
the page back where it stood, at once and again on the next frame. The
card's scrollIntoView ran between the two and the second restore undid
it.

Modals.jsx now says how long its restore sequence lasts and offers
afterScrollRestore(fn): with no sheet closing fn runs at once, so a plain
tick is unchanged; otherwise the last re-assert runs fn itself, right
after its own scrollTo. It has no timer of its own, because two ~350 ms
timers armed milliseconds apart run in either order, and the latest
close's re-assert ends the window by counting closes rather than reading
the clock, which a coarsened Date.now() can make look early. Only a real
close arms the window, so StrictMode's effect-cleanup-effect on mount
does not.
The card's scroll goes through it and reads the row from the latest
render when it fires.

Waiting for the restore changes the runs of the same effect that the
marker did not cause: mount, pairing two exercises, List -> Cards and
moving a unit. Those used to ask for the scroll too and a closing sheet
(the weigh-in, the exercise menu, the Layout menu) undid it, so they
looked like no-ops. Now they are not asked for while a restore is
pending, and otherwise scroll only a row that is off screen, judged
against the fixed bars at the bottom and, at the top, the status bar
inset and the connection bar. A move
is judged by index, and moveUnitAt records the marker's new index itself,
so a unit moved up is not a move and a superset of two rows of the same
exercise still is.

The top is measured off an element sized by calc(var(--sat) +
var(--conn)): --sat itself computes to text like "max(0px,47px)" in
WebKit and Chromium, which parseFloat cannot read.

Workout.superset-scroll.test.jsx (15 tests, 11 fail on the old code),
Workout.sheet-scroll.test.jsx (the real Workout, Modals, store and
picker together; the rating case fails on the old code) and six new
Modals tests, all failing on the old code. In Playwright's WebKit with
iPhone emulation a rating now centres the partner's row (was left below
the fold) and starting, pairing and Move up still leave the page still.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@DuarteSantos8
DuarteSantos8 merged commit 7d249d3 into DuarteSantos8:main Oct 7, 2026
3 of 4 checks passed
@DuarteSantos8

Copy link
Copy Markdown
Owner

Merged and released in v1.3.10, thanks @kurktchiev!

https://github.com/DuarteSantos8/openGym/releases/tag/v1.3.10

@kurktchiev
kurktchiev deleted the gh/superset-rating-scroll branch October 9, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants