Slideshow: should the browser back button behave the same as the X icon? [$5] #160

setnes · 2015-05-25T22:28:37Z

The browser's back button doesn't behave as expected. If clicking the big X in the upper right side of the screen exits the slide show, perhaps the back button should do the same thing. I feel a bit trapped in the slide show on a mobile device if the back button doesn't actually exit. I'm not sure what is correct here from a UI standpoint though. One could also make a case that back should just go to the previous image.

@oparoz's assessment of the situation

Files

How it works

Folder1->Folder2.
Slideshow is launched. Folder2 is renamed Folder2#slideshow, but it's the same position in the history
Pressing close rewrites the URL again, back to Folder2, but pressing back goes back to Folder1!

Pressing on "close":

keeps the position in the files list
"erases" the slideshow from history by rewriting the URL

Pressing on "back":

gives the impression of going two steps back since the slideshow doesn't modify the history.
loses both the position in the list and the folder from which the slideshow was launched

@oparoz's acceptance criteria

Files

Exact vertical position in Files is not lost when exiting the slideshow via the back button
Files app is not reloaded when exiting the slideshow via the back button. Blocked by hash in url in files app leads to root core#14828
Pressing back several times goes through the entire history
Pressing forward after exiting the slideshow reloads the last slide which was in view in the slideshow. Blocked by hash in url in files app leads to root core#14828

Photowall

Exact vertical position in Photowall is not lost when exiting the slideshow via the back button
Current album is not reloaded when exiting the slideshow via the back button
Reloading the app while browsing the slideshow should load the exact same slide
Pressing back several times goes through the entire history
Pressing forward after exiting the slideshow reloads the last slide which was in view in the slideshow

General

Everything above works both in the private and public spaces

Did you help close this issue? Go claim the $5 bounty on Bountysource.

setnes · 2015-05-29T23:09:56Z

I did a few more tests. It looks like the browser history is handled differently depending on if we start in the files view or the gallery view.

If I...

start a slideshow from the files view
flip through photos
click the browser back button

...I am taken back to the gallery thumbnail view. It's not the files view, but at least the back button takes me out of the slideshow.

If I...

start a slideshow from the gallery view
flip through photos
click the browser back button

...the URL changes to end with !. Another back button click changes to the URL to the previous photo, but nothing changes on the screen. If I keep clicking the back button, I can watch the URL change through photos (without loading them) until eventually I back out to the gallery thumbnail view.

setnes · 2015-05-29T23:30:32Z

I see galleryfileaction.js has a reference to history.replaceState inside a slideShow.onStop function. Perhaps we just need something similar in gallery.js. I might experiment with this to see if I can make it more consistent.

EDIT: Nope... bad idea. :)

oparoz · 2015-05-29T23:48:47Z

Hehe, interesting findings! Thanks for documenting that.
I was surprised by your first scenario :D

I didn't look at this side of things at all. I only focused on fixing the URLs so that the Files or thumbnail views were not reloaded and losing their current position.

I'm guessing that since the URL changes, we could make sure it updates the slideshow and when we reach the beginning we would quit and go back to the original view

The gallery side has the same fix as the Files side for going back to where we were when the back button is closes.

On the whole, I don't feel this is a priority since the X button is always present and we can navigate by either clicking on the buttons, using shortcuts or hopefully in the near future using swiping movements.

oparoz · 2015-05-29T23:50:55Z

I also want to add that every time you quit the slideshow, the history will have a hole in it. Nothing should happen when you have reach !, so in terms of usability, I don't think encouraging users to use the back button is the best thing, but if you have ideas on how to fix it, I'll listen :)

setnes · 2015-06-10T03:41:50Z

I want to add something else I've noticed to this issue thread. If you use the "f" key to get into fullscreen mode, you have to click the "X" icon twice to exit the slideshow. It's not entirely the same issue, but it's all related to exiting cleanly from the slideshow.

oparoz · 2015-06-11T19:38:59Z

OK. Will try to take that into consideration when trying to implement a fix

setnes · 2015-06-12T02:31:11Z

The following isn't a good fix for this because it causes a page reload whenever the browser back button is used or the X icon is clicked. It does demonstrate better back button behavior though. It also gets us out of fullscreen mode without a second click.

What is the best way to force the slideshow to close?

I put this in slideshowzoomablepreview.js right after the new "orientationchange resize" bind logic.

