Improves some text field behavior. #8118
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,14 +190,21 @@ Action InitDisplayPanel(Widget panel) | |
|
|
||
| var frameLimitTextfield = panel.Get<TextFieldWidget>("FRAME_LIMIT_TEXTFIELD"); | ||
| frameLimitTextfield.Text = ds.MaxFramerate.ToString(); | ||
| frameLimitTextfield.OnEnterKey = () => | ||
| { | ||
| int fps; | ||
| Exts.TryParseIntegerInvariant(frameLimitTextfield.Text, out fps); | ||
| ds.MaxFramerate = fps.Clamp(1, 1000); | ||
| frameLimitTextfield.Text = ds.MaxFramerate.ToString(); | ||
| frameLimitTextfield.YieldKeyboardFocus(); | ||
| return true; | ||
| }; | ||
| frameLimitTextfield.OnEscKey = () => | ||
|
There was a problem hiding this comment. When I press ESC I usually assume the GUI will remove the cursor, not switch to the last selection. There was a problem hiding this comment. This resets the text field to last saved value and then removes the cursor. Should it save the current value when you press ESC? There was a problem hiding this comment. I'd expect ESC to act like a Cancel button, not like Enter/OK/Apply. There was a problem hiding this comment. Oh ok now I get what you mean. I'll try to fix that. There was a problem hiding this comment. @penev92: He means when you click the other text field while one of them has focus and then press ESC the other text field gets focus back. This is good in lobby but bad here. There was a problem hiding this comment. And in the lobby, if you alternatingly click on chat field and player name field, you are also adding them multiple times to the stack. But there it doesn't cause any problems because chat field doesn't yield focus when ESC is pressed, it just resets the text, so there is no focus switching with ESC. And when the lobby widget is removed, these text fields become null and stay on the stack and get cleaned up with the next YieldKeyboardFocus() call. There was a problem hiding this comment. This is pretty weird. On Mon, Jun 8, 2015 at 10:10 AM, Deniz notifications@github.com wrote:
There was a problem hiding this comment. How should it be done otherwise? There was a problem hiding this comment. Ok this also happens in Input settings menu with hotkey assignment. If you keep clicking on different hotkey fields, they all get added to the stack and when you press ESC you switch focus in reverse order. So there has to be a way to prevent other text fields in the same widget from getting focus. There was a problem hiding this comment.
Convention is that TAB cycles input fields and ESC just leaves them imho. |
||
| { | ||
| frameLimitTextfield.Text = ds.MaxFramerate.ToString(); | ||
| frameLimitTextfield.YieldKeyboardFocus(); | ||
| return true; | ||
| }; | ||
| frameLimitTextfield.IsDisabled = () => !ds.CapFramerate; | ||
|
|
||
| // Player profile | ||
|
|
@@ -206,11 +213,25 @@ Action InitDisplayPanel(Widget panel) | |
| var nameTextfield = panel.Get<TextFieldWidget>("PLAYERNAME"); | ||
| nameTextfield.IsDisabled = () => worldRenderer.World.Type != WorldType.Shellmap; | ||
| nameTextfield.Text = Settings.SanitizedPlayerName(ps.Name); | ||
| nameTextfield.OnEnterKey = () => | ||
| { | ||
| nameTextfield.Text = nameTextfield.Text.Trim(); | ||
| if (nameTextfield.Text.Length == 0) | ||
| nameTextfield.Text = Settings.SanitizedPlayerName(ps.Name); | ||
| else | ||
| { | ||
| nameTextfield.Text = Settings.SanitizedPlayerName(nameTextfield.Text); | ||
| ps.Name = nameTextfield.Text; | ||
| } | ||
|
|
||
| nameTextfield.YieldKeyboardFocus(); | ||
| return true; | ||
| }; | ||
| nameTextfield.OnEscKey = () => | ||
| { | ||
| nameTextfield.Text = Settings.SanitizedPlayerName(ps.Name); | ||
| nameTextfield.YieldKeyboardFocus(); | ||
| return true; | ||
| }; | ||
|
|
||
| var colorPreview = panel.Get<ColorPreviewManagerWidget>("COLOR_MANAGER"); | ||
|
|
@@ -475,16 +496,7 @@ Action InitInputPanel(Widget panel) | |
| BindHotkeyPref(kv, ks, globalTemplate, hotkeyList); | ||
| } | ||
|
|
||
| return () => { }; | ||
|
There was a problem hiding this comment. Apparently this code block was unnecessary :/ |
||
| } | ||
|
|
||
| Action ResetInputPanel(Widget panel) | ||
|
|
||
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.
This seems to be completely unused outside this class.
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.
Yes that is a template method. A widget can overwrite it and define custom conditions to prevent others from taking focus. This was partially implemented in YieldKeyboardFocus() and used here: https://github.com/OpenRA/OpenRA/pull/8118/files#diff-5a71e6a689956fb5a5f3315e795c9a75L292 . I moved that to this method to make it clearer.