Repository navigation
fix(sheets): pin and un-pin the page in the same commit as the sheet, and never scroll for a sheet's own history pop - #257
Merged
Conversation
…tory pop Reported from a phone, mid-session. Picking an RIR level, one tap that closes the picker sheet and ticks the set, made the page "jump down to the bottom and back up to the exercise". A plain tick did not, so it was the sheet. Modals pins the body behind a sheet (position:fixed; top:-y) and puts the scroll back when the sheet goes; both ran in a passive effect, after paint. iOS applies scrolls asynchronously, so it showed a frame un-pinned at scroll 0 before the restore, with the 350 ms keyboard retry pulling it back. The pin is a layout effect now, so the un-pin and the restore land in the same frame as the sheet leaving, and the restore is said once more on the next frame. App.jsx's back-navigation scroll restore also ignores a POP that lands on the same screen. That is a sheet's history entry going away (Modals pushes one per sheet for the system back button), not a page change, and the recorded position a frame later only added a scroll of its own. Desktop WebKit paints the un-pin and the restore together, so it does not show the symptom; the test pins the order instead: restore at once, on the next frame, and after the keyboard's animation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
kurktchiev
force-pushed
the
gh/sheet-close
branch
from
September 21, 2026 19:26
90eaadb to
71e1603
Compare
Owner
|
Thanks @kurktchiev, no more jump on iOS when a sheet closes. Released in v1.3.9: https://github.com/DuarteSantos8/openGym/releases/tag/v1.3.9 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One commit. Two small changes that have to go together, because each on its own only moves the jump
around.
What breaks
Reported from a phone, mid-session. Picking an RIR level, one tap that closes the picker sheet and
ticks the set, made the page "jump down to the bottom and back up to the exercise". A plain tick did
not, so it was the sheet.
Modalspins the body behind a sheet (position: fixed; top: -y) and puts the scroll back when thesheet goes. Both ran in a passive effect, after paint. iOS applies scrolls asynchronously, so there
was a frame with the body un-pinned at scroll 0 before the restore, and the 350 ms keyboard retry then
pulled the page back up. That is the jump, and back.
App.jsxmade it worse from the other side. Its back-navigation scroll restore fires on every POP,and
Modalspushes a history entry per sheet so the system back button closes it. Closing a sheet istherefore a POP that lands on the screen you are already on, and the restore read the recorded
position a frame later and added a scroll of its own on top of the sheet's.
Reproduce
On iOS, scrolled some way down the workout screen, tick a set with effort ratings on. The picker
opens, you pick, and the page jumps to the bottom and comes back.
What changed
useLayoutEffect, so the un-pin and the restore land in the same frame as the sheetleaving. There is no frame at scroll 0 to see.
pinned, because iOS can apply a scroll issued in the same task a frame late or against the
still-short layout.
App.jsxignores a POP that lands on the same path. That is a sheet's history entry going away, nota page change, and it has no scroll of its own to restore.
The 350 ms keyboard retry that was already there stays; a sheet closed with the keyboard still up (tap
"+" in the picker, then finish) makes iOS scroll the page again after everything above has run.
Tests
Modals.test.jsx, one case: the body is pinned on open with the scroll position folded intotop,and on close the scroll is restored three times over, in order. At once, in the same commit; again on
the next animation frame; and again after the keyboard's dismiss animation. Against
mainit fails onthe second of those, because the passive effect only ever issued one.
Desktop WebKit paints the un-pin and the restore together, so it does not show the symptom. The test
pins the order rather than the pixels.
Checklist
npx vitest runpasses infrontend/: 1585 passing across 126 files (mainis 1584; +1)npm testinapi/untouched by this PRCHANGELOG.mdis left alone🤖 Generated with Claude Code