Merge lp:~julien-spautz/slingshot/magic-numbers into lp:~elementary-pantheon/slingshot/trunk

Proposed by Julien Spautz on 2014-08-14
Status: Merged
Approved by: Cody Garver on 2014-10-05
Approved revision: 443
Merged at revision: 455
Proposed branch: lp:~julien-spautz/slingshot/magic-numbers
Merge into: lp:~elementary-pantheon/slingshot/trunk
Diff against target: 415 lines (+62/-64)
7 files modified
CMakeLists.txt (+1/-1)
src/Pixels.vala (+9/-0)
src/SlingshotView.vala (+39/-32)
src/Widgets/AppEntry.vala (+1/-1)
src/Widgets/CategoryView.vala (+2/-5)
src/Widgets/Grid.vala (+6/-21)
src/Widgets/Sidebar.vala (+4/-4)
To merge this branch: bzr merge lp:~julien-spautz/slingshot/magic-numbers
Reviewer Review Type Date Requested Status
elementary Pantheon team 2014-08-14 Pending
Review via email: mp+230886@code.launchpad.net

Commit message

Centralize and clean up magic numbers of dimensions, padding, spacing, etc. Also fixes bug #1016154.

Description of the change

I removed a few magic numbers related to padding and replaced them with named constants.

This also fixes some alignment and padding issues, such as the one pixel margin on the left in category view (bug #1016154), the horizontal resizing when switching between grid and category view, and the slight shift of the grid items when switching between grid and category view.

Also I think the padding on the right and left side were not identical in trunk.

To post a comment you must log in.
442. By spautz-julien on 2014-08-14

replaced another magic number

Cody Garver (codygarver) wrote :

src/SlingshotView.vala:763.25-763.58: warning: local variable `content_area' declared but never used
                    var content_area = get_content_area ();
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

443. By spautz-julien on 2014-08-15

removed superfluous line

Julien Spautz (julien-spautz) wrote :

fixed

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-06-12 09:36:00 +0000
3+++ CMakeLists.txt 2014-08-15 01:04:52 +0000
4@@ -70,6 +70,7 @@
5 src/SlingshotView.vala
6 src/Settings.vala
7 src/Utils.vala
8+ src/Pixels.vala
9 src/Backend/AppSystem.vala
10 src/Backend/DBusService.vala
11 src/Backend/App.vala
12@@ -127,4 +128,3 @@
13
14 # Translations
15 add_subdirectory (po)
16-
17
18=== added file 'src/Pixels.vala'
19--- src/Pixels.vala 1970-01-01 00:00:00 +0000
20+++ src/Pixels.vala 2014-08-15 01:04:52 +0000
21@@ -0,0 +1,9 @@
22+namespace Slingshot.Pixels {
23+
24+ const int PADDING = 17;
25+ const int ROW_SPACING = 20;
26+ const int ITEM_SIZE = 130;
27+ const int BOTTOM_SPACE = 180;
28+ const int SIDEBAR_GRID_PADDING = 5;
29+ const int SIDEBAR_WIDTH = PADDING + ITEM_SIZE - SIDEBAR_GRID_PADDING - 1;
30+}
31\ No newline at end of file
32
33=== modified file 'src/SlingshotView.vala'
34--- src/SlingshotView.vala 2014-07-10 09:40:47 +0000
35+++ src/SlingshotView.vala 2014-08-15 01:04:52 +0000
36@@ -64,15 +64,10 @@
37 return grid_view.get_page_rows ();
38 }
39 }
40+
41 private int default_columns;
42 private int default_rows;
43
44- public int view_height {
45- get {
46- return (int) (rows * 130 + rows * grid_view.row_spacing + 35);
47- }
48- }
49-
50 private int column_focus = 0;
51 private int row_focus = 0;
52
53@@ -100,12 +95,21 @@
54
55 if (Slingshot.settings.screen_resolution != @"$(screen.get_width ())x$(screen.get_height ())")
56 setup_size ();
57- height_request = default_rows * 145 + 180;
58+
59+ height_request = calculate_grid_height () + Pixels.BOTTOM_SPACE;
60 setup_ui ();
61 connect_signals ();
62
63 debug ("Apps loaded");
64-
65+ }
66+
67+ public int calculate_grid_height () {
68+ return (int) (default_rows * Pixels.ITEM_SIZE +
69+ (default_rows - 1) * Pixels.ROW_SPACING);
70+ }
71+
72+ public int calculate_grid_width () {
73+ return (int) default_columns * Pixels.ITEM_SIZE;
74 }
75
76 private void setup_size () {
77@@ -114,11 +118,11 @@
78 Slingshot.settings.screen_resolution = @"$(screen.get_width ())x$(screen.get_height ())";
79 default_columns = 5;
80 default_rows = 3;
81- while ((default_columns * 130 + 48 >= 2 * screen.get_width () / 3)) {
82+ while ((calculate_grid_width () + 2 * Pixels.PADDING >= 2 * screen.get_width () / 3)) {
83 default_columns--;
84 }
85
86- while ((default_rows * 145 + 72 >= 2 * screen.get_height () / 3)) {
87+ while ((calculate_grid_height () + Pixels.BOTTOM_SPACE >= 2 * screen.get_height () / 3)) {
88 default_rows--;
89 }
90
91@@ -173,22 +177,25 @@
92 top.attach (dummy_search_entry, 2, 0, 1, 1);
93
94 center = new Gtk.Grid ();
95-
96+
97 stack = new Gtk.Stack ();
98- stack.set_size_request (default_columns * 130, default_rows * 145);
99+ stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
100+
101 center.attach (stack, 0, 0, 1, 1);
102
103 // Create the "NORMAL_VIEW"
104 var scrolled_normal = new Gtk.ScrolledWindow (null, null);
105 grid_view = new Widgets.Grid (default_rows, default_columns);
106 scrolled_normal.add_with_viewport (grid_view);
107+
108 stack.add_named (scrolled_normal, "normal");
109
110 // Create the "SEARCH_VIEW"
111 var search_view_container = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
112
113 real_search_entry = new Widgets.LargeSearchEntry ();
114- real_search_entry.margin_left = real_search_entry.margin_right = 12;
115+ real_search_entry.margin_left = Pixels.PADDING;
116+ real_search_entry.margin_right = Pixels.PADDING;
117
118 search_view = new Widgets.SearchView (this);
119 search_view.start_search.connect ((match, target) => {
120@@ -204,16 +211,23 @@
121 // Create the "CATEGORY_VIEW"
122 category_view = new Widgets.CategoryView (this);
123 stack.add_named (category_view, "category");
124-
125- container.attach (Utils.set_padding (top, 12, 12, 12, 12), 0, 0, 1, 1);
126- container.attach (Utils.set_padding (center, 0, 12, 12, 12), 0, 1, 1, 1);
127+
128+ Utils.set_padding (top, 12, Pixels.PADDING, 12, Pixels.PADDING);
129+ Utils.set_padding (center, 0, Pixels.PADDING, 12, Pixels.PADDING);
130+
131+ container.attach (top, 0, 0, 1, 1);
132+ container.attach (center, 0, 1, 1, 1);
133
134 event_box = new Gtk.EventBox ();
135 event_box.add (main_stack);
136 // Add the container to the dialog's content area
137 content_area = get_content_area () as Gtk.Box;
138 content_area.pack_start (event_box);
139-
140+ content_area.set_margin_left (SHADOW_SIZE-1);
141+ content_area.set_margin_right (SHADOW_SIZE-1);
142+ content_area.set_margin_top (SHADOW_SIZE-1);
143+ content_area.set_margin_bottom (SHADOW_SIZE-1);
144+
145 if (Slingshot.settings.use_category)
146 set_modality (Modality.CATEGORY_VIEW);
147 else
148@@ -234,8 +248,8 @@
149 null, Gdk.CURRENT_TIME);
150 }
151
152- var pointer_status = pointer.grab (get_window (), Gdk.GrabOwnership.NONE, true,
153- Gdk.EventMask.SMOOTH_SCROLL_MASK | Gdk.EventMask.BUTTON_PRESS_MASK |
154+ var pointer_status = pointer.grab (get_window (), Gdk.GrabOwnership.NONE, true,
155+ Gdk.EventMask.SMOOTH_SCROLL_MASK | Gdk.EventMask.BUTTON_PRESS_MASK |
156 Gdk.EventMask.BUTTON_RELEASE_MASK | Gdk.EventMask.POINTER_MOTION_MASK,
157 null, Gdk.CURRENT_TIME);
158
159@@ -362,7 +376,7 @@
160 can_trigger_hotcorner = false;
161 }
162 }
163-
164+
165 private void reposition (bool show=true) {
166
167 debug("Repositioning");
168@@ -449,7 +463,7 @@
169 if ((event.state & Gdk.ModifierType.MOD1_MASK) != 0) {
170 hide ();
171 }
172-
173+
174 break;
175
176 case "Escape":
177@@ -721,10 +735,8 @@
178 stack.set_visible_child_name ("normal");
179
180 // change the paddings/margins back to normal
181- get_content_area ().set_margin_left (PADDINGS.left + SHADOW_SIZE + 5);
182- center.set_margin_left (12);
183- top.set_margin_left (12);
184- stack.set_size_request (default_columns * 130, default_rows * 145);
185+ center.set_margin_left (Pixels.PADDING);
186+ stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
187
188 dummy_search_entry.grab_focus ();
189 break;
190@@ -738,10 +750,8 @@
191 stack.set_visible_child_name ("category");
192
193 // remove the padding/margin on the left
194- get_content_area ().set_margin_left (PADDINGS.left + SHADOW_SIZE);
195 center.set_margin_left (0);
196- top.set_margin_left (17);
197- stack.set_size_request (default_columns * 130 + 17, default_rows * 145);
198+ stack.set_size_request (calculate_grid_width () + Pixels.PADDING, calculate_grid_height ());
199
200 dummy_search_entry.grab_focus ();
201 break;
202@@ -749,9 +759,6 @@
203 case Modality.SEARCH_VIEW:
204 view_selector.hide ();
205 main_stack.set_visible_child_name ("search");
206-
207- var content_area = get_content_area ();
208- content_area.margin_left = content_area.margin_right = SHADOW_SIZE - 1;
209 break;
210
211 }
212@@ -831,7 +838,7 @@
213 if (!first_start) {
214 grid_view.resize (default_rows, default_columns);
215 populate_grid_view ();
216- height_request = default_rows * 145 + 180;
217+ height_request = calculate_grid_height () + Pixels.BOTTOM_SPACE;
218
219 category_view.app_view.resize (default_rows, default_columns);
220 category_view.show_filtered_apps (category_view.category_ids.get (category_view.category_switcher.selected));
221
222=== modified file 'src/Widgets/AppEntry.vala'
223--- src/Widgets/AppEntry.vala 2014-05-26 06:11:39 +0000
224+++ src/Widgets/AppEntry.vala 2014-08-15 01:04:52 +0000
225@@ -42,7 +42,7 @@
226
227 app_paintable = true;
228 set_visual (get_screen ().get_rgba_visual());
229- set_size_request (130, 130);
230+ set_size_request (Pixels.ITEM_SIZE, Pixels.ITEM_SIZE);
231 desktop_id = app.desktop_id;
232 desktop_path = app.desktop_path;
233
234
235=== modified file 'src/Widgets/CategoryView.vala'
236--- src/Widgets/CategoryView.vala 2014-06-08 11:08:37 +0000
237+++ src/Widgets/CategoryView.vala 2014-08-15 01:04:52 +0000
238@@ -42,9 +42,6 @@
239 setup_ui ();
240 setup_sidebar ();
241 connect_events ();
242-
243- set_size_request (view.columns*130 + 17, view.view_height);
244-
245 }
246
247 private void setup_ui () {
248@@ -52,7 +49,7 @@
249 separator = new Gtk.Separator (Gtk.Orientation.VERTICAL);
250
251 app_view = new Widgets.Grid (view.rows, view.columns - 1);
252- app_view.margin_left = 5;
253+ app_view.margin_left = Pixels.SIDEBAR_GRID_PADDING;
254
255 container.attach (separator, 1, 0, 1, 2);
256 container.attach (app_view, 2, 0, 1, 1);
257@@ -131,4 +128,4 @@
258
259 }
260
261-}
262+}
263\ No newline at end of file
264
265=== modified file 'src/Widgets/Grid.vala'
266--- src/Widgets/Grid.vala 2014-02-24 20:43:38 +0000
267+++ src/Widgets/Grid.vala 2014-08-15 01:04:52 +0000
268@@ -25,7 +25,7 @@
269 }
270
271 public class Grid : Gtk.Box {
272- public int row_spacing = 20;
273+
274 public Widgets.Switcher page_switcher;
275
276 private Gtk.Stack stack;
277@@ -69,7 +69,7 @@
278 current_grid.row_homogeneous = true;
279 current_grid.column_homogeneous = true;
280
281- current_grid.row_spacing = row_spacing;
282+ current_grid.row_spacing = Pixels.ROW_SPACING;
283 current_grid.column_spacing = 0;
284 grids.set (page.number, current_grid);
285 stack.add_titled (current_grid, page.number.to_string (), page.number.to_string ());
286@@ -79,7 +79,6 @@
287 }
288
289 public void append (Gtk.Widget widget) {
290-
291 update_position ();
292
293 current_grid.attach (widget, (int)current_col, (int)current_row, 1, 1);
294@@ -88,7 +87,6 @@
295 }
296
297 private void update_position () {
298-
299 if (current_col == page.columns) {
300 current_col = 0;
301 current_row++;
302@@ -99,11 +97,9 @@
303 create_new_grid ();
304 current_row = 0;
305 }
306-
307 }
308
309 public void clear () {
310-
311 foreach (Gtk.Grid grid in grids.values) {
312 foreach (Gtk.Widget widget in grid.get_children ()) {
313 widget.destroy ();
314@@ -117,7 +113,6 @@
315 page.number = 1;
316 create_new_grid ();
317 stack.set_visible_child (current_grid);
318-
319 }
320
321 public Gtk.Widget get_child_at (int column, int row) {
322@@ -128,21 +123,15 @@
323 }
324
325 public int get_page_columns () {
326-
327 return (int) page.columns;
328-
329 }
330
331 public int get_page_rows () {
332-
333 return (int) page.rows;
334-
335 }
336
337 public int get_n_pages () {
338-
339 return (int) page.number;
340-
341 }
342
343 public int get_current_page () {
344@@ -152,7 +141,7 @@
345 public void go_to_next () {
346 int page_number = get_current_page ()+1;
347 if (page_number <= get_n_pages ())
348- stack.set_visible_child_name ("%d".printf (page_number));
349+ stack.set_visible_child_name (page_number.to_string ());
350
351 page_switcher.update_selected ();
352 }
353@@ -160,30 +149,26 @@
354 public void go_to_previous () {
355 int page_number = get_current_page ()-1;
356 if (page_number > 0)
357- stack.set_visible_child_name ("%d".printf (page_number));
358+ stack.set_visible_child_name (page_number.to_string ());
359
360 page_switcher.update_selected ();
361 }
362
363 public void go_to_last () {
364- stack.set_visible_child_name ("%d".printf (get_n_pages ()));
365+ stack.set_visible_child_name (get_n_pages ().to_string ());
366 page_switcher.update_selected ();
367 }
368
369 public void go_to_number (int number) {
370- stack.set_visible_child_name ("%d".printf (number));
371+ stack.set_visible_child_name (number.to_string ());
372 page_switcher.update_selected ();
373 }
374
375 public void resize (int rows, int columns) {
376-
377 clear ();
378 page.rows = rows;
379 page.columns = columns;
380 page.number = 1;
381-
382 }
383-
384 }
385-
386 }
387\ No newline at end of file
388
389=== modified file 'src/Widgets/Sidebar.vala'
390--- src/Widgets/Sidebar.vala 2013-12-26 00:08:04 +0000
391+++ src/Widgets/Sidebar.vala 2014-08-15 01:04:52 +0000
392@@ -59,13 +59,13 @@
393 set_show_expanders (false);
394 set_level_indentation (8);
395
396- set_size_request (145, -1);
397+ set_size_request (Pixels.SIDEBAR_WIDTH, -1);
398 get_style_context ().add_class ("sidebar");
399
400 var cell = new Gtk.CellRendererText ();
401 cell.wrap_mode = Pango.WrapMode.WORD;
402- cell.wrap_width = 110;
403- cell.xpad = 17;
404+ cell.wrap_width = Pixels.SIDEBAR_WIDTH - 2 * Pixels.PADDING;
405+ cell.xpad = Pixels.PADDING;
406
407 insert_column_with_attributes (-1, "Filters", cell, "markup", Columns.TEXT);
408
409@@ -130,4 +130,4 @@
410
411 }
412
413-}
414+}
415\ No newline at end of file

Subscribers

People subscribed via source and target branches