Merge lp:~fault/slingshot/fix-1520802 into lp:~elementary-pantheon/slingshot/trunk

Proposed by Casper Christiansen on 2015-12-19
Status: Merged
Approved by: Daniel Fore on 2015-12-23
Approved revision: 620
Merged at revision: 618
Proposed branch: lp:~fault/slingshot/fix-1520802
Merge into: lp:~elementary-pantheon/slingshot/trunk
Diff against target: 312 lines (+73/-117)
3 files modified
src/SlingshotView.vala (+37/-116)
src/Widgets/CategoryView.vala (+0/-1)
src/Widgets/Grid.vala (+36/-0)
To merge this branch: bzr merge lp:~fault/slingshot/fix-1520802
Reviewer Review Type Date Requested Status
Daniel Fore 2015-12-19 Approve on 2015-12-23
Review via email: mp+281012@code.launchpad.net

Commit message

Fix implementation of keyboard navigation in grid and category views

Description of the change

Implements keyboard navigation for the icon view to fix #1520802. As a side effect also fixes various issues with the previous implementation of navigation in the category view.

To post a comment you must log in.
Daniel Fore (danrabbit) wrote :

Please keep the code style consistent: https://elementary.io/docs/code/reference#code-style

Doesn't appear to fix keyboard navigation in grid view here. Testing on Loki Alpha0

review: Needs Fixing
lp:~fault/slingshot/fix-1520802 updated on 2015-12-23
617. By Casper Christiansen on 2015-12-23

Fixed coding style inconsistency

Casper Christiansen (fault) wrote :

> Please keep the code style consistent:
> https://elementary.io/docs/code/reference#code-style
>
> Doesn't appear to fix keyboard navigation in grid view here. Testing on Loki
> Alpha0

Fixed the coding style, but I'm not sure why it doesn't work for you. Seems Wingpanel doesn't reload plugins properly when they're changed, did you make sure to restart its process?

Daniel Fore (danrabbit) wrote :

Ah my mistake, I must have forgot to reload the panel. It seems to work as expected with the exception that pressing "enter" doesn't seem to launch the app.

Left a few more diff comments about code style :)

lp:~fault/slingshot/fix-1520802 updated on 2015-12-23
618. By Casper Christiansen on 2015-12-23

Fixed 'enter' key not launching the selected app

Casper Christiansen (fault) wrote :

Oh neat, didn't notice the diff comments before. :) Anyway both code style and 'enter' for launching should now be fixed.

Daniel Fore (danrabbit) wrote :

Awesome, it seems to be working fully as expected :D Can you merge trunk? Looks there are some small merge conflicts and then I think this is ready to go!

lp:~fault/slingshot/fix-1520802 updated on 2015-12-23
619. By Casper Christiansen on 2015-12-23

Merged with trunk

620. By Casper Christiansen on 2015-12-23

Fixed navigating from search entry to normal grid skipping an item

