fixes #28696 #28725
fixes #28696 #28725
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@steveklabnik is fixed this issue |
|
I reviewed this locally and it works well, but I'd like someone else to sign off on it too. |
| @@ -890,6 +890,22 @@ | |||
| return "\u2212"; // "\u2212" is '−' minus sign | |||
| } | |||
|
|
|||
| $(".search-input").on("keyup",function() { | |||
| if ($(this).val().length === 0) { | |||
| window.history.pushState("", "std - Rust", "?search="); | |||
There was a problem hiding this comment.
Either use replaceState, or ensure that we only push at most 1 instance of this. Otherwise we just mess up the browser’s history stack.
There was a problem hiding this comment.
thanks for the advice @nagisa I am going to change that
| } | ||
| }); | ||
|
|
||
| $('.search-input').on('search', function () { |
There was a problem hiding this comment.
Ah, also, search event is webkit (safari, epiphany) specific. It is not standard, nor it is supported widely.
There was a problem hiding this comment.
I added search for webkit for ("X") in chrome... or maybe remove X in chrome... what do you think?
There was a problem hiding this comment.
Just use a standard event (input should do), like in this PR.
|
@nagisa is happy with this, I'm happy with this. r=me after a squash. |
|
Yes thanks @marti1125! This also looks good to me as well. Can you also expand the commit messages to have a more detailed description of what was changed, what was fixed, and how it works? It's helpful when browsing the history of the repo to have a little more context than "update main.js" and "fixes #xxxx" :) |
|
@alexcrichton I think is done =D |
|
Thanks! Could you also squash all the commits together and perhaps merge some of the messages? You can find some good examples of how to write nice commit messages here. |
|
@alexcrichton I am going to create a new clean PR with solution |
#28696