after closing the image, the images list will stay in the same position.

[noveens]

Fixes: 26318 ( issue at owncloud/core )

(LINK : owncloud/core#26318 )

Licence: MIT or AGPL

Description

After clicking the exit button on the image pop-up, the list doesn't seem to stay at the same row, and this PR fixes that.

Tested on

• Ubuntu/Chrome

TODO

• See if the documentation needs to be updated.
• See if any of the tests need to be updated.

Check list

• Code is properly documented
• Code is properly formatted
• Commits have been squashed
• Tests (unit, integration, api and/or acceptance) are included
• Documentation (manuals or wiki) has been updated or is not required

Reviewers

@PVince81 @DeepDiver1975

[mention-bot]

@noveens, thanks for your PR! By analyzing the history of the files in this pull request, we identified @oparoz and @manishbisht to be potential reviewers.

[PVince81]

Okay, so the reason why it did the "browser back button" thing is because of URL changes.

Before you open the image, the URL looks like "http://localhost/owncloud/index.php/apps/files/?dir=/test&fileid=14".
Then after opening, the gallery appends some hash element: http://localhost/owncloud/index.php/apps/files/?dir=/test&fileid=14#/test/image.png

When you click on the arrows to switch images, it will also change the URL. Then using the browser back button would bring you to the previous URL and image. This is the purpose of this URL changing.

When closing before your PR, it would simulate clicking on the back button which would go back to the previous URL. However this logic triggers file app code for reloading the current folder, even though the current folder hasn't changed.

Your PR is not 100% correct because it only closes the slideshow but leaves the URL as is.

I think the correct fix is to rather fix the files app code to not trigger a reload if the URL path did not change, this is this ticket: owncloud/core#15520. Fixing this one is likely to also fix the problem for any other app.

@noveens can you have a look at owncloud/core#15520 instead ?

Thanks for looking into this. While this PR is not the right fix, it provided insights about what the gallery app is actually doing 😄

[noveens]

@PVince81 ,
After looking further into this, the problems exactly is :

When an image is opened, a new state is pushed onto the browser history stack.
So, when an image is opened and user clicks the back button, the image closes and list refreshes ( This should be like this ).

Now, in the requests for this bug,
when the close slideshow button is pressed, the file-list must not be refreshed.
But when the image is opened, a history-state is pushed to the history stack, and when we close the image, we have to do a back event to maintain the history stack.

So, IMO, we have 2 options as of now:

1. Don't push the browser state onto the browser-history stack while opening an image.

2. Push the state, but when closing the image, perform a back event but, after the file list gets refreshed, change the focus to the image that was previously opened.

Feel free to contradict this or add any arguments.

Thanks.

[qwertymodo]

In large directories, a refresh takes a very long time, so refreshing and then restoring the position is NOT a good solution. My vote is option #1. I don't see the need for pushing a browser state for an overlay. An overlay isn't a page change, it shouldn't be considered a back-button history event.

[noveens]

I agree with @qwertymodo

[PVince81]

@noveens have you tried fixing owncloud/core#15520 ?

The file list refresh is caused by the fact that the on pop state event always tells the file list (not the full page) to reload regardless whether the path has changed or not.

[noveens]

@PVince81 ,
I think we should close this, owncloud/core#15520 solves this.

[noveens]

Fix at : owncloud/core#27282