Merge lp:~tintou/slingshot/consistent-searchbar into lp:~elementary-pantheon/slingshot/trunk

Proposed by Corentin Noël on 2014-10-14
Status: Merged
Approved by: Daniel Fore on 2014-10-14
Approved revision: 466
Merged at revision: 461
Proposed branch: lp:~tintou/slingshot/consistent-searchbar
Merge into: lp:~elementary-pantheon/slingshot/trunk
Diff against target: 569 lines (+76/-191)
8 files modified
CMakeLists.txt (+2/-2)
src/SlingshotView.vala (+59/-105)
src/Widgets/AppEntry.vala (+1/-2)
src/Widgets/CategoryView.vala (+1/-1)
src/Widgets/Grid.vala (+2/-0)
src/Widgets/LargeSearchEntry.vala (+0/-73)
src/Widgets/SearchItem.vala (+1/-2)
src/Widgets/SearchView.vala (+10/-6)
To merge this branch: bzr merge lp:~tintou/slingshot/consistent-searchbar
Reviewer Review Type Date Requested Status
Tom Beckmann (community) Approve on 2014-10-14
Daniel Fore ux 2014-10-14 Approve on 2014-10-14
Review via email: mp+238334@code.launchpad.net

Commit message

* Get a consistent search entry.
 * Do search asynchronously.
 * Animate the view change with recent Gtk technology.
 * Simplify the code regarding the addition of margins.

Description of the change

 * Get a consistent search entry.
 * Do search asynchronously.
 * Anymate the view change with recent Gtk technology.
 * Simplify the code regarding the addition of margins.

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

* If I don't have the search entry focused and start typing, focus will switch the entry, but the first character will be missing. If we can fix this, I think we also shouldn't focus the search entry by default so that the hint text will be shown.

* We seem to have lost the color distinction between :focus and :hover. Is that a theme issue?

* I love the animations and the margins look much better. This is amazing. The transition between the views is very smooth.

464. By Corentin Noël on 2014-10-14

Some fixes.

Daniel Fore (danrabbit) wrote :

UX approve. Fantastic :)

review: Approve (ux)
465. By Corentin Noël on 2014-10-14

Restrict results to 20.

466. By Corentin Noël on 2014-10-14

Refinements.

Tom Beckmann (tombeckmann) wrote :

