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

Incorrect UX for back navigation after search in docs #57135

Closed
mqudsi opened this issue Dec 26, 2018 · 8 comments
Closed

Incorrect UX for back navigation after search in docs #57135

mqudsi opened this issue Dec 26, 2018 · 8 comments
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@mqudsi
Copy link
Contributor

mqudsi commented Dec 26, 2018

The UX for navigating after a search in the documentation in incorrect. Upon searching in the documentation, the search takes over the contents of the current page, i.e. it is not displayed in a lightbox or pop-up div. In this case, the use of the back navigation button should act as exiting the search and returning to the page in the documentation that the user was on prior to triggering the search feature.

In practice, using the back button goes back to the 2nd to last page the user was on before triggering search, and a backwards-then-forwards double navigation is required to end up on the page that the user was on before search.

The solution is to use the history push state api to add an entry to the backwards navigation stack that would return the user to the last-viewed page upon triggering the search after clicking the backwards navigation button.

(Yes, I am aware that it is possible to close the search box and go back to the last page via other means. The point is that the content of the window act as if a full navigation has taken place, in which case the most natural means of returning to the original content for the user is to press the back button and not escape, etc.)

@csmoe csmoe added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 27, 2018
@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Jan 7, 2019

This feature is already implemented and works as described on Firefox 64. Perhaps the author's browser doesn't support the history API or feature detection is not working correctly.

Edit:

It looks like the history api doesn't interact well with file:// URIs. Maybe this is what @mqudsi was experiencing.

@GuillaumeGomez
Copy link
Member

It's not working the same way on file:// urls but it should indeed.

@GuillaumeGomez
Copy link
Member

Here's the PR for reference: #28400

@mqudsi
Copy link
Contributor Author

mqudsi commented Jan 10, 2019

Yes, I was working with locally compiled documentation.

@mqudsi
Copy link
Contributor Author

mqudsi commented Jan 10, 2019

Fwiw; I believe the correct solution here is to trigger an actual Page navigation upon search, such that use of the history api not needed.

@GuillaumeGomez
Copy link
Member

@mqudsi: You don't want that. In case you're looking for an element on the current page, it'll reload the whole page, which isn't very nice. I tested to ren-enable the history management locally with file:// and it seemed to work so I'll send a PR for it.

@mqudsi
Copy link
Contributor Author

mqudsi commented Jan 11, 2019

@GuillaumeGomez I mean only when search has already taken over the page (so the contents are completely hidden and navigation or ajax manipulation looks the same). Remember this is local, so there is no network-associated delay.

@GuillaumeGomez
Copy link
Member

Try the iterator trait page locally. 😈

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 7, 2019
…=QuietMisdreavus

Re-enable history api on file:// protocol

Fixes rust-lang#57135.

I tested locally on chrome (since it was the browser having issues with history management on `file://` protocol) and it worked fine so I guess we can re-enable it.

r? @QuietMisdreavus
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants