Merge lp:~rastersoft-gmail/slingshot/fixed_width_fix into lp:~elementary-pantheon/slingshot/trunk

Proposed by Sergio Costas on 2014-08-30
Status: Rejected
Rejected by: Cody Garver on 2016-05-02
Proposed branch: lp:~rastersoft-gmail/slingshot/fixed_width_fix
Merge into: lp:~elementary-pantheon/slingshot/trunk
Diff against target: 384 lines (+119/-63)
6 files modified
CMakeLists.txt (+1/-0)
src/SlingshotView.vala (+18/-22)
src/Widgets/AppEntry.vala (+31/-26)
src/Widgets/Grid.vala (+21/-14)
src/Widgets/LabelFixedWidth.vala (+36/-0)
src/Widgets/Switcher.vala (+12/-1)
To merge this branch: bzr merge lp:~rastersoft-gmail/slingshot/fixed_width_fix
Reviewer Review Type Date Requested Status
Daniel Fore 2014-08-30 Needs Fixing on 2014-08-31
Review via email: mp+232790@code.launchpad.net

Description of the change

Slingshot, by default, presumes that the user has installed and is using the Elementary OS theme. It also presumes that the language used is english. This is because it has hardcoded several sizes for icons, texts and so on that only are correct in that case.

When using slingshot in other languages, the cathegory list string can be larger, which results in the icons in the last column being cut. Also, when using a different theme, the font size and other metrics change, which results in a bigger cut.

This patch fixes this by creating a new label widget that can keep an specific width, instead of growing without control like the standard ones in Gtk. This allows to remove all the fixed widths and heights in the code, and rely on the automatic Gtk layout system. This also allows to use more than one line in the application name, removing the need for text ellipsing.

To post a comment you must log in.
448. By Sergio Costas on 2014-08-30

Fixed code style

Sergio Costas (rastersoft-gmail) wrote :

Fixed code style. Now it should comply.

Daniel Fore (danrabbit) wrote :

I don't see the app presuming that our theme is shipped as a flaw. That is intended and many of our apps depend on features specific to our theme.

However, the only issues I can see are:

The padding added to the left of the category view needs to be removed. That pane should be 0px from the edge of the popover. This would probably fix the issue of the popover growing when you change views.

There should be some 12px padding between the icon grid and the view switcher

review: Needs Fixing
Sergio Costas (rastersoft-gmail) wrote :

Presuming an specific theme is not the flaw itself; the problem is that, by setting fixed sizes for everything, under some circumstances the layout fails. One is when the user changes the theme, which I understand that you don't care about; but the other is when the user sets its own language (spanish in my case) and one cathegory is much longer than in english. You can see an example here:

http://www.rastersoft.com/blogpic/slingshot_size_bug.png

As you can see there, I'm using Elementary OS with the Elementary theme; "system tools" is translated as "herramientas del sistema", but it is much longer, which causes that the icon column at right gets cut. This patch also fixes this, because it removes the fixed sizes (except for the launch buttons, which remains at 130x130).

I'll try to fix the paddings you comment.

Julián Unrrein (junrrein) wrote :

If spacing issues are solved by this branch, we can discard https://code.launchpad.net/~julien-spautz/slingshot/magic-numbers/+merge/230886.

449. By Sergio Costas on 2014-08-31

Fixed layout margins

450. By Sergio Costas on 2014-08-31

Added some comments

Sergio Costas (rastersoft-gmail) wrote :

Ok, it should be fixed now. Please, check it and tell me if it is fine now, or still needs more changes.

Now also keeps the same height even when the pager is hiden.

451. By Sergio Costas on 2014-09-01

Removed unnecesary copyright notes

Sergio Costas (rastersoft-gmail) wrote :

Done.

452. By Sergio Costas on 2014-09-01

Merged current branch

Sergio Costas (rastersoft-gmail) wrote :

Fixed conflict.

453. By Sergio Costas on 2014-10-11

Merged changes in ru.po

454. By Sergio Costas on 2014-10-11

Merged last changes. Ready again to be merged to master.

Unmerged revisions