Great changes!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2014-10-05 03:06:34 +0000
3+++ CMakeLists.txt 2014-10-14 20:19:51 +0000
4@@ -78,7 +78,6 @@
5 src/Backend/SynapseSearch.vala
6 src/Widgets/AppEntry.vala
7 src/Widgets/Grid.vala
8- src/Widgets/LargeSearchEntry.vala
9 src/Widgets/Switcher.vala
10 src/Widgets/SearchView.vala
11 src/Widgets/SearchItem.vala
12@@ -93,6 +92,7 @@
13 vapi/config.vapi
14 OPTIONS
15 --thread
16+ --target-glib=2.32
17 --vapidir=${CMAKE_BINARY_DIR}/lib/synapse-core
18 --vapidir=${CMAKE_BINARY_DIR}/lib/synapse-plugins
19 -g
20@@ -127,4 +127,4 @@
21 add_schema ("org.pantheon.desktop.slingshot.gschema.xml")
22
23 # Translations
24-add_subdirectory (po)
25+add_subdirectory (po)
26\ No newline at end of file
27
28=== modified file 'src/SlingshotView.vala'
29--- src/SlingshotView.vala 2014-10-14 01:30:37 +0000
30+++ src/SlingshotView.vala 2014-10-14 20:19:51 +0000
31@@ -27,10 +27,10 @@
32 public class SlingshotView : Granite.Widgets.PopOver {
33
34 // Widgets
35- public Gtk.SearchEntry dummy_search_entry;
36- public Widgets.LargeSearchEntry real_search_entry;
37+ public Gtk.SearchEntry search_entry;
38 public Gtk.Stack stack;
39 public Granite.Widgets.ModeButton view_selector;
40+ private Gtk.Revealer view_selector_revealer;
41
42 // Views
43 private Widgets.Grid grid_view;
44@@ -38,7 +38,6 @@
45 private Widgets.CategoryView category_view;
46
47 public Gtk.Grid top;
48- public Gtk.Grid center;
49 public Gtk.Grid container;
50 public Gtk.Stack main_stack;
51 public Gtk.Box content_area;
52@@ -109,7 +108,7 @@
53 }
54
55 public int calculate_grid_width () {
56- return (int) default_columns * Pixels.ITEM_SIZE;
57+ return (int) default_columns * Pixels.ITEM_SIZE + 24;
58 }
59
60 private void setup_size () {
61@@ -139,18 +138,19 @@
62
63 // Create the base container
64 container = new Gtk.Grid ();
65-
66- main_stack = new Gtk.Stack ();
67-
68- main_stack.add_named (container, "apps");
69+ container.row_spacing = 12;
70
71 // Add top bar
72 top = new Gtk.Grid ();
73-
74- var top_separator = new Gtk.Label (""); // A fake label
75- top_separator.set_hexpand(true);
76+ top.orientation = Gtk.Orientation.HORIZONTAL;
77+ top.margin_left = 12;
78+ top.margin_right = 12;
79
80 view_selector = new Granite.Widgets.ModeButton ();
81+ view_selector.margin_right = 12;
82+ view_selector_revealer = new Gtk.Revealer ();
83+ view_selector_revealer.transition_type = Gtk.RevealerTransitionType.SLIDE_RIGHT;
84+ view_selector_revealer.add (view_selector);
85
86 var image = new Gtk.Image.from_icon_name ("view-grid-symbolic", Gtk.IconSize.MENU);
87 image.tooltip_text = _("View as Grid");
88@@ -165,61 +165,45 @@
89 else
90 view_selector.selected = 0;
91
92- dummy_search_entry = new Gtk.SearchEntry ();
93- dummy_search_entry.placeholder_text = _("Search Apps");
94- dummy_search_entry.width_request = 250;
95- dummy_search_entry.button_press_event.connect ((e) => {return e.button == 3;});
96+ search_entry = new Gtk.SearchEntry ();
97+ search_entry.placeholder_text = _("Search Apps");
98+ search_entry.hexpand = true;
99+ search_entry.button_press_event.connect ((e) => {return e.button == 3;});
100
101 if (Slingshot.settings.show_category_filter) {
102- top.attach (view_selector, 0, 0, 1, 1);
103+ top.add (view_selector_revealer);
104 }
105- top.attach (top_separator, 1, 0, 1, 1);
106- top.attach (dummy_search_entry, 2, 0, 1, 1);
107-
108- center = new Gtk.Grid ();
109+ top.add (search_entry);
110
111 stack = new Gtk.Stack ();
112- stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
113-
114- center.attach (stack, 0, 0, 1, 1);
115+ stack.transition_type = Gtk.StackTransitionType.SLIDE_LEFT_RIGHT;
116
117 // Create the "NORMAL_VIEW"
118 var scrolled_normal = new Gtk.ScrolledWindow (null, null);
119+ scrolled_normal.set_size_request (calculate_grid_width (), calculate_grid_height ());
120 grid_view = new Widgets.Grid (default_rows, default_columns);
121 scrolled_normal.add_with_viewport (grid_view);
122-
123 stack.add_named (scrolled_normal, "normal");
124
125+ // Create the "CATEGORY_VIEW"
126+ category_view = new Widgets.CategoryView (this);
127+ category_view.set_size_request (calculate_grid_width (), calculate_grid_height ());
128+ stack.add_named (category_view, "category");
129+
130 // Create the "SEARCH_VIEW"
131- var search_view_container = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
132-
133- real_search_entry = new Widgets.LargeSearchEntry ();
134- real_search_entry.margin_left = Pixels.PADDING;
135- real_search_entry.margin_right = Pixels.PADDING;
136-
137 search_view = new Widgets.SearchView (this);
138+ search_view.set_size_request (calculate_grid_width (), calculate_grid_height ());
139 search_view.start_search.connect ((match, target) => {
140- search.begin (real_search_entry.text, match, target);
141+ search.begin (search_entry.text, match, target);
142 });
143
144- search_view_container.pack_start (real_search_entry, false);
145- search_view_container.pack_start (new Gtk.Separator (Gtk.Orientation.HORIZONTAL), false);
146- search_view_container.pack_start (search_view);
147-
148- main_stack.add_named (search_view_container, "search");
149-
150- // Create the "CATEGORY_VIEW"
151- category_view = new Widgets.CategoryView (this);
152- stack.add_named (category_view, "category");
153-
154- Utils.set_padding (top, 12, Pixels.PADDING, 12, Pixels.PADDING);
155- Utils.set_padding (center, 0, Pixels.PADDING, 12, Pixels.PADDING);
156-
157+ stack.add_named (search_view, "search");
158+
159 container.attach (top, 0, 0, 1, 1);
160- container.attach (center, 0, 1, 1, 1);
161+ container.attach (stack, 0, 1, 1, 1);
162
163 event_box = new Gtk.EventBox ();
164- event_box.add (main_stack);
165+ event_box.add (container);
166 // Add the container to the dialog's content area
167 content_area = get_content_area () as Gtk.Box;
168 content_area.pack_start (event_box);
169@@ -227,7 +211,7 @@
170 content_area.set_margin_right (SHADOW_SIZE-1);
171 content_area.set_margin_top (SHADOW_SIZE-1);
172 content_area.set_margin_bottom (SHADOW_SIZE-1);
173-
174+
175 if (Slingshot.settings.use_category)
176 set_modality (Modality.CATEGORY_VIEW);
177 else
178@@ -298,38 +282,20 @@
179 private void connect_signals () {
180
181 this.focus_in_event.connect (() => {
182- get_current_search_entry ().grab_focus ();
183+ search_entry.grab_focus ();
184 return false;
185 });
186
187 event_box.key_press_event.connect (on_key_press);
188- dummy_search_entry.key_press_event.connect (search_entry_key_press);
189- real_search_entry.widget.key_press_event.connect (search_entry_key_press);
190+ search_entry.key_press_event.connect (search_entry_key_press);
191
192- real_search_entry.search_changed.connect (() => {
193- search.begin (real_search_entry.text);
194- });
195- dummy_search_entry.search_changed.connect (() => {
196+ search_entry.search_changed.connect (() => {
197 if (modality != Modality.SEARCH_VIEW)
198 set_modality (Modality.SEARCH_VIEW);
199- });
200- dummy_search_entry.grab_focus ();
201-
202- dummy_search_entry.activate.connect (search_entry_activated);
203- real_search_entry.widget.activate.connect (search_entry_activated);
204-
205- // the focus-out event is fired as soon as the stack transition is ended
206- // at which point we're able to focus the real_search_entry
207- dummy_search_entry.focus_out_event.connect (() => {
208- real_search_entry.text = dummy_search_entry.text;
209- real_search_entry.widget.grab_focus ();
210- var cursor_pos = real_search_entry.text.length;
211- real_search_entry.widget.select_region (cursor_pos, cursor_pos);
212-
213- dummy_search_entry.text = "";
214-
215- return false;
216- });
217+ search.begin (search_entry.text);
218+ });
219+ search_entry.grab_focus ();
220+ search_entry.activate.connect (search_entry_activated);
221
222 search_view.app_launched.connect (() => hide ());
223
224@@ -433,11 +399,6 @@
225 }
226 }
227
228- public Gtk.Entry get_current_search_entry ()
229- {
230- return modality == Modality.SEARCH_VIEW ? real_search_entry.widget : dummy_search_entry;
231- }
232-
233 /*
234 Overriding the default handler results in infinite loop of error messages
235 when an input method is in use (Gtk3 bug?). Key press events are
236@@ -459,8 +420,6 @@
237 return true;
238 }
239
240- var search_entry = get_current_search_entry ();
241-
242 switch (key) {
243 case "F4":
244 if ((event.state & Gdk.ModifierType.MOD1_MASK) != 0) {
245@@ -678,6 +637,7 @@
246 if (!search_entry.has_focus) {
247 search_entry.grab_focus ();
248 search_entry.move_cursor (Gtk.MovementStep.BUFFER_ENDS, 0, false);
249+ search_entry.key_press_event (event);
250 }
251 return false;
252
253@@ -713,16 +673,20 @@
254
255 public void show_slingshot () {
256
257- dummy_search_entry.text = "";
258- real_search_entry.text = "";
259+ search_entry.text = "";
260
261 reposition ();
262 show_all ();
263 present ();
264
265- set_focus(null);
266- get_current_search_entry ().grab_focus ();
267+ set_focus (null);
268+ search_entry.grab_focus ();
269+ //This is needed in order to not animate if the previous view was the search view.
270+ view_selector_revealer.transition_type = Gtk.RevealerTransitionType.NONE;
271+ stack.transition_type = Gtk.StackTransitionType.NONE;
272 set_modality ((Modality) view_selector.selected);
273+ view_selector_revealer.transition_type = Gtk.RevealerTransitionType.SLIDE_RIGHT;
274+ stack.transition_type = Gtk.StackTransitionType.SLIDE_LEFT_RIGHT;
275 }
276
277 private void set_modality (Modality new_modality) {
278@@ -733,35 +697,25 @@
279
280 if (Slingshot.settings.use_category)
281 Slingshot.settings.use_category = false;
282- view_selector.show_all ();
283- main_stack.set_visible_child_name ("apps");
284+ view_selector_revealer.set_reveal_child (true);
285 stack.set_visible_child_name ("normal");
286
287- // change the paddings/margins back to normal
288- center.set_margin_left (Pixels.PADDING);
289- stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
290-
291- dummy_search_entry.grab_focus ();
292+ search_entry.grab_focus ();
293 break;
294
295 case Modality.CATEGORY_VIEW:
296
297 if (!Slingshot.settings.use_category)
298 Slingshot.settings.use_category = true;
299- view_selector.show_all ();
300- main_stack.set_visible_child_name ("apps");
301+ view_selector_revealer.set_reveal_child (true);
302 stack.set_visible_child_name ("category");
303
304- // remove the padding/margin on the left
305- center.set_margin_left (0);
306- stack.set_size_request (calculate_grid_width () + Pixels.PADDING, calculate_grid_height ());
307-
308- dummy_search_entry.grab_focus ();
309+ search_entry.grab_focus ();
310 break;
311
312 case Modality.SEARCH_VIEW:
313- view_selector.hide ();
314- main_stack.set_visible_child_name ("search");
315+ view_selector_revealer.set_reveal_child (false);
316+ stack.set_visible_child_name ("search");
317 break;
318
319 }
320@@ -780,7 +734,7 @@
321 // picked up. If we add an idle and recheck that the entry is indeed still
322 // empty before switching, this problem is gone.
323 Idle.add (() => {
324- if (real_search_entry.text.strip () == "")
325+ if (search_entry.text.strip () == "")
326 set_modality ((Modality) view_selector.selected);
327 return false;
328 });
329@@ -799,10 +753,10 @@
330 matches = yield synapse.search (text);
331 }
332
333- search_view.clear ();
334- search_view.set_results (matches, text);
335-
336- search_view.selected = 0;
337+ Idle.add (() => {
338+ search_view.set_results (matches, text);
339+ return false;
340+ });
341
342 }
343
344@@ -900,7 +854,7 @@
345 return;
346 }
347 else if (category_column_focus == 0 && delta_column < 0) {
348- get_current_search_entry ().grab_focus ();
349+ search_entry.grab_focus ();
350 category_column_focus = 0;
351 category_row_focus = 0;
352 return;
353
354=== modified file 'src/Widgets/AppEntry.vala'
355--- src/Widgets/AppEntry.vala 2014-10-05 03:06:34 +0000
356+++ src/Widgets/AppEntry.vala 2014-10-14 20:19:51 +0000
357@@ -36,7 +36,6 @@
358 private Backend.App application;
359
360 public AppEntry (Backend.App app) {
361- this.relief = Gtk.ReliefStyle.NONE;
362 Gtk.TargetEntry dnd = {"text/uri-list", 0, 0};
363 Gtk.drag_source_set (this, Gdk.ModifierType.BUTTON1_MASK, {dnd},
364 Gdk.DragAction.COPY);
365@@ -112,4 +111,4 @@
366 application.launch ();
367 app_launched ();
368 }
369-}
370+}
371\ No newline at end of file
372
373=== modified file 'src/Widgets/CategoryView.vala'
374--- src/Widgets/CategoryView.vala 2014-08-14 20:44:46 +0000
375+++ src/Widgets/CategoryView.vala 2014-10-14 20:19:51 +0000
376@@ -124,7 +124,7 @@
377 else
378 page_switcher.hide ();
379
380- view.get_current_search_entry ().grab_focus ();
381+ view.search_entry.grab_focus ();
382
383 }
384
385
386=== modified file 'src/Widgets/Grid.vala'
387--- src/Widgets/Grid.vala 2014-08-14 20:09:40 +0000
388+++ src/Widgets/Grid.vala 2014-10-14 20:19:51 +0000
389@@ -37,6 +37,8 @@
390 private Page page;
391
392 public Grid (int rows, int columns) {
393+ margin_left = 12;
394+ margin_right = 12;
395 page.rows = rows;
396 page.columns = columns;
397 page.number = 1;
398
399=== removed file 'src/Widgets/LargeSearchEntry.vala'
400--- src/Widgets/LargeSearchEntry.vala 2014-06-10 10:30:05 +0000
401+++ src/Widgets/LargeSearchEntry.vala 1970-01-01 00:00:00 +0000
402@@ -1,73 +0,0 @@
403-// -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
404-//
405-// Copyright (C) 2014 Tom Beckmann <tomjonabc@gmail.com>
406-//
407-// This program is free software: you can redistribute it and/or modify
408-// it under the terms of the GNU General Public License as published by
409-// the Free Software Foundation, either version 3 of the License, or
410-// (at your option) any later version.
411-//
412-// This program is distributed in the hope that it will be useful,
413-// but WITHOUT ANY WARRANTY; without even the implied warranty of
414-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
415-// GNU General Public License for more details.
416-//
417-// You should have received a copy of the GNU General Public License
418-// along with this program. If not, see <http://www.gnu.org/licenses/>.
419-//
420-
421-namespace Slingshot.Widgets
422-{
423- public class LargeSearchEntry : Gtk.Box {
424- const int ICON_SIZE = 24;
425-
426- public signal void search_changed ();
427-
428- public string text {
429- get {
430- return widget.text;
431- }
432- set {
433- widget.text = value;
434- }
435- }
436-
437- public Gtk.Entry widget { get; private set; }
438-
439- public LargeSearchEntry ()
440- {
441- Object (orientation: Gtk.Orientation.HORIZONTAL, spacing: 4);
442-
443- var find_pixbuf = get_icon ("edit-find-symbolic");
444- var clear_pixbuf = get_icon ("edit-clear-symbolic");
445-
446- var clear_box = new Gtk.Button ();
447- clear_box.add (new Gtk.Image.from_pixbuf (clear_pixbuf));
448- clear_box.relief = Gtk.ReliefStyle.NONE;
449- clear_box.clicked.connect (() => text = "" );
450-
451- widget = new Gtk.Entry ();
452- widget.get_style_context ().add_class ("search-entry-large");
453-
454- pack_start (new Gtk.Image.from_pixbuf (find_pixbuf), false);
455- pack_start (widget);
456- pack_start (clear_box, false);
457-
458- widget.changed.connect (() => search_changed ());
459- }
460-
461- Gdk.Pixbuf? get_icon (string name)
462- {
463- Gdk.Pixbuf? pixbuf = null;
464- var info = Gtk.IconTheme.get_default ().lookup_icon (name, ICON_SIZE, 0);
465- try {
466- if (info != null)
467- pixbuf = info.load_symbolic ({ 0.5, 0.5, 0.5, 1 });
468- } catch (Error e) {}
469-
470- return pixbuf;
471- }
472-
473- }
474-}
475-
476
477=== modified file 'src/Widgets/SearchItem.vala'
478--- src/Widgets/SearchItem.vala 2014-06-12 09:14:57 +0000
479+++ src/Widgets/SearchItem.vala 2014-10-14 20:19:51 +0000
480@@ -35,7 +35,6 @@
481 Object (app: app);
482
483 get_style_context ().add_class ("app");
484- get_style_context ().add_class ("search-item");
485
486 var markup = Backend.SynapseSearch.markup_string_with_search (app.name, search_term);
487
488@@ -79,4 +78,4 @@
489 }
490 }
491
492-}
493+}
494\ No newline at end of file
495
496=== modified file 'src/Widgets/SearchView.vala'
497--- src/Widgets/SearchView.vala 2014-06-12 09:14:57 +0000
498+++ src/Widgets/SearchView.vala 2014-10-14 20:19:51 +0000
499@@ -21,6 +21,7 @@
500 public class SearchView : Gtk.ScrolledWindow {
501 const int CONTEXT_WIDTH = 200;
502 const int CONTEXT_ARROW_SIZE = 12;
503+ const int MAX_RESULTS = 20;
504 const int MAX_RESULTS_BEFORE_LIMIT = 10;
505
506 public signal void start_search (Synapse.SearchMatch search_match, Synapse.Match? target);
507@@ -90,9 +91,10 @@
508 items = new Gee.HashMap<Backend.App, SearchItem> ();
509
510 main_box = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
511+ main_box.margin_left = 12;
512+ main_box.margin_right = 12;
513
514 context_box = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
515- context_box.width_request = CONTEXT_WIDTH;
516 context_fixed = new Gtk.Fixed ();
517 context_fixed.margin_left = CONTEXT_ARROW_SIZE;
518 context_fixed.put (context_box, 0, 0);
519@@ -103,7 +105,6 @@
520 revealer = new Gtk.Revealer ();
521 revealer.transition_duration = 400;
522 revealer.transition_type = Gtk.RevealerTransitionType.CROSSFADE;
523- revealer.width_request = CONTEXT_WIDTH + CONTEXT_ARROW_SIZE;
524 revealer.no_show_all = true;
525 revealer.add (context);
526
527@@ -149,7 +150,7 @@
528 // if we're showing more than about 10 results and we have more than
529 // categories, we limit the results per category to the most relevant
530 // ones.
531- var limit = int.MAX;
532+ var limit = MAX_RESULTS;
533 if (matches.size + 3 > MAX_RESULTS_BEFORE_LIMIT && categories_order.size > 2)
534 limit = 5;
535
536@@ -185,14 +186,16 @@
537
538 var header = new Gtk.Label (label);
539 header.xalign = 0;
540- header.margin_left = header.margin_top = 8;
541+ header.margin_left = 8;
542 header.margin_bottom = 4;
543 header.use_markup = true;
544+ header.get_style_context ().add_class ("category-label");
545 header.show ();
546- header.get_style_context ().add_class ("search-category-header");
547 main_box.pack_start (header, false);
548
549 var list = categories.get (type);
550+ var old_selected = selected;
551+ clear ();
552 for (var i = 0; i < limit && i < list.size; i++) {
553 var match = list.get (i);
554
555@@ -208,6 +211,7 @@
556 n_results++;
557 }
558 }
559+ selected = old_selected;
560 }
561 }
562
563@@ -352,4 +356,4 @@
564
565 }
566
567-}
568+}
569\ No newline at end of file

Subscribers

People subscribed via source and target branches