-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Improve help popup #75976
Improve help popup #75976
Conversation
Some changes occurred in HTML/CSS/JS. |
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 second commit I'm not comfortable approving ... I don't see anything wrong but I don't think the frontend is tested very well and it doesn't look obviously correct. Have you tested it locally?
I guess my concern is that there's a risk we break the help popup for not much gain.
Yes I did. :) You can check easily this js code if you want: function foo() {
console.log("foo!");
foo = function() {};
}
foo(); // will display "foo!"
foo(); // will display nothing The dark magic from JS. :3 |
05c8c74
to
86e42c2
Compare
Updated! |
Can you add a comment [above r=me after that. |
We discussed this on discord - all the code is (or should be) using @bors r+ |
📌 Commit 86e42c2 has been approved by |
Improve help popup Fixes rust-lang#75623. The second commit is just a slight improvement: the help popup won't be created until someone presses "?" or ESC. Not a big improvement in itself but considering the low amount of code required, I think it was worth the shot. r? @jyn514
⌛ Testing commit 86e42c2 with merge 26160d4083c97f5a75a25f04f1d9ba6c7952aa1e... |
@bors retry yield (this was included in a rollup that i forgot to start after creating it :P ) |
☀️ Test successful - checks-actions, checks-azure |
Fixes #75623.
The second commit is just a slight improvement: the help popup won't be created until someone presses "?" or ESC. Not a big improvement in itself but considering the low amount of code required, I think it was worth the shot.
r? @jyn514