$(window).bind('popstate', function () {
        if (self.zoomable !== null) {
                location.reload();
        }
});

setnes · 2015-06-20T16:48:21Z

This issue was still bothering me, so I worked on it more. :) I have it so it no longer needs a page reload, and it still saves the position in the gallery.

I moved the popstate bind into gallery.js. It is at the end of Gallery.slideShow. The existing slideshow logic never pushes a new address into the history. It only ever replaces the address that is currently shown. This is what allowed my original reload hack to work. The URL after a browser back action was valid for a reload. Now, instead of a reload, I am stopping and hiding the slideshow only if zoomable is not null. I wanted to be sure that the gallery's onStop function got called. I was having problems getting that to work without this logic in the same scope as the onStop function (slideShow.stop calls this.onStop).

$(window).bind('popstate', function () {
    if ( slideShow.zoomablePreview.zoomable !== null ) {

        // Stop the slideshow.  This eventually causes onStop to fire (above).
        slideShow.stop();

        // Hide the slideshow.
        slideShow.container.hide();

        // Clean up some cruft.  (little unsure about this)
        slideShow.zoomablePreview.zoomable.dispose();
        slideShow.zoomablePreview.zoomable = null;
        slideShow.controls.active = false;

    };
});

I feel strongly about the back button needing to exit the slideshow based on my experience on mobile android devices. If you think this logic is decent I can create a pull request at some point (busy for the rest of the day). It is working good for me.

oparoz · 2015-06-21T00:16:51Z

setnes · 2015-06-21T18:38:14Z

I agree with most of that, but I think we should discuss one of those items.

Pressing back several times goes through the entire history.

I feel that pressing back while you are in the slideshow (no matter which image you are on) should take you back to the point where you entered the slideshow. Pressing forward from there should take you to the image you were last on in the slideshow. Once out of the slideshow, the back button should not be impacted at all by this change. You should still be able to back through gallery folders, etc, as you would normally expect without any funny business.

What I see in the code suggests that this was the original intent. The slideshow calls history.replaceState (NOT pushState) to change the URL displayed for each photo as we navigate through the photos. This does not actually change ANY history, it simply changes the current URL displayed as you flip through photos. If you click the back button from anywhere, the URL is correctly taking you back to where you entered the gallery. The only thing missing that we need to add is the part that actually stops and hides the slideshow and therefore displays the correct content for the URL in the address bar. This is all that I really thing needs to be added... everything else is already there and working.

Also, if you reload the page while watching the slideshow, everything works great because the URL displayed is the correct URL to reload the page with.

I originally thought the history was getting mangled, but I was actually just backing through multiple in-and-out-of-slideshow navigations contained in my history without actually seeing the running slideshow close. This is why my second comment on this issue is painting a confusing picture.

I really hope I'm making sense. :)

To simplify this, the back button should really just call slideShow.controls._stop(). This works great for me, but it is currently marked as a private function. Can we make that public, or make a public function that calls _stop?

I encountered another issue where calling slideShow.controls._stop() fixes everything. If the user clicks play instead of clicking through the slideshow manually, I was not resetting the slideshow timer and would be taken back into the slideshow.

oparoz · 2015-06-21T19:13:00Z

I'm all OK with this. That's what I meant by it.

An example navigation
F1->F2->S2'12-16->F2->G2->S2'1-4-G2->G1->S1'7-8->G1

Using the back button from the end
G1->S1'8->G1->G2->S2'4->G2->F2->S2'16->F2->F1

F=Files
G=Gallery
S=Slideshow
First number is current depth
2nd number is range of pictures in the slideshow

setnes · 2015-06-21T19:41:33Z

Ah... after I cracked your cryptic code, yes. I think we're on the same page. :) You don't want any holes shot in the history. The slideshow itself should leave a history entry based on the last image that was viewed, not ALL images that were viewed. I think that all sorta works today. My tweak will only fix the exit situation so the slideshow actually stops and hides when navigating back through the history. I have this working for gallery, but nothing has changed for files yet (haven't looked). Right now, a slideshow from files will exit the slideshow with the back button, but goes to a gallery view instead of files. That was a lower priority for me since the back button is sorta working.

I need to jump through some github hoops here to make my first change visible in a pull request. I think we'll need to refactor it though because of the call to a function labeled private.

oparoz · 2015-06-21T19:46:27Z

:)

