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

Add hotkey settings panel with a hotkey remap dialog #16552

Merged
merged 5 commits into from Jun 28, 2019

Conversation

@dragunoff
Copy link
Contributor

commented May 17, 2019

Implement a hotkey settings remap dialog with validation (to prevent duplicate hotkeys) and buttons to unbind and reset to default. To make room for the dialog the hotkeys are moved to their own settings panel.

изображение
The new hotkey panel before selecting a hotkey.

изображение
The panel and dialog during rebinding (input is blinking)

изображение
Default binding

изображение
Invalid binding (keeps blinking and won't save)

изображение
Cleared binding (sets the key to UNKNOWN None)

изображение
I did it a little differently for TD (the buttons use glyphs).

This probably needs and upgrade rule somewhere. I'm very much open to feedback (code and UI/UX related).

Closes #16001
Closes #14670
Closes #3986
Closes #13200

@dragunoff dragunoff referenced this pull request May 17, 2019

@dragunoff dragunoff force-pushed the dragunoff:feature/hotkey-settings-ui branch from 3780cb8 to 5de94aa May 23, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

Updated and rebased.

Added initial validation for duplicates that may arise due to new hotkeys conflicting with user defined ones or the user manually editing settings.yaml.
duplicate-hotkeys-initial

My code in SettingsLogic.cs is starting to look like spaghetti...

OpenRA.Mods.Common/Widgets/ButtonWidget.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/HotkeyDialogLogic.cs Outdated Show resolved Hide resolved
@teinarss

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

@dragunoff dragunoff force-pushed the dragunoff:feature/hotkey-settings-ui branch from 5de94aa to 2ebbd87 Jun 19, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

Rebased and updated to address comments.

While I was at it I also added a tootip for truncated button text:
изображение

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Jun 19, 2019

Tbh, maybe we should consider increasing the width of those. It is not really obvious that Ctrl + Shift is truncated there.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

Tbh, maybe we should consider increasing the width of those. It is not really obvious that Ctrl + Shift is truncated there.

🤔 It's not obvious indeed. I'll try adding an ellipsis.

Making the buttons wider would cause some of the lengthier descriptions to overflow. To be able to increase width here we need a bigger settings window (see #16596).

@dragunoff dragunoff force-pushed the dragunoff:feature/hotkey-settings-ui branch from 2ebbd87 to 40cba19 Jun 20, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

изображение

Added ellipsis to truncated buttons that have a tooltip container.

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Jun 23, 2019

Needs a rebase.

dragunoff added 5 commits May 10, 2019
Scissor the text of a ButtonWidget if it overflows and display a tooltip
Also:
* add a variable to a comomn pattern used for truncating text in HotkeyEntryWidget and TextFieldWidget
Add hotkey settings panel with a hotkey remap dialog
* Add HotkeyDialogLogic.cs
* Add dialog-hotkey.yaml to all mods
* Add `GetFirstDuplicate` method to HotkeyManager to aid in validation
* Add "Player" and/or "Spectator" type to all hotkeys to allow for
validation based on overlapping types
* Change settings.yaml and SettingsLogic.cs to work with the new dialog

@dragunoff dragunoff force-pushed the dragunoff:feature/hotkey-settings-ui branch from 40cba19 to 1589d93 Jun 23, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2019

Rebased.

@abcdefg30 abcdefg30 merged commit c9ff54b into OpenRA:bleed Jun 28, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

commented Jun 28, 2019

var isTextOverflowing = font.Measure(text).X > contentWidth;
if (isTextOverflowing && TooltipContainer != null)
{
Game.Renderer.EnableScissor(new Rectangle(RenderOrigin.X + LeftMargin, RenderOrigin.Y,

This comment has been minimized.

Copy link
@pchote

pchote Jun 28, 2019

Member

Why is this scissoring and truncating the text? The scissoring will be redundant here, and has a large performance overhead that we really want to avoid.

This comment has been minimized.

Copy link
@dragunoff

dragunoff Jun 28, 2019

Author Contributor

You mean that truncating is sufficient? I didn't realize that... At first it was only scissoring (modeled after hotkey input/text input widget) but there had to be a visual cue so I added truncation. I can fix that in a follow up.

This comment has been minimized.

Copy link
@pchote

pchote Jun 28, 2019

Member

TruncateText guarantees that the text width is less than contentWidth, provided that contentWidth is wide enough to fit at least "...".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.