454. By Sergio Costas on 2014-10-11

Merged last changes. Ready again to be merged to master.

453. By Sergio Costas on 2014-10-11

Merged changes in ru.po

452. By Sergio Costas on 2014-09-01

Merged current branch

451. By Sergio Costas on 2014-09-01

Removed unnecesary copyright notes

450. By Sergio Costas on 2014-08-31

Added some comments

449. By Sergio Costas on 2014-08-31

Fixed layout margins

448. By Sergio Costas on 2014-08-30

Fixed code style

447. By Sergio Costas on 2014-08-30

Now the labels are aligned

446. By Sergio Costas on 2014-08-30

The Gtk.Fixed is not needed.

445. By Sergio Costas on 2014-08-30

Update merge from trunk

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-11 19:03:29 +0000
4@@ -84,6 +84,7 @@
5 src/Widgets/SearchItem.vala
6 src/Widgets/Sidebar.vala
7 src/Widgets/CategoryView.vala
8+ src/Widgets/LabelFixedWidth.vala
9 PACKAGES
10 ${CORE_DEPS}
11 ${UI_DEPS}
12
13=== modified file 'src/SlingshotView.vala'
14--- src/SlingshotView.vala 2014-10-05 03:06:34 +0000
15+++ src/SlingshotView.vala 2014-10-11 19:03:29 +0000
16@@ -95,8 +95,6 @@
17
18 if (Slingshot.settings.screen_resolution != @"$(screen.get_width ())x$(screen.get_height ())")
19 setup_size ();
20-
21- height_request = calculate_grid_height () + Pixels.BOTTOM_SPACE;
22 setup_ui ();
23 connect_signals ();
24
25@@ -114,21 +112,26 @@
26
27 private void setup_size () {
28
29+ int max_width;
30+ int max_height;
31+
32 debug ("In setup_size ()");
33 Slingshot.settings.screen_resolution = @"$(screen.get_width ())x$(screen.get_height ())";
34 default_columns = 5;
35 default_rows = 3;
36- while ((calculate_grid_width () + 2 * Pixels.PADDING >= 2 * screen.get_width () / 3)) {
37+
38+ max_width = 2 * screen.get_width () / 3;
39+ max_height = 2 * screen.get_height () / 3;
40+
41+ while ((calculate_grid_width () + 2 * Pixels.PADDING) >= max_width)
42 default_columns--;
43- }
44
45- while ((calculate_grid_height () + Pixels.BOTTOM_SPACE >= 2 * screen.get_height () / 3)) {
46+ while ((calculate_grid_height () + Pixels.BOTTOM_SPACE) >= max_height)
47 default_rows--;
48- }
49
50- if (Slingshot.settings.columns != default_columns) {
51+ if (Slingshot.settings.columns != default_columns)
52 Slingshot.settings.columns = default_columns;
53- }
54+
55 if (Slingshot.settings.rows != default_rows)
56 Slingshot.settings.rows = default_rows;
57 }
58@@ -179,16 +182,12 @@
59 center = new Gtk.Grid ();
60
61 stack = new Gtk.Stack ();
62- stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
63-
64+ stack.homogeneous = false;
65 center.attach (stack, 0, 0, 1, 1);
66
67 // Create the "NORMAL_VIEW"
68- var scrolled_normal = new Gtk.ScrolledWindow (null, null);
69 grid_view = new Widgets.Grid (default_rows, default_columns);
70- scrolled_normal.add_with_viewport (grid_view);
71-
72- stack.add_named (scrolled_normal, "normal");
73+ stack.add_named (grid_view, "normal");
74
75 // Create the "SEARCH_VIEW"
76 var search_view_container = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
77@@ -222,12 +221,12 @@
78 event_box.add (main_stack);
79 // Add the container to the dialog's content area
80 content_area = get_content_area () as Gtk.Box;
81- content_area.pack_start (event_box);
82+ content_area.pack_start (event_box,false,false);
83 content_area.set_margin_left (SHADOW_SIZE-1);
84 content_area.set_margin_right (SHADOW_SIZE-1);
85 content_area.set_margin_top (SHADOW_SIZE-1);
86 content_area.set_margin_bottom (SHADOW_SIZE-1);
87-
88+
89 if (Slingshot.settings.use_category)
90 set_modality (Modality.CATEGORY_VIEW);
91 else
92@@ -736,11 +735,9 @@
93 view_selector.show_all ();
94 main_stack.set_visible_child_name ("apps");
95 stack.set_visible_child_name ("normal");
96-
97 // change the paddings/margins back to normal
98 center.set_margin_left (Pixels.PADDING);
99- stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
100-
101+ //stack.set_size_request (calculate_grid_width (), calculate_grid_height ());
102 dummy_search_entry.grab_focus ();
103 break;
104
105@@ -751,11 +748,10 @@
106 view_selector.show_all ();
107 main_stack.set_visible_child_name ("apps");
108 stack.set_visible_child_name ("category");
109-
110 // remove the padding/margin on the left
111 center.set_margin_left (0);
112- stack.set_size_request (calculate_grid_width () + Pixels.PADDING, calculate_grid_height ());
113
114+ top.set_margin_left (17);
115 dummy_search_entry.grab_focus ();
116 break;
117
118@@ -935,4 +931,4 @@
119 }
120 }
121
122-}
123\ No newline at end of file
124+}
125
126=== modified file 'src/Widgets/AppEntry.vala'
127--- src/Widgets/AppEntry.vala 2014-10-05 03:06:34 +0000
128+++ src/Widgets/AppEntry.vala 2014-10-11 19:03:29 +0000
129@@ -19,6 +19,7 @@
130 public class Slingshot.Widgets.AppEntry : Gtk.Button {
131
132 public Gtk.Label app_label;
133+ private Gtk.DrawingArea drawing;
134 private Gdk.Pixbuf icon;
135 private Gtk.Box layout;
136
137@@ -35,6 +36,12 @@
138
139 private Backend.App application;
140
141+ // these variables allow to tune the sizes and margins
142+ private int d_width = 130; // desired width
143+ private int d_height = 130; // desired height
144+ private int d_margin = 6; // desired margin. Must be an even value
145+ private int d_icon_label_sep = 4; // half the desired separation between the icon and the text
146+
147 public AppEntry (Backend.App app) {
148 this.relief = Gtk.ReliefStyle.NONE;
149 Gtk.TargetEntry dnd = {"text/uri-list", 0, 0};
150@@ -42,8 +49,9 @@
151 Gdk.DragAction.COPY);
152
153 app_paintable = true;
154- set_visual (get_screen ().get_rgba_visual());
155- set_size_request (Pixels.ITEM_SIZE, Pixels.ITEM_SIZE);
156+
157+ set_visual (get_screen ().get_rgba_visual ());
158+ set_size_request (this.d_width, this.d_height);
159 desktop_id = app.desktop_id;
160 desktop_path = app.desktop_path;
161
162@@ -56,19 +64,32 @@
163
164 get_style_context ().add_class ("app");
165
166- app_label = new Gtk.Label (app_name);
167- app_label.halign = Gtk.Align.CENTER;
168- app_label.justify = Gtk.Justification.CENTER;
169- app_label.set_line_wrap (true); // Need a smarter way
170- app_label.set_single_line_mode (false);
171- app_label.set_ellipsize (Pango.EllipsizeMode.END);
172+ this.app_label = new LabelFixedWidth (app_name, this.d_width - 2 * this.d_margin);
173+ this.app_label.set_alignment ((float)0.5, 0);
174+ this.app_label.justify = Gtk.Justification.CENTER;
175+ this.app_label.set_line_wrap (true); // Need a smarter way
176+ this.app_label.set_single_line_mode (false);
177+
178+ // Let's put the icon in a Gtk.DrawingArea instead of overriding the DRAW method
179+ this.drawing = new Gtk.DrawingArea ();
180+ this.drawing.set_size_request (this.d_width - 2 * this.d_margin, 64);
181+ this.drawing.draw.connect ( (w,cr) => {
182+ // This test is needed, because pycrust and XRCed kills the oficial slingshot (something happens with their icons)
183+ if (this.icon != null) {
184+ Gdk.cairo_set_source_pixbuf (cr, this.icon, (this.d_width - 2 * this.d_margin - icon.width) / 2.0, (64 - icon.height) / 2.0);
185+ cr.paint_with_alpha (alpha);
186+ }
187+ return true;
188+ });
189
190 layout = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
191 layout.homogeneous = false;
192
193- layout.pack_start (app_label, false, true, 0);
194+ layout.pack_start (this.drawing, false, false, this.d_icon_label_sep);
195+ layout.pack_start (this.app_label, true, true, this.d_icon_label_sep);
196
197- add (Utils.set_padding (layout, 78, 5, 5, 5));
198+ // The widgets (label and drawingarea) both have a margin of d_icon_label_sep, so let's add the extra margin to make them d_margin thick
199+ this.add (Utils.set_padding (layout, this.d_margin - this.d_icon_label_sep, this.d_margin - this.d_icon_label_sep, this.d_margin - this.d_icon_label_sep, this.d_margin - this.d_icon_label_sep));
200
201 this.clicked.connect (launch_app);
202
203@@ -92,22 +113,6 @@
204
205 }
206
207- protected override bool draw (Cairo.Context cr) {
208- Gtk.Allocation size;
209- get_allocation (out size);
210-
211- base.draw (cr);
212-
213- // Draw icon
214- if (icon != null) {
215- Gdk.cairo_set_source_pixbuf (cr, icon, (icon.width - size.width) / -2.0, 10);
216- cr.paint_with_alpha (alpha);
217- }
218-
219- return true;
220-
221- }
222-
223 public void launch_app () {
224 application.launch ();
225 app_launched ();
226
227=== modified file 'src/Widgets/Grid.vala'
228--- src/Widgets/Grid.vala 2014-08-14 20:09:40 +0000
229+++ src/Widgets/Grid.vala 2014-10-11 19:03:29 +0000
230@@ -35,27 +35,31 @@
231 private uint current_row = 0;
232 private uint current_col = 0;
233 private Page page;
234+
235+ private Gtk.Box fake_grid;
236
237 public Grid (int rows, int columns) {
238 page.rows = rows;
239 page.columns = columns;
240 page.number = 1;
241- var main_grid = new Gtk.Grid ();
242- main_grid.orientation = Gtk.Orientation.VERTICAL;
243+ this.orientation = Gtk.Orientation.VERTICAL;
244 stack = new Gtk.Stack ();
245 stack.transition_type = Gtk.StackTransitionType.SLIDE_LEFT_RIGHT;
246 page_switcher = new Widgets.Switcher ();
247+ page_switcher.margin_top = 12;
248 page_switcher.set_stack (stack);
249- var fake_grid = new Gtk.Grid ();
250- fake_grid.hexpand = true;
251- var stack_layout = new Gtk.Layout ();
252- stack_layout.expand = true;
253- stack_layout.add (stack);
254+ this.fake_grid = new Gtk.Box (Gtk.Orientation.VERTICAL,0);
255+ this.fake_grid.hexpand = true;
256+ this.fake_grid.pack_start (page_switcher,false,true,0);
257+ page_switcher.set_minimum_size.connect ( (w,s) => {
258+ // By connecting to this signal we get the desired height of the page switcher
259+ // This allows to set the height of the fake grid even if the page switcher is hiden,
260+ // allowing to keep the popup size constant
261+ this.fake_grid.set_size_request (-1,s);
262+ });
263
264- main_grid.add (stack_layout);
265- main_grid.add (fake_grid);
266- main_grid.add (page_switcher);
267- add (main_grid);
268+ this.pack_start (stack,false,true,0);
269+ this.pack_start (fake_grid,false,true,0);
270
271 grids = new Gee.HashMap<int, Gtk.Grid> (null, null);
272 create_new_grid ();
273@@ -65,7 +69,7 @@
274 private void create_new_grid () {
275 // Grid properties
276 current_grid = new Gtk.Grid ();
277- current_grid.expand = true;
278+ current_grid.expand = false;
279 current_grid.row_homogeneous = true;
280 current_grid.column_homogeneous = true;
281
282@@ -75,7 +79,9 @@
283 stack.add_titled (current_grid, page.number.to_string (), page.number.to_string ());
284
285 // Fake grids in case there are not enough apps to fill the grid
286- current_grid.attach (new Gtk.Grid (), 0, 0, (int)page.columns, (int)page.rows);
287+ var tmpgrid = new Gtk.Grid ();
288+ tmpgrid.expand = false;
289+ current_grid.attach (tmpgrid, 0, 0, (int)page.columns, (int)page.rows);
290 }
291
292 public void append (Gtk.Widget widget) {
293@@ -171,4 +177,5 @@
294 page.number = 1;
295 }
296 }
297-}
298\ No newline at end of file
299+}
300+
301
302=== added file 'src/Widgets/LabelFixedWidth.vala'
303--- src/Widgets/LabelFixedWidth.vala 1970-01-01 00:00:00 +0000
304+++ src/Widgets/LabelFixedWidth.vala 2014-10-11 19:03:29 +0000
305@@ -0,0 +1,36 @@
306+// -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
307+//
308+// Copyright (C) 2014 Raster Software Vigo
309+//
310+// This program is free software: you can redistribute it and/or modify
311+// it under the terms of the GNU General Public License as published by
312+// the Free Software Foundation, either version 3 of the License, or
313+// (at your option) any later version.
314+//
315+// This program is distributed in the hope that it will be useful,
316+// but WITHOUT ANY WARRANTY; without even the implied warranty of
317+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
318+// GNU General Public License for more details.
319+//
320+// You should have received a copy of the GNU General Public License
321+// along with this program. If not, see <http://www.gnu.org/licenses/>.
322+//
323+
324+public class Slingshot.Widgets.LabelFixedWidth : Gtk.Label {
325+
326+ int d_preferred_width;
327+
328+ // This widget is a label, modified to return to its container that it
329+ // always have a fixed width. This ensures that, when adding it inside
330+ // a Gtk.Fixed widget, it will keep its size, instead of growing
331+ public LabelFixedWidth (string text, int width) {
332+ this.label = text;
333+ this.d_preferred_width = width;
334+ this.set_size_request (width, -1);
335+ }
336+
337+ public override void get_preferred_width (out int min, out int required) {
338+ min = this.d_preferred_width;
339+ required = this.d_preferred_width;
340+ }
341+}
342
343=== modified file 'src/Widgets/Switcher.vala'
344--- src/Widgets/Switcher.vala 2014-05-26 06:26:17 +0000
345+++ src/Widgets/Switcher.vala 2014-10-11 19:03:29 +0000
346@@ -28,6 +28,8 @@
347 private Gtk.Stack stack;
348 private Gee.HashMap<Gtk.Widget, Gtk.ToggleButton> buttons;
349
350+ public signal void set_minimum_size (int size);
351+
352 public Switcher () {
353 orientation = Gtk.Orientation.HORIZONTAL;
354 spacing = 2;
355@@ -49,6 +51,7 @@
356
357 private void add_child (Gtk.Widget widget) {
358 var button = new Gtk.ToggleButton.with_label ((buttons.size +1).to_string ());
359+ button.show_all ();
360 button.width_request = 30;
361 button.can_focus = false;
362 button.get_style_context ().add_class ("switcher");
363@@ -73,6 +76,14 @@
364
365 public override void show () {
366 base.show ();
367+ int min1;
368+ int min2;
369+ this.get_preferred_height (out min1, out min2);
370+ // whenever this widget is shown, checks its desired height and sends a signal with this value
371+ // This allows other widgets to set their height to the same than this, even when this one is hiden
372+ if (min2 != 0)
373+ this.set_minimum_size (min2);
374+
375 if (buttons.size <= 1)
376 hide ();
377 }
378@@ -122,4 +133,4 @@
379 }
380 }
381 }
382-}
383\ No newline at end of file
384+}

Subscribers

People subscribed via source and target branches