-
Notifications
You must be signed in to change notification settings - Fork 323
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
Add tooltips to the action bar #6035
Conversation
d88144e
to
ce7010e
Compare
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.
Beside the code comments:
- The text on the tooltip is not vertically centered - it looks bad. This should be fixed.
- The tooltip should appear with some delay. This should be controlled by the mouse tooltip logic. We can modify it in a separate task, but its important to fix it.
CHANGELOG.md
Outdated
@@ -126,6 +126,7 @@ | |||
shortened labels for entries with long module paths. When an option is | |||
selected from the dropdown, the necessary module imports are inserted, | |||
eliminating the need for fully qualified names. | |||
- [Icon buttons now have tooltips.][6035] |
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.
Please write a better description - it is user-facing changelog.
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.
Any suggestions? Please be constructive when providing PR comments.
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.
I was sure it was constructive enough. User-facing changelog should describe what is the change, why it was made and what benefits it gives users.
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.
I played with GPT for a minute:
[Added tooltips to icon buttons][6035] for improved usability. Users can now quickly understand each button's function.
// Explicitly define the tooltip placement if the default was used, to ensure that, when | ||
// multiple tooltips are being used, we always override to the one we want. |
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.
I don't understand this sentence. What is "the one that we want"? Also, it has some minor English grammar mistakes. See granary or chatgpt to check correctness, please.
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.
I've added some extra context. Let me know if you're still confused. Happy to jump into a call to explain it.
@wdanilo: I agree on both points you raise. However, they seem orthogonal to this change, so I suggest doing both in a separate PR. |
@Procrat please create an issue for text position + show delay, let's make it now, in a separate PR. |
I've create an issue here for the visual improvement of tooltips in general. |
* develop: Layout fixes (#6066) Use new Enso Hash Codes and Comparable (#6060) Search suggestions by static attribute (#6036) Use .node-version for pinning Node.js version (#6057) Fix code generation for suggestion preview (#6054) Implementation of EnsoGL predefined Rectangle shape. (#6033) Tidy up the public module level statics (#6032) Cursor aware Component Browser (#5770)
QA: Passed. |
Pull Request Description
Implements #5933: adding tooltips to the buttons next to nodes.
To make the UI consistent, I've added tooltips to the
ToggleButton
class directly, since whenever you have an icon button, it seems helpful to have a tooltip.ToggleButton
is only used for the profiling button in the top-right corner and the buttons next to nodes. The output context switch button isn't implemented yet, but once it is, adding a tooltip should be one-liner.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
./run ide build
.