Ah, yes, I forgot to reply regarding the method's visibility. It's fine to change it.

The only reason I even want to load the slideshow during history navigation is just to keep the code simple and the user experience consistent. I don't think it adds great value to load the slideshow you opened 30 minutes earlier.

oparoz · 2015-06-30T07:18:44Z

Current situation

Files

How it works

Folder1->Folder2.
Slideshow is launched. Folder2 is renamed Folder2#slideshow, but it's the same position in the history
Pressing close rewrites the URL again, back to Folder2, but pressing back goes back to Folder1!

Pressing on "close":

keeps the position in the files list
"erases" the slideshow from history by rewriting the URL

That's ideal as I've reconsidered keeping the slideshow in the history as I don't see the value in loading the last slide, especially when navigating through albums or folders.

Pressing on "back":

gives the impression of going two steps back since the slideshow doesn't modify the history.
loses both the position and the proper folder

In order to support the back button we can try the following:

Go history.forward() once we detect that we went back. Does not work The URL changes, but the Files app has already registered the new location. If we give a new window.location, it works, but it's horrible as the view will be loaded twice.
Add an extra state before loading the slideshow. Works, but the files list will be always be reloaded when the slideshow is closed since we're jumping back in history. We can't know in advance if the user is going to press "back" or "close" and we can't delete any state.

So, it seems we'll have to choose between:

Keep position in list, but back button does not work and
Back button works, but list is refreshed on exit

Not once have I thought about using the back button when using ownCloud, but that's because I'm always using it on a desktop.

setnes · 2015-06-30T13:19:21Z

My current pull request didn't address anything with the files interface. That is currently left as it always has been. It only changed the back button behavior from the gallery view. It also hasn't been refactored. It is simply calling a privately named slideShow.controls._stop(). What do you think of how it works from gallery view, not files view? If it works well, I can refactor by creating a slideShow.controls.back() or something similar that then calls _stop().

The behavior of things from the files view needs work, yes, but one thing at a time. :)

oparoz · 2015-06-30T16:23:26Z

Yes, I was too tired to make the same assessment on the Gallery side :D.
Your patch did work on the Gallery side, but I'm now also trying to get rid of the slideshow opening when going through history. One of the reason being that the Files app resets itself when navigating to a URL which contains the hashtag (owncloud/core#14828).
I've moved your patch to the Slideshow constructor so that it can be used on both apps and I've modified the close button's behaviour so that it matches the back button.
I think I have it working. I did notice a strange bug at one point where the view got lost, but with so much browser abuse, it could have been a user error...

So we could have a working Gallery, with position memory and if Files gets fixed in 8.2, we might be able to have a coherent behaviour on both sides.

If I can keep the position in Files by using pushState, then we might have a working solution before that...

setnes · 2015-07-01T03:40:54Z

One thing to note is that _stop() was being executed twice in my fix if the close button was pressed. I left it that way because it was actually fixing the other problem related to needing to press the button twice to close a slideshow in fullscreen mode. It wasn't causing any other issues to call it twice that I could see. A better fix would be to somehow see if we were already inside a call to _stop() before calling it again. That would also require fixing the double button press to exit fullscreen.

oparoz · 2015-07-01T10:24:32Z

Ah, thanks for the reminder. I've worked to eliminate lots of unnecessary events and divs and will try to find a solution to exit the slideshow with one click when in fullscreen mode...

oparoz · 2015-07-02T00:41:10Z

Good news, I'll be able to restore the position in the Files app :). The list still gets refreshed, but there is nothing we can do on 8.0 and 8.1.

oparoz · 2015-07-07T23:11:45Z

Can be tested by using ultimate8 on core stable8

@libasys you may be interested in the fix for GaDe

setnes · 2015-07-10T05:27:51Z

Is this also on ultimatedev? If so, I see a difference between using the browser back and the X. The thumbnail for the current position is grayed out when returning using the browser back and it is not grayed out when returning using the X. This is the same grayed out look the thumbnail has when you are clicking on it with a mouse. I'm not sure if the desired behavior is to show the position with the grayed out thumbnail or not. It is still very much usable either way.

oparoz · 2015-07-10T11:29:08Z

Good eye. It's always been there on Firefox and I didn't care much for it, but now that I'm trying to harmonize the apps' behaviour, I should take care of this.

We will need a new ticket so that designers can help me decide which option is best.

