-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
"back" button function fixed #1573
The head ref may contain hidden characters: "\"back\"button-function-fixed"
"back" button function fixed #1573
Conversation
frontend/src/views/files/Preview.vue
Outdated
@@ -341,7 +341,7 @@ export default { | |||
this.$store.commit("updateRequest", {}); | |||
|
|||
let uri = url.removeLastDir(this.$route.path) + "/"; | |||
this.$router.push({ path: uri }); | |||
this.$router.replace({ path: uri }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming that the current directory is already on the history, replace
will insert another history entry for the same directory, making users to press the back button twice to go back from that directory. Use the push
method instead to avoid a duplicated history entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I've tested it. it's true.
But if we use "push", it will revisit the last pic or file if you click the "back" button.
Commonly, we use "this.$router.go(-1)" here, it's perfect now.
fixed!
frontend/src/views/files/Preview.vue
Outdated
|
||
let uri = url.removeLastDir(this.$route.path) + "/"; | ||
this.$router.push({ path: uri }); | ||
this.$router.go(-1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The go
method cannot be used because:
- We do not have control of the user history, the previous entry is not necessarily from FileBrowser.
- The user history is empty when accessing the page directly by its URL, since there's no previous page nothing will happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review.
Then, what should I use? Or, just use the same default func as the "back" button: history.back(); it also works here. Hope this time it's ok. when there is no history, it will close the browser.
Or if it still has problems, hope you can make a PR for it. 🥇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the changes addressed on the methods prev()
and next()
, keeping close()
untouched, are enough for this PR. If some browser is not behaving correctly with these changes, please add information how to reproduce the issue and will take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will change it back.
I'm not sure that it works as expected. Tested on latest chrome filebrowser.mov |
These changes affects the back/previous buttons from the previewer. Instead of adding an history entry for every item when navigating on the previewer, it will keep only a single entry of the latest opened file. For example: navigating through 10 images using the next button will add 10 entries to the history. With this PR, you will need to press the back button only once to exit from the preview, instead of pressing it 10 times. |
Yeah, o1egl, you are testing the editor.vue, not the preview.vue. This PR only fixed the next and prev buttons in preview.vue. When you browse the pictures using prev and next buttons, and then click back button, you will see the difference. to |
fixed #1562