Fix browser navigation not working between /home, /login, /register, etc #2383
Conversation
| onLoginClick() { | ||
| onLoginClick(ev) { | ||
| ev.preventDefault(); | ||
| ev.stopPropagation(); |
There was a problem hiding this comment.
Is preventDefault enough by itself, or do you really need stopPropagation also?
There was a problem hiding this comment.
preventDefault is enough for our use case, although we manage the onClick handling and should prevent propagation as well to be safe.
|
I'm not sure what I broke, but 0907775 breaks the build presumably due to a test failure (which test? who knows) and 9f8d5ba fixes it. Also, I spent a bunch of time chasing a red herring because our build process has a failure in it despite the green status: element-hq/element-web#7985 |
All of the anchors were pointed at `#` which, when clicked, would trigger a hash change in the browser. This change races the change made by the screen handling where the screen handling ends up losing. Because the hash is then tracked as empty rather than `#/login` (for example), the state machine considers future changes as no-ops and doesn't do anything with them. By using `preventDefault` and `stopPropagation` on the anchor click events, we prevent the browser from automatically going to an empty hash, which then means the screen handling isn't racing the browser, and the hash change state machine doesn't no-op. After applying that fix, going between pages worked great unless you were going from /login to /home. This is because the MatrixChat state machine was now out of sync (a `view` of `LOGIN` but a `page` of `HomePage` - an invalid state). All we have to do here is ensure the right view is used when navigating to the homepage. Fixes element-hq/element-web#4061 Note: the concerns in 4061 about logging out upon entering the view appear to have been solved. Navigating to the login page doesn't obliterate your session, at least in my testing.
turt2live
force-pushed
the
travis/back-button
branch
from
9f8d5ba to
07e8374
Compare
|
I was curious about what actually failed in the earlier builds such as this one. I guess I missed when looking at the build before that bottom of the log says "This log is too long to be displayed. Please reduce the verbosity of your build or download the raw log." At the end of that raw log, we see that There are also various messages about Olm apparently not being installed throughout the log. This seems like a normal warning (since Olm is optional and not explicitly installed) which is only appearing because of the test failures, which triggered verbose browser logs to print. I was able to replicate the test failures locally using your "take out setState" commit (b7c39d8) by running: in The failing tests seem to be checking whether the |
turt2live
force-pushed
the
travis/back-button
branch
from
07e8374 to
e519281
Compare
|
@jryans should good now, hopefully. Sorry for the forcepush - trying to avoid sticking revert commits into the PR. |
|
(I don't mind the force push personally... I think it's good to clean up the history where reasonable to do so.) |
…button"" This reverts commit 0978687.
…button"" This reverts commit 0978687.
All of the anchors were pointed at
#which, when clicked, would trigger a hash change in the browser. This change races the change made by the screen handling where the screen handling ends up losing. Because the hash is then tracked as empty rather than#/login(for example), the state machine considers future changes as no-ops and doesn't do anything with them.By using
preventDefaultandstopPropagationon the anchor click events, we prevent the browser from automatically going to an empty hash, which then means the screen handling isn't racing the browser, and the hash change state machine doesn't no-op.After applying that fix, going between pages worked great unless you were going from /login to /home. This is because the MatrixChat state machine was now out of sync (a
viewofLOGINbut apageofHomePage- an invalid state). All we have to do here is ensure the right view is used when navigating to the homepage.Fixes element-hq/element-web#4061
Note: the concerns in 4061 about logging out upon entering the view appear to have been solved. Navigating to the login page doesn't obliterate your session, at least in my testing.