Add hotkey hints to editor menus #29691
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It didn't ever really make sense for it to be sharing the implementation details of that (e.g. colouring of primary/dangerous actions), and with the hotkey display things got outright hacky, so I'm decoupling it entirely.
bdach
commented
Sep 4, 2024
Comment on lines
+107
to
+111
| // this hack ensures that the menu can auto-size while leaving enough space for the hotkey display. | ||
| // the gist of it is that while the hotkey display is not in the text / "content" that determines sizing | ||
| // (because it cannot be, because we want the hotkey display to align to the *right* and not the left), | ||
| // enough padding to fit the hotkey with _its_ spacing is added as padding of the text to compensate. | ||
| text.Padding = new MarginPadding { Right = hotkey.Alpha > 0 || showChevron ? hotkey.DrawWidth + 15 : 0 }; |
There was a problem hiding this comment.
This is the one hack I did not manage to get rid of, so do keep that in mind.
There is probably one way I could get rid of this but it likely involves copious GridContainer usage (to the tune of one per menu item) so I'm not sure of the general feelings on that solution.
| @@ -78,8 +81,11 @@ private void load(OverlayColourProvider colourProvider, TextureStore textures) | |||
|
|
|||
| protected override DrawableMenuItem CreateDrawableMenuItem(MenuItem item) => new DrawableEditorBarMenuItem(item); | |||
|
|
|||
| private partial class DrawableEditorBarMenuItem : DrawableOsuMenuItem | |||
| internal partial class DrawableEditorBarMenuItem : DrawableMenuItem | |||
There was a problem hiding this comment.
This de-inheritance and resulting code duplication was incurred intentionally to get rid of ugly digging in the innards of the base to hide the checkbox display to the left of menu items. See: 0c4f5bc
|
For 162558e see:
|
peppy
approved these changes
Oct 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
2024-09-04.12-26-32.mp4
RFC.
This is pretty old code that I had lying around on a branch somewhere and showed off on discord once but I didn't get around to cleaning it up until now. It still has a hack in one place that I'm not super sure how to get rid of, so caveat emptor.
Note that this also slightly alters the design and spacing of the menus globally - all menus will now have a space to the left of the item to make sure a checkbox or something else can be displayed there.
For demonstration purposes this is only shown off in the editor right now, but there are probably other places where these could be directly applied pretty much immediately.
Note that the
HotkeyDisplayis designed to be reusable, and supportsKeyCombinations,GlobalActions, andPlatformActions out of the box.