Unity

Merge lp:~azzar1/unity/fix-bug-955158 into lp:unity

  1. fix-bug-955158
  2. Merge into trunk
Proposed by Andrea Azzarone
Status: Merged
Approved by: Andrea Azzarone
Approved revision: no longer in the source branch.
Merged at revision: 2493
Proposed branch: lp:~azzar1/unity/fix-bug-955158
Merge into: lp:unity
Diff against target: 103 lines (+25/-18)
2 files modified
launcher/QuicklistView.cpp (+25/-16)
launcher/QuicklistView.h (+0/-2)
To merge this branch: bzr merge lp:~azzar1/unity/fix-bug-955158
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Approve
Daniel van Vugt Pending
Tim Penhey Pending
jenkins continuous-integration Pending
Review via email: mp+114334@code.launchpad.net

This proposal supersedes a proposal from 2012-06-26.

Commit message

Fix padding issues in quicklist view.

Description of the change

== Problem ==
padding between last quicklist item and bottom edge is non-deterministic (changes randomly)

== Test ==
Not applicable: visual change.

To post a comment you must log in.
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:2445
http://s-jenkins:8080/job/unity-ci/25/

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

There are a lot of identical expressions:

38 + _top_space->SetMinimumHeight((_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius + _offset_correction);
39 + _top_space->SetMaximumHeight((_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius + _offset_correction);
40 +
41 + _bottom_space->SetMinimumHeight((_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius);
42 + _bottom_space->SetMaximumHeight((_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius);

Please only compute each different expression once. Like...

int b = (_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius;
int t = b + _offset_correction;
_top_space->SetMinimumHeight(t);
_top_space->SetMaximumHeight(t);
_bottom_space->SetMinimumHeight(b);
_bottom_space->SetMaximumHeight(b);

review: Needs Fixing
Revision history for this message
Andrea Azzarone (azzar1) wrote : Posted in a previous version of this proposal

> There are a lot of identical expressions:
>
> 38 + _top_space->SetMinimumHeight((_anchor_height - TotalItemHeight) /
> 2 + _padding + _corner_radius + _offset_correction);
> 39 + _top_space->SetMaximumHeight((_anchor_height - TotalItemHeight) /
> 2 + _padding + _corner_radius + _offset_correction);
> 40 +
> 41 + _bottom_space->SetMinimumHeight((_anchor_height -
> TotalItemHeight) / 2 + _padding + _corner_radius);
> 42 + _bottom_space->SetMaximumHeight((_anchor_height -
> TotalItemHeight) / 2 + _padding + _corner_radius);
>
> Please only compute each different expression once. Like...
>
> int b = (_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius;
> int t = b + _offset_correction;
> _top_space->SetMinimumHeight(t);
> _top_space->SetMaximumHeight(t);
> _bottom_space->SetMinimumHeight(b);
> _bottom_space->SetMaximumHeight(b);

Done.

review: Needs Resubmitting
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:2446
http://s-jenkins:8080/job/unity-ci/32/

review: Approve (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Tim Penhey (thumper) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

No proposals found for merge of lp:~andyrock/unity/ql-c++11-for into lp:unity.

Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

No proposals found for merge of lp:~andyrock/unity/ql-c++11-for into lp:unity.

Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

No proposals found for merge of lp:~andyrock/unity/ql-c++11-for into lp:unity.

Revision history for this message
Andrea Azzarone (azzar1) :
review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

No commit message specified.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/QuicklistView.cpp'
2--- launcher/QuicklistView.cpp 2012-07-03 14:08:09 +0000
3+++ launcher/QuicklistView.cpp 2012-07-11 05:37:34 +0000
4@@ -65,8 +65,6 @@
5 , _corner_radius(4)
6 , _padding(13)
7 , _left_padding_correction(-1)
8- , _bottom_padding_correction_normal(-2)
9- , _bottom_padding_correction_single_item(-4)
10 , _offset_correction(-1)
11 , _cairo_text_has_changed(true)
12 , _current_item_index(-1)
13@@ -339,7 +337,7 @@
14
15 if (!_enable_quicklist_for_testing)
16 {
17- if ((_item_list.size() != 0))
18+ if (!_item_list.empty())
19 {
20 int offscreen_size = GetBaseY() +
21 GetBaseHeight() -
22@@ -446,44 +444,55 @@
23 continue;
24 }
25 else if (!item->GetParentObject())
26+ {
27 _item_layout->AddView(item, 1, nux::eCenter, nux::eFull);
28+ }
29
30 int textWidth = 0;
31 int textHeight = 0;
32 item->GetTextExtents(textWidth, textHeight);
33- if (textWidth > MaxItemWidth)
34- MaxItemWidth = textWidth;
35+ textHeight += QuicklistMenuItem::ITEM_MARGIN * 2;
36+
37+ MaxItemWidth = std::max(MaxItemWidth, textWidth);
38 TotalItemHeight += textHeight;
39 }
40
41 if (TotalItemHeight < _anchor_height)
42 {
43- _top_space->SetMinMaxSize(1, (_anchor_height - TotalItemHeight) / 2 + 1 + _padding + _corner_radius);
44- _bottom_space->SetMinMaxSize(1, (_anchor_height - TotalItemHeight) / 2 + 1 +
45- _padding + _corner_radius +
46- _bottom_padding_correction_single_item);
47+ int b = (_anchor_height - TotalItemHeight) / 2 + _padding + _corner_radius;
48+ int t = b + _offset_correction;
49+
50+ _top_space->SetMinimumHeight(t);
51+ _top_space->SetMaximumHeight(t);
52+
53+ _bottom_space->SetMinimumHeight(b);
54+ _bottom_space->SetMaximumHeight(b);
55 }
56 else
57 {
58- _top_space->SetMinMaxSize(_padding + _corner_radius, _padding + _corner_radius);
59- _bottom_space->SetMinMaxSize(_padding + _corner_radius - 2,
60- _padding + _corner_radius +
61- _bottom_padding_correction_normal);
62+ int b = _padding + _corner_radius;
63+ int t = b + _offset_correction;
64+
65+ _top_space->SetMinimumHeight(t);
66+ _top_space->SetMaximumHeight(t);
67+
68+ _bottom_space->SetMinimumHeight(b);
69+ _bottom_space->SetMaximumHeight(b);
70 }
71
72 _item_layout->SetMinimumWidth(MaxItemWidth);
73
74- BaseWindow::PreLayoutManagement();
75+ CairoBaseWindow::PreLayoutManagement();
76 }
77
78 long QuicklistView::PostLayoutManagement(long LayoutResult)
79 {
80- long result = BaseWindow::PostLayoutManagement(LayoutResult);
81+ long result = CairoBaseWindow::PostLayoutManagement(LayoutResult);
82
83 UpdateTexture();
84
85 int x = _padding + _anchor_width + _corner_radius + _offset_correction;
86- int y = _padding + _corner_radius + _offset_correction;
87+ int y = _top_space->GetMinimumHeight();
88
89 for (auto item : _item_list)
90 {
91
92=== modified file 'launcher/QuicklistView.h'
93--- launcher/QuicklistView.h 2012-07-03 14:08:09 +0000
94+++ launcher/QuicklistView.h 2012-07-11 05:37:34 +0000
95@@ -153,8 +153,6 @@
96 float _corner_radius;
97 float _padding;
98 float _left_padding_correction;
99- float _bottom_padding_correction_normal;
100- float _bottom_padding_correction_single_item;
101 float _offset_correction;
102 nux::HLayout* _hlayout;
103 nux::VLayout* _vlayout;