Remove 'search' url query parameter #4332
Conversation
| @@ -147,7 +145,6 @@ | |||
| ast.links = {}; | |||
|
|
|||
| queryParams.set('version', this.version); | |||
| queryParams.set('search', this.searchCondition); | |||
| const curUrl = window.location.pathname + | |||
| '?' + queryParams.toString() + window.location.hash; | |||
| history.pushState(null, '', curUrl); | |||
There was a problem hiding this comment.
I think a better way to do what you want (not change the browser history) would be to use replaceState instead of pushState here.
Getting rid of the search condition param would not load the search condition if someone copy/pasted the url, which is probably not very useful either since we now have anchors, but it feels like a separate issue.
There was a problem hiding this comment.
which is probably not very useful either since we now have anchors, but it feels like a separate issue.
While what qualifies as "very useful" is inherently subjective, but for me the same rationale applies. If someone comes to us and says "I don't like the way rustfmt changes my match formatting how can change it" I'd like to be able to give them a link and say "here's the config options available that impact match formatting if you want to try something different".
However, if we can only have one or the other, then I would agree that the anchors for individual options is a higher priority
There was a problem hiding this comment.
@ayazhafiz @calebcartwright Thank you for your reviews! I am quite a newbie to the frontend stuff, good to learn a new thing 😄
I agree that keeping the search query parameter is useful. Will try using replaceState and see if that works as intended. If it does, I will update the PR so that we keep the search query parameter.
|
Going to close given the age and conflicts, but can revisit in the future |
This PR removes the
searchquery parameter from the GitHub page.Currently, when we type in the search box, the URL gets kept in sync with the input. This is a bit annoying as it keeps adding each URL to the history.
Since the value of the search box is managed by
vue-async-computed, we do not need the query parameter to pass around the value of the search box.@ayazhafiz @calebcartwright If I understand correctly this PR does not go against the motivation discussed at #4261, though please let me know if I am missing something.