Merge lp:~widelands-dev/widelands/bug-1808169-disable-focus into lp:widelands

Proposed by Notabilis
Status: Merged
Merged at revision: 8971
Proposed branch: lp:~widelands-dev/widelands/bug-1808169-disable-focus
Merge into: lp:widelands
Diff against target: 108 lines (+4/-12)
4 files modified
src/ui_basic/button.cc (+2/-4)
src/ui_basic/checkbox.cc (+0/-3)
src/ui_basic/panel.cc (+1/-1)
src/ui_basic/slider.cc (+1/-4)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1808169-disable-focus
Reviewer Review Type Date Requested Status
Toni Förster played Approve
Review via email: mp+361097@code.launchpad.net

Commit message

Allowing hotkey usage while windows are open.

Description of the change

Allowing to use arrow keys while windows (e.g., ware statistics) are open.

Changed two points:
- The UI::Panel no longer grabs the keyboard focus when it is clicked but shouldn't grab it
- Some components (buttons, slider, checkbox) no longer accept the keyboard focus. I don't know why they ever did so, though. It possibly was connected to the Panel being able to switch between components with the Tab key. But since nearly no component handles any keys, I guess it is no (big) loss. But still my main problem with this merge request is: Does someone knows about or finds a functionality that breaks due to the no longer applied keyboard focus?

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4347. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/469721440.
Appveyor build 4141. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1808169_disable_focus-4141.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I don't see a problem with how things are now. We will need to reconsider this though if we want to implement full keyboard navigation, e.g. tab through checkboxes and activate/deactivate them by hitting the space bar.

8946. By Toni Förster

merged trunk

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4365. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/472186290.
Appveyor build 4158. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1808169_disable_focus-4158.

8947. By Toni Förster

merged trunk

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4404. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/479143097.
Appveyor build 4195. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1808169_disable_focus-4195.

Revision history for this message
Toni Förster (stonerl) wrote :

Can this be merged? I tested this and found no issues.

review: Approve (played)
Revision history for this message
GunChleoc (gunchleoc) wrote :

Let's have it. We can always revisit it whenever we get around to implementing more keyboard navigation.

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 4404. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/479143097.

Revision history for this message
Toni Förster (stonerl) wrote :

Again a Travis hiccup.

