Merge lp:~unity-team/unity/fix-long-section-names into lp:unity

Proposed by Neil J. Patel
Status: Merged
Merged at revision: 567
Proposed branch: lp:~unity-team/unity/fix-long-section-names
Merge into: lp:unity
Diff against target: 281 lines (+143/-9)
3 files modified
unity-private/places/places-place-search-bar.vala (+60/-3)
unity-private/places/places-place-search-entry.vala (+25/-4)
unity-private/places/places-place-search-sections-bar.vala (+58/-2)
To merge this branch: bzr merge lp:~unity-team/unity/fix-long-section-names
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) Approve
Review via email: mp+37236@code.launchpad.net

Description of the change

This is a very horrible way of making sure we can fit all the sections on the screen. It works around a bunch of bugs in ClutterText wrt size-negotiation with ellipsis.

The main thing that happens is:

If the sections bar is too long for the screen when a place is activated, we figure that out in the allocation cycle and then request the section to go into small mode (as well as asking the search entry to become smaller too). At this point, the section will enable ellipsising on it's sections, and the bar will repack the sections to be expand=true, so we'll try and use as much space as possible. The sections themselves need to store their width when they were allowed to show all the text, so they have a reference point (as they cannot get this ever again when they need it).

When we switch places, we need to create a new sections bar to undo the damage we did and then go through the cycle again.

If i had more time, and I'll do this for SRU, I would have staged the "make things fit" into two allocation cycles:

1. First decrease the entry width and, if sections still don't fit,
2. Make sections go into small mode

There is a slight visual flicker when this happens first time, but I can try and fix that for SRU too.

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Code looks ok (in light of the comments above) and I can confirm it works. In fact I don't see the flickering :-)

