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

Hotkey settings dialog needs further refinements #16812

Closed
pchote opened this issue Jul 20, 2019 · 1 comment · Fixed by #16816
Closed

Hotkey settings dialog needs further refinements #16812

pchote opened this issue Jul 20, 2019 · 1 comment · Fixed by #16816
Assignees

Comments

@pchote
Copy link
Member

pchote commented Jul 20, 2019

From IRC:

[18:53:45] | <dragunoff> cause I was not able to make tooltips work there... they just stop showing
...
[18:54:13] | <pchote> this suggests your rebinding ui adds a redundant tooltip root
[18:54:19] | <pchote> which may be related to your issue
[18:54:29] | <pchote> if the root is child of a hidden node, for example
[18:54:33] | <pchote> that would make the tooltip invisible
[18:55:02] | <pchote>  also IMO we should get rid of the "select a hotkey to start rebinding"  line completely - just select the first hotkey by default
[18:55:58] | <pchote> also the clear / reset / cancel buttons don't have the standard bold button text :(
[18:57:17] | <dragunoff> the bold text - it's on purpose, I didn't want them to clash with the "main" panel buttons
[18:57:39] | <pchote> do we have anywhere else in the UI that unbolds them?
[18:57:40] | <dragunoff> but if non-bold text is nowhere else to be seen on buttons, then I'm fine with changing it
[18:59:45] | <dragunoff> looks like bold is the norm, so then these should be changed for consistency
...
[19:00:37] | <dragunoff> wrt to "select a hotkey to start rebinding" - I'm fine with removing that one
[19:01:04] | <pchote> i'm pretty sure this is the cause of your issue in #16759
[19:01:05] | <orabot> Pull request #16759 (closed) by dragunoff: Add missing tooltip container to settings.yaml \| http://bugs.openra.net/16759
[19:01:21] | <pchote> so we could turn that into the "fix pchotes complaints" pr? :)
[19:01:35] | <dragunoff> and also making a click on an already active button to restart the rebinding
[19:01:46] | <pchote> thats a good idea
[19:01:59] | <dragunoff> I caught myself many times clicking on the active button just to have the dialog disappear
[19:02:14] | <dragunoff> it's a little frustrating
[19:02:57] | <dragunoff> yeah, ok -- but I'm not sure I understand the redundant tooltip root problem
[19:03:16] | <pchote> if you check out the bleed commit before the fix merge
[19:03:33] | <pchote> make sure you have a hotkey defined that overflows and needs a tooltip
[19:03:49] | <pchote> start the game and open the settings menu, and then without hovering over the long hotkey, select a different one
[19:04:03] | <pchote> if you hover over the long hotkey it will then show the tooltip instead of crashing
[19:04:27] | <pchote> this implies that selecting the hotkey must be creating a tooltip container widget
[19:04:35] | <dragunoff> got it
[19:04:49] | <pchote> if you have multiple widgets with the same id and try to query it, confusing thing will happen
[19:05:55] | <dragunoff> I still don't understand the widgets system so I code by looking at how it's done in other places... I will try to fix that
[19:06:13] | <dragunoff> so you mean that the extra tooltipContainer is not needed?
[19:07:06] | <pchote> i'm trying to see where it is defined
[19:08:13] | <pchote> ah you have a separate dialog-hotkey.yaml
[19:08:30] | <pchote> the TooltipContainer in there will be the problem
[19:08:45] | <pchote> but if we are going to get rid of the placeholder then that dialog can be merged directly inline in settings.yaml
@pchote pchote added this to the Next Release milestone Jul 20, 2019
@dragunoff
Copy link
Contributor

To summarize:

  1. Remove redundant TooltipContainer from dialog-hotkey.yaml
  2. Make button font bold (for common yamls)
  3. Remove remap placeholder and highlight first hotkey by default
  4. Make clicking on an already active hotkey restart the rebinding instead of hiding the panel
  5. Inline dialog-hotkey.yaml into settings.yaml

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

Successfully merging a pull request may close this issue.

3 participants