Daniel Fore (danrabbit) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/SlingshotView.vala'
2--- src/SlingshotView.vala 2015-12-19 14:15:38 +0000
3+++ src/SlingshotView.vala 2015-12-23 19:35:51 +0000
4@@ -66,12 +66,6 @@
5 private int default_columns;
6 private int default_rows;
7
8- private int column_focus = 0;
9- private int row_focus = 0;
10-
11- private int category_column_focus = 0;
12- private int category_row_focus = 0;
13-
14 private int primary_monitor = 0;
15
16 Gdk.Screen screen;
17@@ -328,14 +322,6 @@
18 return true;
19 }
20
21- switch (event.keyval) {
22- case Gdk.Key.Tab:
23- // context view is disabled until we get plugins that are actually
24- // useful with a context
25- // search_view.toggle_context (!search_view.in_context_view);
26- return true;
27- }
28-
29 return on_key_press (event);
30 }
31
32@@ -387,7 +373,7 @@
33 case "Return":
34 case "KP_Enter":
35 return false;
36-
37+
38 case "Alt_L":
39 case "Alt_R":
40 break;
41@@ -425,15 +411,11 @@
42
43 case "Tab":
44 if (modality == Modality.NORMAL_VIEW) {
45- view_selector.selected = 1;
46- var new_focus = category_view.app_view.get_child_at (category_column_focus, category_row_focus);
47- if (new_focus != null)
48- new_focus.grab_focus ();
49+ view_selector.selected = (int) Modality.CATEGORY_VIEW;
50+ category_view.app_view.top_left_focus ();
51 } else if (modality == Modality.CATEGORY_VIEW) {
52- view_selector.selected = 0;
53- var new_focus = grid_view.get_child_at (column_focus, row_focus);
54- if (new_focus != null)
55- new_focus.grab_focus ();
56+ view_selector.selected = (int) Modality.NORMAL_VIEW;
57+ grid_view.top_left_focus ();
58 }
59 break;
60
61@@ -466,7 +448,7 @@
62 if (event.state == Gdk.ModifierType.SHIFT_MASK) { // Shift + Up
63 if (category_view.category_switcher.selected != 0) {
64 category_view.category_switcher.selected--;
65- top_left_focus ();
66+ category_view.app_view.top_left_focus ();
67 }
68 } else if (search_entry.has_focus) {
69 category_view.category_switcher.selected--;
70@@ -480,11 +462,15 @@
71
72 case "Down":
73 if (modality == Modality.NORMAL_VIEW) {
74+ if (search_entry.has_focus) {
75+ grid_view.top_left_focus ();
76+ } else {
77 normal_move_focus (0, +1);
78+ }
79 } else if (modality == Modality.CATEGORY_VIEW) {
80 if (event.state == Gdk.ModifierType.SHIFT_MASK) { // Shift + Down
81 category_view.category_switcher.selected++;
82- top_left_focus ();
83+ category_view.app_view.top_left_focus ();
84 } else if (search_entry.has_focus) {
85 category_view.category_switcher.selected++;
86 } else { // the user has already selected an AppEntry
87@@ -500,7 +486,7 @@
88 grid_view.go_to_previous ();
89 } else if (modality == Modality.CATEGORY_VIEW) {
90 category_view.category_switcher.selected--;
91- top_left_focus ();
92+ category_view.app_view.top_left_focus ();
93 }
94 break;
95
96@@ -509,7 +495,7 @@
97 grid_view.go_to_next ();
98 } else if (modality == Modality.CATEGORY_VIEW) {
99 category_view.category_switcher.selected++;
100- top_left_focus ();
101+ category_view.app_view.top_left_focus ();
102 }
103 break;
104
105@@ -534,7 +520,7 @@
106 grid_view.go_to_number (1);
107 } else if (modality == Modality.CATEGORY_VIEW) {
108 category_view.category_switcher.selected = 0;
109- top_left_focus ();
110+ category_view.app_view.top_left_focus ();
111 }
112 break;
113
114@@ -547,7 +533,7 @@
115 grid_view.go_to_last ();
116 } else if (modality == Modality.CATEGORY_VIEW) {
117 category_view.category_switcher.selected = category_view.category_switcher.cat_size - 1;
118- top_left_focus ();
119+ category_view.app_view.top_left_focus ();
120 }
121 break;
122
123@@ -644,7 +630,7 @@
124 if (event.state == Gdk.ModifierType.SHIFT_MASK) // Shift + Right
125 category_view.app_view.go_to_next ();
126 else if (search_entry.has_focus) // there's no AppEntry selected, the user is switching category
127- top_left_focus ();
128+ category_view.app_view.top_left_focus ();
129 else //the user has already selected an AppEntry
130 category_move_focus (+1, 0);
131 }
132@@ -759,94 +745,29 @@
133 }
134
135 private void normal_move_focus (int delta_column, int delta_row) {
136-/* TODO
137- if (get_focus () as Widgets.AppEntry != null) { // we check if any AppEntry has focus. If it does, we move
138- if (column_focus + delta_column < 0 || row_focus + delta_row < 0)
139- return;
140-
141- var new_focus = grid_view.get_child_at (column_focus + delta_column, row_focus + delta_row); // we check if the new widget exists
142- if (new_focus == null) {
143- if (delta_column <= 0)
144- return;
145- else {
146- new_focus = grid_view.get_child_at (column_focus + delta_column, 0);
147- if (new_focus == null)
148- return;
149-
150- row_focus = -delta_row; // so it's 0 at the end
151- }
152- }
153-
154- column_focus += delta_column;
155- row_focus += delta_row;
156- if (delta_column > 0 && column_focus % grid_view.get_page_columns () == 0 ) //check if we need to change page
157- grid_view.go_to_next ();
158- else if (delta_column < 0 && (column_focus + 1) % grid_view.get_page_columns () == 0) //check if we need to change page
159- grid_view.go_to_previous ();
160-
161- new_focus.grab_focus ();
162- } else { // we move to the first app in the top left corner of the current page
163- column_focus = (grid_view.get_current_page ()-1) * grid_view.get_page_columns ();
164- if (column_focus >= 0)
165- grid_view.get_child_at (column_focus, 0).grab_focus ();
166- row_focus = 0;
167- }
168-*/
169+ if (grid_view.set_focus_relative (delta_column, delta_row)) {
170+ return;
171+ }
172+
173+ if (delta_column < 0 || delta_row < 0) {
174+ search_entry.grab_focus ();
175+ }
176 }
177
178 private void category_move_focus (int delta_column, int delta_row) {
179- var new_focus = category_view.app_view.get_child_at (category_column_focus + delta_column, category_row_focus + delta_row);
180- if (new_focus == null) {
181- if (delta_row < 0 && category_view.category_switcher.selected != 0) {
182- category_view.category_switcher.selected--;
183- top_left_focus ();
184- return;
185- } else if (delta_row > 0 && category_view.category_switcher.selected != category_view.category_switcher.cat_size - 1) {
186- category_view.category_switcher.selected++;
187- top_left_focus ();
188- return;
189- } else if (delta_column > 0 && (category_column_focus + delta_column) % category_view.app_view.get_page_columns () == 0
190- && category_view.app_view.get_current_page ()+ 1 != category_view.app_view.get_n_pages ()) {
191- category_view.app_view.go_to_next ();
192- top_left_focus ();
193- return;
194- } else if (category_column_focus == 0 && delta_column < 0) {
195- search_entry.grab_focus ();
196- category_column_focus = 0;
197- category_row_focus = 0;
198- return;
199- } else {
200- return;
201- }
202- }
203-
204- category_column_focus += delta_column;
205- category_row_focus += delta_row;
206- if (delta_column > 0 && category_column_focus % category_view.app_view.get_page_columns () == 0 ) { // check if we need to change page
207- category_view.app_view.go_to_next ();
208- } else if (delta_column < 0 && (category_column_focus + 1) % category_view.app_view.get_page_columns () == 0) {
209- // check if we need to change page
210- category_view.app_view.go_to_previous ();
211- }
212-
213- new_focus.grab_focus ();
214- }
215-
216- // this method moves focus to the first AppEntry in the top left corner of the current page. Works in CategoryView only
217- private void top_left_focus () {
218- // this is the first column of the current page
219- int first_column = (grid_view.get_current_page ()-1) * category_view.app_view.get_page_columns ();
220- category_view.app_view.get_child_at (first_column, 0).grab_focus ();
221- category_column_focus = first_column;
222- category_row_focus = 1;
223- }
224-
225- public void reset_category_focus () {
226- category_column_focus = 0;
227- category_row_focus = 0;
228- }
229-
230-
231+ if (category_view.app_view.set_focus_relative (delta_column, delta_row)) {
232+ return;
233+ }
234+
235+ if (delta_row < 0 && category_view.category_switcher.selected > 0) {
236+ category_view.category_switcher.selected--;
237+ category_view.app_view.top_left_focus ();
238+ } else if (delta_row > 0) {
239+ category_view.category_switcher.selected++;
240+ category_view.app_view.top_left_focus ();
241+ } else if (delta_column < 0 || delta_row < 0) {
242+ search_entry.grab_focus ();
243+ }
244+ }
245 }
246-
247 }
248
249=== modified file 'src/Widgets/CategoryView.vala'
250--- src/Widgets/CategoryView.vala 2015-09-23 22:46:57 +0000
251+++ src/Widgets/CategoryView.vala 2015-12-23 19:35:51 +0000
252@@ -88,7 +88,6 @@
253
254 private void connect_events () {
255 category_switcher.selection_changed.connect ((name, nth) => {
256- view.reset_category_focus ();
257 string category = category_ids.get (nth);
258 show_filtered_apps (category);
259 });
260
261=== modified file 'src/Widgets/Grid.vala'
262--- src/Widgets/Grid.vala 2015-02-01 15:02:37 +0000
263+++ src/Widgets/Grid.vala 2015-12-23 19:35:51 +0000
264@@ -25,6 +25,11 @@
265 }
266
267 public class Grid : Gtk.Box {
268+
269+ public int focused_column { get; private set; }
270+ public int focused_row { get; private set; }
271+
272+ public Gtk.Widget? focused_widget { get; private set; }
273
274 public Widgets.Switcher page_switcher;
275
276@@ -178,5 +183,36 @@
277 page.columns = columns;
278 page.number = 1;
279 }
280+
281+ public bool set_focus (int column, int row) {
282+ var targetWidget = get_child_at (column, row);
283+
284+ if (targetWidget != null) {
285+ go_to_number (((int) (column / page.columns)) + 1);
286+
287+ focused_column = column;
288+ focused_row = row;
289+ focused_widget = targetWidget;
290+
291+ focused_widget.grab_focus ();
292+
293+ return true;
294+ }
295+
296+ return false;
297+ }
298+
299+ public bool set_paginated_focus (int column, int row) {
300+ int first_column = (get_current_page () - 1) * get_page_columns ();
301+ return set_focus (first_column, 0);
302+ }
303+
304+ public bool set_focus_relative (int delta_column, int delta_row) {
305+ return set_focus (focused_column + delta_column, focused_row + delta_row);
306+ }
307+
308+ public void top_left_focus () {
309+ set_paginated_focus (0, 0);
310+ }
311 }
312 }

Subscribers

People subscribed via source and target branches