oparoz · 2015-07-11T00:28:50Z

@setnes Don't forget to collect your bounty ;)
https://www.bountysource.com/issues/17669693-slideshow-should-the-browser-back-button-behave-the-same-as-the-x-icon

Public sharing was broken because the legacy router passes those values as `$_GET` which has to be accessed this way in this situation. Furthermore this makes legacy links work again as they used `t` instead of `token` Needs a backport to stable8. Fixes owncloud/gallery#162 and owncloud/gallery#160

oparoz changed the title ~~Should the browser back button behave the same as the X icon?~~ Slideshow: should the browser back button behave the same as the X icon? May 29, 2015

oparoz added bug enhancement ready labels May 31, 2015

oparoz added v8.0 v8.1 labels Jun 21, 2015

oparoz changed the title ~~Slideshow: should the browser back button behave the same as the X icon?~~ Slideshow: should the browser back button behave the same as the X icon? [$5] Jun 21, 2015

oparoz added the bounty label Jun 21, 2015

setnes mentioned this issue Jun 21, 2015

Stop the slideshow when using the back button. #177

Closed

oparoz added feature:slideshow in progress and removed ready labels Jun 22, 2015

oparoz mentioned this issue Jun 30, 2015

Gallery+ viewer jumps to top position after closing an image #126

Closed

oparoz added this to the 13 milestone Jul 2, 2015

oparoz added QA-testing and removed in progress labels Jul 5, 2015

oparoz mentioned this issue Jul 5, 2015

Slideshow cleanup #207

Merged

oparoz closed this as completed in #207 Jul 10, 2015

oparoz removed the QA-testing label Jul 13, 2015

oparoz mentioned this issue Sep 8, 2015

File list refreshes after closing Gallery slideshow #313

Closed

oparoz mentioned this issue Dec 6, 2015

If there is no browser history, exit the slideshow by calling stop. #468

Open

oparoz mentioned this issue Sep 4, 2016

File list refreshes after closing Gallery slideshow nextcloud/gallery#72

Closed

oparoz removed the bounty label Apr 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Slideshow: should the browser back button behave the same as the X icon? [$5] #160

Slideshow: should the browser back button behave the same as the X icon? [$5] #160

setnes commented May 25, 2015

setnes commented May 29, 2015

setnes commented May 29, 2015

oparoz commented May 29, 2015

oparoz commented May 29, 2015

setnes commented Jun 10, 2015

oparoz commented Jun 11, 2015

setnes commented Jun 12, 2015

setnes commented Jun 20, 2015

oparoz commented Jun 21, 2015

setnes commented Jun 21, 2015

oparoz commented Jun 21, 2015

setnes commented Jun 21, 2015

oparoz commented Jun 21, 2015

oparoz commented Jun 30, 2015

setnes commented Jun 30, 2015

oparoz commented Jun 30, 2015

setnes commented Jul 1, 2015

oparoz commented Jul 1, 2015

oparoz commented Jul 2, 2015

oparoz commented Jul 7, 2015

setnes commented Jul 10, 2015

oparoz commented Jul 10, 2015

oparoz commented Jul 11, 2015

Slideshow: should the browser back button behave the same as the X icon? [$5] #160

Slideshow: should the browser back button behave the same as the X icon? [$5] #160

Comments

setnes commented May 25, 2015

@oparoz's assessment of the situation

Files

@oparoz's acceptance criteria

Files

Photowall

General

setnes commented May 29, 2015

setnes commented May 29, 2015

oparoz commented May 29, 2015

oparoz commented May 29, 2015

setnes commented Jun 10, 2015

oparoz commented Jun 11, 2015

setnes commented Jun 12, 2015

setnes commented Jun 20, 2015

oparoz commented Jun 21, 2015

Files

Photowall

General

setnes commented Jun 21, 2015

oparoz commented Jun 21, 2015

setnes commented Jun 21, 2015

oparoz commented Jun 21, 2015

oparoz commented Jun 30, 2015

Current situation

Files

setnes commented Jun 30, 2015

oparoz commented Jun 30, 2015

setnes commented Jul 1, 2015

oparoz commented Jul 1, 2015

oparoz commented Jul 2, 2015

oparoz commented Jul 7, 2015

setnes commented Jul 10, 2015

oparoz commented Jul 10, 2015

oparoz commented Jul 11, 2015