@bunnybot merge force

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/ui_basic/button.cc'
--- src/ui_basic/button.cc 2018-12-13 07:24:01 +0000
+++ src/ui_basic/button.cc 2019-01-13 20:25:18 +0000
@@ -60,7 +60,8 @@
60 title_image_(title_image),60 title_image_(title_image),
61 background_style_(g_gr->styles().button_style(init_style)) {61 background_style_(g_gr->styles().button_style(init_style)) {
62 set_thinks(false);62 set_thinks(false);
63 set_can_focus(true);63 // Don't allow focus
64 assert(!get_can_focus());
64}65}
6566
66Button::Button // for textual buttons. If h = 0, h will resize according to the font's height.67Button::Button // for textual buttons. If h = 0, h will resize according to the font's height.
@@ -143,8 +144,6 @@
143 if (enabled_ == on)144 if (enabled_ == on)
144 return;145 return;
145146
146 set_can_focus(on);
147
148 // disabled buttons should look different...147 // disabled buttons should look different...
149 if (on)148 if (on)
150 enabled_ = true;149 enabled_ = true;
@@ -316,7 +315,6 @@
316 return false;315 return false;
317316
318 if (enabled_) {317 if (enabled_) {
319 focus();
320 grab_mouse(true);318 grab_mouse(true);
321 pressed_ = true;319 pressed_ = true;
322 if (repeating_) {320 if (repeating_) {
323321
=== modified file 'src/ui_basic/checkbox.cc'
--- src/ui_basic/checkbox.cc 2018-12-13 07:24:01 +0000
+++ src/ui_basic/checkbox.cc 2019-01-13 20:25:18 +0000
@@ -50,7 +50,6 @@
50 uint16_t h = pic->height();50 uint16_t h = pic->height();
51 set_desired_size(w, h);51 set_desired_size(w, h);
52 set_size(w, h);52 set_size(w, h);
53 set_can_focus(true);
54 set_flags(Has_Custom_Picture, true);53 set_flags(Has_Custom_Picture, true);
55}54}
5655
@@ -98,7 +97,6 @@
98 * Args: enabled true if the checkbox should be enabled, false otherwise97 * Args: enabled true if the checkbox should be enabled, false otherwise
99 */98 */
100void Statebox::set_enabled(bool const enabled) {99void Statebox::set_enabled(bool const enabled) {
101 set_can_focus(enabled);
102 if (((flags_ & Is_Enabled) > 1) && enabled)100 if (((flags_ & Is_Enabled) > 1) && enabled)
103 return;101 return;
104102
@@ -177,7 +175,6 @@
177 */175 */
178bool Statebox::handle_mousepress(const uint8_t btn, int32_t, int32_t) {176bool Statebox::handle_mousepress(const uint8_t btn, int32_t, int32_t) {
179 if (btn == SDL_BUTTON_LEFT && (flags_ & Is_Enabled)) {177 if (btn == SDL_BUTTON_LEFT && (flags_ & Is_Enabled)) {
180 focus();
181 clicked();178 clicked();
182 return true;179 return true;
183 }180 }
184181
=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc 2018-12-13 07:24:01 +0000
+++ src/ui_basic/panel.cc 2019-01-13 20:25:18 +0000
@@ -528,7 +528,7 @@
528 * \return true if the mouseclick was processed, false otherwise528 * \return true if the mouseclick was processed, false otherwise
529 */529 */
530bool Panel::handle_mousepress(const uint8_t btn, int32_t, int32_t) {530bool Panel::handle_mousepress(const uint8_t btn, int32_t, int32_t) {
531 if (btn == SDL_BUTTON_LEFT) {531 if (btn == SDL_BUTTON_LEFT && get_can_focus()) {
532 focus();532 focus();
533 }533 }
534 return false;534 return false;
535535
=== modified file 'src/ui_basic/slider.cc'
--- src/ui_basic/slider.cc 2018-12-13 07:24:01 +0000
+++ src/ui_basic/slider.cc 2019-01-13 20:25:18 +0000
@@ -78,7 +78,7 @@
78 bar_size_(bar_size),78 bar_size_(bar_size),
79 cursor_size_(cursor_size) {79 cursor_size_(cursor_size) {
80 set_thinks(false);80 set_thinks(false);
81 set_can_focus(true);81 assert(!get_can_focus());
82 calculate_cursor_position();82 calculate_cursor_position();
83}83}
8484
@@ -205,7 +205,6 @@
205 if (enabled_ == enabled)205 if (enabled_ == enabled)
206 return;206 return;
207207
208 set_can_focus(enabled);
209 enabled_ = enabled;208 enabled_ = enabled;
210 if (!enabled) {209 if (!enabled) {
211 pressed_ = false;210 pressed_ = false;
@@ -402,7 +401,6 @@
402 if (btn != SDL_BUTTON_LEFT)401 if (btn != SDL_BUTTON_LEFT)
403 return false;402 return false;
404403
405 focus();
406 if (x >= cursor_pos_ && x <= cursor_pos_ + cursor_size_) {404 if (x >= cursor_pos_ && x <= cursor_pos_ + cursor_size_) {
407 // click on cursor405 // click on cursor
408 cursor_pressed(x);406 cursor_pressed(x);
@@ -469,7 +467,6 @@
469 if (btn != SDL_BUTTON_LEFT)467 if (btn != SDL_BUTTON_LEFT)
470 return false;468 return false;
471469
472 focus();
473 if (y >= cursor_pos_ && y <= cursor_pos_ + cursor_size_) {470 if (y >= cursor_pos_ && y <= cursor_pos_ + cursor_size_) {
474 // click on cursor471 // click on cursor
475 cursor_pressed(y);472 cursor_pressed(y);

Subscribers

People subscribed via source and target branches

to status/vote changes: