Repository navigation
fix(client): sync every window that moves, without echoing followed positions - #2805
Open
CurlyPeter wants to merge 1 commit into
Open
CurlyPeter wants to merge 1 commit into
CurlyPeter wants to merge 1 commit into
Conversation
…ositions - skip incoming updates by the sender's id instead of its type, so a presenter view follows another presenter view (and a viewer on a trusted origin another such viewer) - do not write the shared state back after a navigation that only followed it, or a failed one: on a slow connection that write could arrive after a newer move and send the other windows back - compare an update with a follow still under way, not only with the current route, so a quick move back is not taken as already done - a window that loads joins the shared position instead of moving everyone to the slide in its URL (comparing the URL's clicks, since the clicks context reads 0 before the slide mounts); without a shared position yet, or without a server, its own position is shared as before
✅ Deploy Preview for slidev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This branch has not been deployed
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.
Description
When more than one window drives a presentation, the navigation sync goes wrong in a few ways. Typical setups: a presenter view on a laptop plus a second presenter view on a tablet, a presenter view plus an audience window on
localhost, either of them over a slow connection.onPatchskips updates whoselastUpdate.typeequals the receiver's own type, so a move in one presenter view is not followed by the other presenter views (likewise for viewers on a trusted origin).updateSharedStateruns after every navigation, including the oneonPatchstarts to follow an update, so every follower sends the position it just received under its ownlastUpdate. On a slow connection such a write can reach the server after a newer move, and the other windows jump back.updateSharedStatetoo, so a window that loads at an older URL (a reload after a lost connection, a restored tab, a presenter view opened at/presenter/) moves the whole presentation to that slide.This PR, all in
packages/client/setup/root.ts:onPatchskips only the window's own updates (lastUpdate.id).page: 0), or without a server, the loading window's position is shared as before.Viewers on untrusted origins are unchanged: they never send.
Behaviour change to note: with a server that already holds a position, opening a URL with a slide number (
/12,/presenter/5) now joins the current slide instead of moving everyone there; navigating afterwards works as before. If you prefer to honour an explicitly opened URL, I can limit joining to reloads and to the redirect from/.Verification
A Playwright script against a dev server: two presenter views and two viewers on a trusted origin, each in its own browser context, later a fifth presenter view whose sync requests are delayed by 150 ms. Every window's
navPOSTs are recorded.lastUpdate. Withmain'ssetup/root.ts, two or three followers wrote the position back on every move.main's version, they moved everyone back.Measured on v53.0.0; the change applies unchanged to
main.eslinton the changed file andvue-tsc --noEmitpass onmain.vitestshows the same 11 failures as onmain(API snapshots and the syntax integration test, unrelated).Related: #2611 / #2612 are about coordinating several presenters (who may drive). This PR adds no such concept; it makes the existing "everyone follows the latest move" consistent.