Approved!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'unity-private/places/places-place-search-bar.vala'
--- unity-private/places/places-place-search-bar.vala 2010-09-20 15:53:08 +0000
+++ unity-private/places/places-place-search-bar.vala 2010-10-01 10:58:40 +0000
@@ -32,6 +32,7 @@
32 private PlaceSearchNavigation navigation;32 private PlaceSearchNavigation navigation;
33 public PlaceSearchEntry entry;33 public PlaceSearchEntry entry;
34 public PlaceSearchSectionsBar sections;34 public PlaceSearchSectionsBar sections;
35 public Clutter.Rectangle space;
35 public PlaceSearchExtraAction extra_action;36 public PlaceSearchExtraAction extra_action;
3637
37 private bool _search_fail = false;38 private bool _search_fail = false;
@@ -80,7 +81,7 @@
80 sections.show ();81 sections.show ();
8182
82 /* We need a dummy to be able to space the action label */83 /* We need a dummy to be able to space the action label */
83 var space = new Clutter.Rectangle.with_color ({255, 255, 255, 0});84 space = new Clutter.Rectangle.with_color ({255, 255, 255, 0});
84 pack (space, true, true);85 pack (space, true, true);
85 space.show ();86 space.show ();
86 87
@@ -111,6 +112,27 @@
111 return entry.text.text;112 return entry.text.text;
112 }113 }
113114
115 private bool update_broken_views ()
116 {
117 sections.small_mode = true;
118 entry.small_mode = true;
119
120 sections.ref ();
121 extra_action.ref ();
122
123 remove_actor (sections);
124 remove_actor (space);
125 remove_actor (extra_action);
126
127 pack (sections, true, true);
128 pack (extra_action, false, true);
129
130 extra_action.unref ();
131 sections.unref ();
132
133 return false;
134 }
135
114 private override void allocate (Clutter.ActorBox box,136 private override void allocate (Clutter.ActorBox box,
115 Clutter.AllocationFlags flags)137 Clutter.AllocationFlags flags)
116 {138 {
@@ -119,6 +141,16 @@
119141
120 base.allocate (box, flags);142 base.allocate (box, flags);
121143
144 float children_width = navigation.width + SPACING +
145 entry.width + SPACING +
146 sections.width + SPACING;
147 if (box.x2 - box.x1 < children_width)
148 {
149 /* Basically sections is too big */
150 if (sections.small_mode == false)
151 Timeout.add (0, update_broken_views);
152 }
153
122 if (entry.x != ex || entry.width != ewidth)154 if (entry.x != ex || entry.width != ewidth)
123 {155 {
124 /* After discussion with upstream Clutter guys, it seems like the156 /* After discussion with upstream Clutter guys, it seems like the
@@ -139,6 +171,7 @@
139 float mheight, nheight;171 float mheight, nheight;
140172
141 entry.get_preferred_height (RANDOM_TEXT_WIDTH, out mheight, out nheight);173 entry.get_preferred_height (RANDOM_TEXT_WIDTH, out mheight, out nheight);
174
142 min_height = mheight + SPACING * 3;175 min_height = mheight + SPACING * 3;
143 nat_height = nheight + SPACING * 3;176 nat_height = nheight + SPACING * 3;
144 }177 }
@@ -181,6 +214,27 @@
181 */214 */
182 public void set_active_entry_view (PlaceEntry entry, int x, uint section=0)215 public void set_active_entry_view (PlaceEntry entry, int x, uint section=0)
183 {216 {
217 if (sections.small_mode)
218 {
219 extra_action.ref ();
220
221 remove_actor (sections);
222 remove_actor (extra_action);
223
224 sections = new PlaceSearchSectionsBar ();
225 pack (sections, false, true);
226 sections.show ();
227
228 /* We need a dummy to be able to space the action label */
229 space = new Clutter.Rectangle.with_color ({255, 255, 255, 0});
230 pack (space, true, true);
231 space.show ();
232
233 pack (extra_action, false, true);
234
235 extra_action.unref ();
236 }
237
184 active_entry = entry;238 active_entry = entry;
185 bg.entry_position = x;239 bg.entry_position = x;
186 sections.set_active_entry (entry);240 sections.set_active_entry (entry);
@@ -400,8 +454,11 @@
400 /* Cut out the search entry */454 /* Cut out the search entry */
401 cr.set_operator (Cairo.Operator.CLEAR);455 cr.set_operator (Cairo.Operator.CLEAR);
402456
403 x = (int)search_entry.x;457 if (search_entry.x < 1.0f)
404 y = (int)(search_entry.y) - 1;458 x += width + 12;
459 else
460 x = (int)search_entry.x;
461 //y = (int)(search_entry.y) - 1;
405 width = x + (int)search_entry.width;462 width = x + (int)search_entry.width;
406 height = y + (int)search_entry.height +1;463 height = y + (int)search_entry.height +1;
407464
408465
=== modified file 'unity-private/places/places-place-search-entry.vala'
--- unity-private/places/places-place-search-entry.vala 2010-09-02 19:19:10 +0000
+++ unity-private/places/places-place-search-entry.vala 2010-10-01 10:58:40 +0000
@@ -37,6 +37,18 @@
37 public Ctk.Text text;37 public Ctk.Text text;
38 public CairoCanvas right_icon;38 public CairoCanvas right_icon;
3939
40 public bool _small_mode = false;
41 public bool small_mode {
42 get {
43 return _small_mode;;
44 }
45 set {
46 _small_mode = value;
47 hint_text.set_markup ("<i>" + _("Search") + "</i>");
48 queue_relayout ();
49 }
50 }
51
40 private uint _cursor_blink_count = 0;52 private uint _cursor_blink_count = 0;
4153
42 private bool upward = true;54 private bool upward = true;
@@ -88,7 +100,8 @@
88 {100 {
89 Object (orientation:Ctk.Orientation.HORIZONTAL,101 Object (orientation:Ctk.Orientation.HORIZONTAL,
90 homogeneous:false,102 homogeneous:false,
91 spacing:0);103 spacing:0,
104 small_mode:false);
92 }105 }
93106
94 ~PlaceSearchEntry ()107 ~PlaceSearchEntry ()
@@ -157,8 +170,16 @@
157 out float nat_width)170 out float nat_width)
158 {171 {
159 /* FIXME: Make these dependant on size of screen & font */172 /* FIXME: Make these dependant on size of screen & font */
160 min_width = 270.0f;173 if (small_mode)
161 nat_width = 270.0f;174 {
175 min_width = 135.0f;
176 nat_width = 135.0f;
177 }
178 else
179 {
180 min_width = 270.0f;
181 nat_width = 270.0f;
182 }
162 }183 }
163184
164 private void on_text_changed ()185 private void on_text_changed ()
@@ -253,7 +274,7 @@
253 {274 {
254 string name = "";275 string name = "";
255276
256 if (entry is PlaceHomeEntry == false)277 if (entry is PlaceHomeEntry == false && _small_mode == false)
257 name = entry.name;278 name = entry.name;
258279
259 hint_text.set_markup ("<i>" +280 hint_text.set_markup ("<i>" +
260281
=== modified file 'unity-private/places/places-place-search-sections-bar.vala'
--- unity-private/places/places-place-search-sections-bar.vala 2010-09-22 09:40:10 +0000
+++ unity-private/places/places-place-search-sections-bar.vala 2010-10-01 10:58:40 +0000
@@ -45,6 +45,41 @@
45 }45 }
46 }46 }
4747
48 private bool _small_mode = false;
49 public bool small_mode {
50 get { return _small_mode; }
51 set {
52 if (_small_mode != value)
53 {
54 _small_mode = value;
55
56 if (_small_mode)
57 {
58 padding = { 0.0f, 0.0f, 0.0f, 0.0f };
59
60 var secs = get_children ();
61 foreach (Clutter.Actor s in secs)
62 {
63 (s as Section).full_alloc_size = s.width;
64 (s as Section).text.ellipsize = Pango.EllipsizeMode.END;
65 }
66 }
67 else
68 {
69 padding = { 0.0f, PADDING, 0.0f, PADDING };
70 var secs = get_children ();
71 foreach (Clutter.Actor s in secs)
72 {
73 (s as Section).text.ellipsize = Pango.EllipsizeMode.NONE;
74 s.queue_relayout ();
75 }
76 }
77
78 bg.update ();
79 }
80 }
81 }
82
48 private PlaceEntry? active_entry = null;83 private PlaceEntry? active_entry = null;
49 private Section? active_section = null;84 private Section? active_section = null;
50 public uint active_section_n = 0;85 public uint active_section_n = 0;
@@ -123,7 +158,6 @@
123 (actor as Section).dirty = true;158 (actor as Section).dirty = true;
124 }159 }
125160
126
127 active_entry = entry;161 active_entry = entry;
128 var model = active_entry.sections_model;162 var model = active_entry.sections_model;
129 sections_model = model;163 sections_model = model;
@@ -173,6 +207,8 @@
173 model.row_added.connect (on_section_added);207 model.row_added.connect (on_section_added);
174 model.row_changed.connect (on_section_changed);208 model.row_changed.connect (on_section_changed);
175 model.row_removed.connect (on_section_removed);209 model.row_removed.connect (on_section_removed);
210
211 bg.update ();
176 }212 }
177213
178 private static int sort_sections (Section asec, Section bsec)214 private static int sort_sections (Section asec, Section bsec)
@@ -184,7 +220,9 @@
184 private void on_section_added (Dee.Model model, Dee.ModelIter iter)220 private void on_section_added (Dee.Model model, Dee.ModelIter iter)
185 {221 {
186 var section = new Section (model, iter);222 var section = new Section (model, iter);
187 pack (section, false, true);223 if (small_mode)
224 section.text.ellipsize = Pango.EllipsizeMode.END;
225 pack (section, false, true);
188 section.show ();226 section.show ();
189227
190 section.button_release_event.connect (on_section_clicked);228 section.button_release_event.connect (on_section_clicked);
@@ -414,6 +452,8 @@
414 private float _last_width = 0.0f;452 private float _last_width = 0.0f;
415 private float _resize_width = 0.0f;453 private float _resize_width = 0.0f;
416454
455 public float full_alloc_size = 0.0f;
456
417 public Section (Dee.Model model, Dee.ModelIter iter)457 public Section (Dee.Model model, Dee.ModelIter iter)
418 {458 {
419 Object (reactive:true,459 Object (reactive:true,
@@ -465,6 +505,22 @@
465505
466 base.get_preferred_width (for_height, out mw, out nw);506 base.get_preferred_width (for_height, out mw, out nw);
467507
508 if (text.ellipsize != Pango.EllipsizeMode.NONE)
509 {
510 if (full_alloc_size < 10.0f || full_alloc_size > 75.0f)
511 {
512 min_width = 75.0f;
513 nat_width = 75.0f;
514 }
515 else
516 {
517 text.ellipsize = Pango.EllipsizeMode.NONE;
518 min_width = full_alloc_size;
519 nat_width = full_alloc_size;
520 }
521 return;
522 }
523
468 min_width = (mw + padding.right + padding.left) * _destroy_factor;524 min_width = (mw + padding.right + padding.left) * _destroy_factor;
469 nat_width = (nw + padding.right + padding.left) * _destroy_factor;525 nat_width = (nw + padding.right + padding.left) * _destroy_factor;
470526