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

Remove query parameters when skipping search results #82234

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Feb 17, 2021

Fixes #81330.

This PR changes the following: when pressing ESC and that no other "action" was performed (understand: no closing the search result, or hiding a menu or something along the line), then we discard the URL query parameters (the ?whatever=dsjfs). What do you think about this change @rust-lang/rustdoc ?

EDIT: finally we're simply removing the query parameter when we're skipping the search results.

r? @Nemo157

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Feb 17, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@camelid
Copy link
Member

camelid commented Feb 17, 2021

Instead of doing this more generally, can we just remove query parameters when people press esc when the search view is active?

@GuillaumeGomez
Copy link
Member Author

You mean when closing the search result view? I can do that too.

@camelid
Copy link
Member

camelid commented Feb 17, 2021

No, I mean I don't understand why it's implemented for things like pressing esc with the theme changer button. Can't we just add a bit of code to the handler for closing the search view that removes the query parameters? In what other situations are there query parameters added?

@GuillaumeGomez
Copy link
Member Author

None iirc.

@camelid
Copy link
Member

camelid commented Feb 17, 2021

So can you change this to only handle the case of closing the search view?

@GuillaumeGomez
Copy link
Member Author

Yes sir!

@GuillaumeGomez GuillaumeGomez changed the title Remove query parameters when pressing escape and no other action was performed Remove query parameters when skipping search results Feb 17, 2021
@GuillaumeGomez
Copy link
Member Author

Updated!

@Nemo157
Copy link
Member

Nemo157 commented Feb 22, 2021

bors r+

@GuillaumeGomez
Copy link
Member Author

@bors: r=Nemo157 rollup

@bors
Copy link
Contributor

bors commented Feb 22, 2021

📌 Commit eeb5552 has been approved by Nemo157

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 22, 2021
…n-esc, r=Nemo157

Remove query parameters when skipping search results

Fixes rust-lang#81330.

This PR changes the following: when pressing ESC and that no other "action" was performed (understand: no closing the search result, or hiding a menu or something along the line), then we discard the URL query parameters (the `?whatever=dsjfs`). What do you think about this change `@rust-lang/rustdoc` ?

EDIT: finally we're simply removing the query parameter when we're skipping the search results.

r? `@Nemo157`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 23, 2021
…n-esc, r=Nemo157

Remove query parameters when skipping search results

Fixes rust-lang#81330.

This PR changes the following: when pressing ESC and that no other "action" was performed (understand: no closing the search result, or hiding a menu or something along the line), then we discard the URL query parameters (the `?whatever=dsjfs`). What do you think about this change ``@rust-lang/rustdoc`` ?

EDIT: finally we're simply removing the query parameter when we're skipping the search results.

r? ``@Nemo157``
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2021
Rollup of 12 pull requests

Successful merges:

 - rust-lang#79423 (Enable smart punctuation)
 - rust-lang#81154 (Improve design of `assert_len`)
 - rust-lang#81235 (Improve suggestion for tuple struct pattern matching errors.)
 - rust-lang#81769 (Suggest `return`ing tail expressions that match return type)
 - rust-lang#81837 (Slight perf improvement on char::to_ascii_lowercase)
 - rust-lang#81969 (Avoid `cfg_if` in `std::os`)
 - rust-lang#81984 (Make WASI's `hard_link` behavior match other platforms.)
 - rust-lang#82091 (use PlaceRef abstractions more consistently)
 - rust-lang#82128 (add diagnostic items for OsString/PathBuf/Owned as well as to_vec on slice)
 - rust-lang#82166 (add s390x-unknown-linux-musl target)
 - rust-lang#82234 (Remove query parameters when skipping search results)
 - rust-lang#82255 (Make `treat_err_as_bug` Option<NonZeroUsize>)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8541435 into rust-lang:master Feb 23, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 23, 2021
@GuillaumeGomez GuillaumeGomez deleted the remove-query-param-on-esc branch February 23, 2021 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[rustdoc] Remove query component from URL by pressing ESC
6 participants