fix browser history #552
Merged
fix browser history #552
Conversation
This file contains 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
|
Is it intended that the patches still have a WIP: subject? |
rschwebel
reviewed
Mar 19, 2024
|
Hi @rschwebel! As you correctly pointed out, the patches are not cleaned up yet and won't be merged in this state. I wanted some early feedback from @m-reichle if my changes fix her problem (or I suppose yours, for that matter :) ). Also I wanted to check wether the test works in CI. While working on this, I had a lot of trouble running the browser tests locally. All calls to Playwrights |
The naming in the test modules is used to enforce a specific order of the tests `tests/test_0305_requests.py` clashed with `tests/test_0305_events.py`. Signed-off-by: Florian Scherf <mail@florianscherf.de>
Signed-off-by: Florian Scherf <mail@florianscherf.de>
The browser history of the client never *really* worked. When you were redirected to a Lona app from an external site, you couldn't go back by clicking on the back button. When you were three links into an Lona app and refreshed the tab, the history before the refresh was unreachable. Previously, both client1 and client2 created a new history entry, using `history.pushState`, when a new view started, regardless the reason of the view starting. So initial setup, refreshes, and reconnects also created history items. This is not correct! `history.pushState` should only be called when you want to *extend* the browser history, beyond the normal history behaviour. This patch limits all calls to `history.pushState` to cases of actual in-app navigation, and adds a test to check the now corrected behaviour. Signed-off-by: Florian Scherf <mail@florianscherf.de>
fscherf
force-pushed
the
fscherf/fix-browser-history
branch
from
99f210b
to
c726709
Compare
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.
Fixes #550