Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SWIK-2088 Fix reloading presentation mode starting on the wrong slide #1010

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rmeissn
Copy link
Member

@rmeissn rmeissn commented Sep 28, 2018

Prior to this PR, when reloading the page in presentation mode, the slideshow always jumped to the first slide instead of the last shown one. It will now correctly reopen the last shown slide.

EDIT: The following description is now implemented with localstorage instead of cookies. All other information are still valid.
Basically I'm setting a cookie (valid for 15 secs) if the page is reloaded and read this cookie again as soon as the page has finished reloading. If there is a slide hash in the cookie, I'll set this hash again and in result the correct slide is opened up. After reading the cookie, the cookie deleted.

This should theoretically work fine with PR #1008 .

This might still block SWIK-1775 - not sure yet.

@rmeissn rmeissn added the bugfix label Sep 28, 2018
@rmeissn rmeissn self-assigned this Sep 28, 2018
@rmeissn
Copy link
Member Author

rmeissn commented Sep 28, 2018

This might be much easier and less code if localstorage is used instead of a cookie.

@kprist kprist requested review from kprist and removed request for ali1k, abijames and kadevgraaf October 24, 2018 07:15
@kprist kprist assigned kprist and unassigned rmeissn Oct 24, 2018
kprist
kprist previously requested changes Oct 24, 2018
Copy link
Member

@kprist kprist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I go to presentation mode from a slide view page, then switch slides and reload, this works as expected.

However, the next time I go to presentation mode for another deck, the presentation does not start at the slide of focus, rather at the the first slide of the immediate deck parent (a regression of sorts). This doesn't always happen, the behaviour does not seem stable.

Furthermore, I expect that if I reload the page after removing the hash, the presentation would start at the slide of the path rather something we saved in local storage. In other words, it's not clear why we need local storage for this.

@rmeissn
Copy link
Member Author

rmeissn commented Oct 24, 2018

@kprist This branch is not using a cookie anymore. Just localstorage to store the last shown slide. It's less code and more convenient than a cookie.

Explanation of behaviour: If you request presentation mode with a fragment (hash) (!) in the URL, this fragment is not transferred to the server, rendering and preloading the page. This is the default browser behaviour. Thus reveal.js is initialized without a fragment and will always start at the first slide of a deck. This branch is saving the fragment in localstorage in order to restore it after a page reload (sole purpose of this branch!). It's not possible to start on a specific slide by manipulating the fragment of the URL (see following paragraph).
If a specific slide is opened fro view mode in presentation mode, the URL is different than when the slide is exchanged by reveal.js in presentation mode. In the first case we add a URL path segment (that is transferred to the server) and in the latter one reveal adds/exchanges the fragment of the URL (that is not transferred to the server).
As the fragment should eventually be available as soon as client side mounting is executed I've tested a lot of different methods to get it from the URL. Unfortunately I found no way to get it. Server side rendering seems to rewrite this somehow by initializing reveal.js without a fragment.

It's not possible to do a page reload and to manually manipulate the URL fragment, as this is again triggering the server side rendering process.

I don't think your second paragraph is limited to this branch, or is it?

The current localstorage entry time to live is set to 15 seconds (to also support slow internet connections or large decks). If you reopen presentation mode for the same deck within these 15 seconds, it will show the last shown slide again, even though you e.g. wanted to open presentation mode from deck view (so to open the first slide).

I'm happy about any suggestions to improve the behaviour (without rewriting how we set up reveal.js).

@kprist
Copy link
Member

kprist commented Oct 24, 2018

Thanks for the clarification and the info about the time limit. Assigning to @kadevgraaf, as I don't think it should be merged now, seems like a bug with reveal.js.

@kprist kprist dismissed their stale review October 24, 2018 08:59

No changes can fix this issue

@kprist kprist assigned kadevgraaf and unassigned kprist Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants