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

Default to adding a newline for everything before the extra_tooltip_text when binding an action to a button #6907

Open
dalthviz opened this issue May 10, 2024 · 3 comments
Labels
discussion Discussion

Comments

@dalthviz
Copy link
Member

dalthviz commented May 10, 2024

Follow up issue from the feedback at #6794 (comment)

Nice! Now wondering if we should default to adding a newline for everything before the extra_tooltip_text...

Originally posted by @brisvag in #6794 (comment)

That makes sense! From a quick check seems like the only other button that has some extra text is pan/zoom:

imagen

Maybe something like Temporarily re-enable by holding Space could be used as text in case the extra text is added always in a new line?

imagen

Also, should an issue be made to tackle that later or maybe is something worthy to be done here?

Originally posted by @dalthviz in #6794 (comment)

I mean I feel like the new line would be safe as a default...
but I don't think I would change it in this PR--if at all. It's easy to add the new line, harder to remove it if someone does want a one-liner.

Originally posted by @psobolewskiPhD in #6794 (comment)

@brisvag
Copy link
Contributor

brisvag commented May 13, 2024

It's easy to add the new line, harder to remove it if someone does want a one-liner.

I think if someone wants a oneliner, that means it should all live in the tooltip_text and not in the extra. It's also nice to have visual consistency between things.

@dalthviz
Copy link
Member Author

Checking the detail on how the tooltip is defined

if name in self._actions:
button.setToolTip(
f'{self._build_tooltip(name)} {extra_tooltip_text}'
)
def _update_tt(event: ShortcutEvent):
if event.name == name:
button.setToolTip(f'{event.tooltip} {extra_tooltip_text}')

maybe for consistency, but also to give some flexibility, the tooltip could be changed to be a template with three elements instead of just two? So:

  • Base text (which currently I think is related with the showing the button action name and action related shortcuts)
  • Extension text for that base text, usually some short text (which is currently used to add to the pan/zoom button the text (or hold Space))
  • Extra info/description that could go in a new line and is a much longer text (which could be used for the transform button for the Alt-Left mouse click to reset text)

And then we could have for the tooltip a template in the following way:

f'{base_text} {extra_tooltip_text}/n{new_line_extra_tooltip_text}'

However, I would say this approach will not help as much with visual consistency as just putting all the extra text in a new line 🤔

@brisvag
Copy link
Contributor

brisvag commented May 16, 2024

yeah, I don't think we need to that complex honestly ^^' I'd say either stick with the trick of adding \n, or add newlines between everything.

@psobolewskiPhD, opinions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion
Projects
None yet
Development

No branches or pull requests

2 participants