Merge lp:~aacid/unity8/sectionDelegateResizes into lp:unity8
- sectionDelegateResizes
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Josh Arenson |
Approved revision: | 2211 |
Merged at revision: | 2360 |
Proposed branch: | lp:~aacid/unity8/sectionDelegateResizes |
Merge into: | lp:unity8 |
Diff against target: |
126 lines (+39/-12) 4 files modified
plugins/Dash/listviewwithpageheader.cpp (+22/-10) plugins/Dash/listviewwithpageheader.h (+1/-1) tests/plugins/Dash/listviewwithpageheadersectiontest.cpp (+15/-0) tests/plugins/Dash/listviewwithpageheadertestsection.qml (+1/-1) |
To merge this branch: | bzr merge lp:~aacid/unity8/sectionDelegateResizes |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Needs Fixing | |
Josh Arenson | Approve | ||
Review via email: mp+287451@code.launchpad.net |
Commit message
Take into account that sectionItem height can change
Description of the change
How to test this:
* Be on desktop
* start smart-scopes-proxy
* run builddir/
* You get something like http://
* Click on All and select Utilities
* You get something like http://
* See the abnormal space on top
* With this patch you should get http://
* Are there any related MPs required for this MP to build/function as expected?
No
* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
* Did you make sure that your branch does not contain spurious tags?
Yes
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
* If you changed the UI, has there been a design review?
N/A
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
- 2209. By Albert Astals Cid
-
Merge
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2209
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2209
https:/
Executed test runs:
UNSTABLE: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Josh Arenson (josharenson) wrote : | # |
I appreciate your detailed instructions, but where can I find smart-scopes-proxy? The code looks good so far, but I can't test this locally as my unity8-dash is (obviously) blank.
Albert Astals Cid (aacid) wrote : | # |
tsdgeos_work@xps:~$ apt-file search smart-scopes-proxy
libunity-scopes1.0: /usr/share/
Maybe you didn't realize that "start smart-scopes-proxy" means actually typing "start smart-scopes-proxy"
Josh Arenson (josharenson) wrote : | # |
This fixes the issue, but introduces a regression.
* Be on desktop
* start smart-scopes-proxy
* run builddir/
* Scroll to the bottom of the scope
* Scroll back up
Expected behavior: scope scrolls up
Actual behavior: pull to refresh is activated and the user is unable to scroll up
This is only happening _sometimes_ and it seems to happen more if you drag the mouse outside of the window before releasing it.
- 2210. By Albert Astals Cid
-
Behave like the secion delegate we have in rela life (size 0 when no text)
This breaks the tests, next commit will fix them
- 2211. By Albert Astals Cid
-
Do not watch the geometry of the floating section item
Albert Astals Cid (aacid) wrote : | # |
Should be better now, can you try again?
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2211
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Josh Arenson (josharenson) wrote : | # |
* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes
* Did CI run pass?
No, looks like a weird jenkins issue. Kicking off a new build, and will top level approve when it passes.
* Did you make sure that the branch does not contain spurious tags?
Yes, clean.
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2211
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2211
https:/
Executed test runs:
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2211
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 2212. By Albert Astals Cid
-
Merge unity8
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2211
https:/
Executed test runs:
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2212
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'plugins/Dash/listviewwithpageheader.cpp' |
2 | --- plugins/Dash/listviewwithpageheader.cpp 2016-02-05 14:56:32 +0000 |
3 | +++ plugins/Dash/listviewwithpageheader.cpp 2016-04-07 07:36:14 +0000 |
4 | @@ -285,7 +285,7 @@ |
5 | |
6 | m_sectionDelegate = delegate; |
7 | |
8 | - m_topSectionItem = getSectionItem(QString()); |
9 | + m_topSectionItem = getSectionItem(QString(), false /*watchGeometry*/); |
10 | m_topSectionItem->setZ(3); |
11 | QQuickItemPrivate::get(m_topSectionItem)->setCulled(true); |
12 | connect(m_topSectionItem, &QQuickItem::heightChanged, this, &ListViewWithPageHeader::stickyHeaderHeightChanged); |
13 | @@ -700,8 +700,10 @@ |
14 | |
15 | void ListViewWithPageHeader::releaseItem(ListItem *listItem) |
16 | { |
17 | - QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(listItem->m_item); |
18 | - itemPrivate->removeItemChangeListener(this, QQuickItemPrivate::Geometry); |
19 | + QQuickItemPrivate::get(listItem->m_item)->removeItemChangeListener(this, QQuickItemPrivate::Geometry); |
20 | + if (listItem->sectionItem()) { |
21 | + QQuickItemPrivate::get(listItem->sectionItem())->removeItemChangeListener(this, QQuickItemPrivate::Geometry); |
22 | + } |
23 | m_itemsToRelease << listItem; |
24 | } |
25 | |
26 | @@ -744,7 +746,7 @@ |
27 | return getSectionItem(section); |
28 | } |
29 | |
30 | -QQuickItem *ListViewWithPageHeader::getSectionItem(const QString §ionText) |
31 | +QQuickItem *ListViewWithPageHeader::getSectionItem(const QString §ionText, bool watchGeometry) |
32 | { |
33 | QQuickItem *sectionItem = nullptr; |
34 | |
35 | @@ -768,7 +770,9 @@ |
36 | } |
37 | m_sectionDelegate->completeCreate(); |
38 | |
39 | - // TODO attach to sectionItem so we can accomodate to it changing its height |
40 | + if (watchGeometry) { |
41 | + QQuickItemPrivate::get(sectionItem)->addItemChangeListener(this, QQuickItemPrivate::Geometry); |
42 | + } |
43 | |
44 | return sectionItem; |
45 | } |
46 | @@ -1126,15 +1130,23 @@ |
47 | } |
48 | } |
49 | |
50 | -void ListViewWithPageHeader::itemGeometryChanged(QQuickItem * /*item*/, const QRectF &newGeometry, const QRectF &oldGeometry) |
51 | +void ListViewWithPageHeader::itemGeometryChanged(QQuickItem *item, const QRectF &newGeometry, const QRectF &oldGeometry) |
52 | { |
53 | const qreal heightDiff = newGeometry.height() - oldGeometry.height(); |
54 | if (heightDiff != 0) { |
55 | - if (!m_inContentHeightKeepHeaderShown && oldGeometry.y() + oldGeometry.height() + m_clipItem->y() <= contentY() && !m_visibleItems.isEmpty()) { |
56 | + if (!m_visibleItems.isEmpty()) { |
57 | ListItem *firstItem = m_visibleItems.first(); |
58 | - firstItem->setY(firstItem->y() - heightDiff); |
59 | - adjustMinYExtent(); |
60 | - layout(); |
61 | + const auto prevFirstItemY = firstItem->y(); |
62 | + if (!m_inContentHeightKeepHeaderShown && oldGeometry.y() + oldGeometry.height() + m_clipItem->y() <= contentY()) { |
63 | + firstItem->setY(firstItem->y() - heightDiff); |
64 | + } else if (item == firstItem->sectionItem()) { |
65 | + firstItem->setY(firstItem->y() + heightDiff); |
66 | + } |
67 | + |
68 | + if (firstItem->y() != prevFirstItemY) { |
69 | + adjustMinYExtent(); |
70 | + layout(); |
71 | + } |
72 | } |
73 | refill(); |
74 | adjustMinYExtent(); |
75 | |
76 | === modified file 'plugins/Dash/listviewwithpageheader.h' |
77 | --- plugins/Dash/listviewwithpageheader.h 2016-02-05 14:56:32 +0000 |
78 | +++ plugins/Dash/listviewwithpageheader.h 2016-04-07 07:36:14 +0000 |
79 | @@ -166,7 +166,7 @@ |
80 | void reallyReleaseItem(ListItem *item); |
81 | void updateWatchedRoles(); |
82 | QQuickItem *getSectionItem(int modelIndex, bool alreadyInserted); |
83 | - QQuickItem *getSectionItem(const QString §ionText); |
84 | + QQuickItem *getSectionItem(const QString §ionText, bool watchGeometry = true); |
85 | void updateSectionItem(int modelIndex); |
86 | void initializeValuesForEmptyList(); |
87 | |
88 | |
89 | === modified file 'tests/plugins/Dash/listviewwithpageheadersectiontest.cpp' |
90 | --- tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2015-11-26 13:50:56 +0000 |
91 | +++ tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2016-04-07 07:36:14 +0000 |
92 | @@ -2180,6 +2180,21 @@ |
93 | verifyItem(2, 480., 390., false, "Agressive", false); |
94 | } |
95 | |
96 | + void firstItemSectionHeightChange() |
97 | + { |
98 | + QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 1), Q_ARG(QVariant, 5)); |
99 | + model->setProperty(0, "type", "halfheight"); |
100 | + verifyItem(0, 50., 170., false, "halfheight", false); |
101 | + } |
102 | + |
103 | + void secondItemSectionHeightChange() |
104 | + { |
105 | + QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 2), Q_ARG(QVariant, 4)); |
106 | + model->setProperty(1, "type", "halfheight"); |
107 | + verifyItem(0, 50., 190., false, "Agressive", false); |
108 | + verifyItem(1, 240., 220., false, "halfheight", false); |
109 | + } |
110 | + |
111 | private: |
112 | QQuickView *view; |
113 | ListViewWithPageHeader *lvwph; |
114 | |
115 | === modified file 'tests/plugins/Dash/listviewwithpageheadertestsection.qml' |
116 | --- tests/plugins/Dash/listviewwithpageheadertestsection.qml 2015-07-15 15:07:19 +0000 |
117 | +++ tests/plugins/Dash/listviewwithpageheadertestsection.qml 2016-04-07 07:36:14 +0000 |
118 | @@ -98,7 +98,7 @@ |
119 | sectionDelegate: Component { |
120 | Rectangle { |
121 | color: "green" |
122 | - height: 40 |
123 | + height: section === "" ? 0 : section != "halfheight" ? 40 : 20; |
124 | Text { text: section; font.pixelSize: 34 } |
125 | anchors { left: parent.left; right: parent.right } |
126 | } |
FAILED: Continuous integration, rev:2208 /unity8- jenkins. ubuntu. com/job/ lp-unity8- 1-ci/524/ /unity8- jenkins. ubuntu. com/job/ build/689 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= vivid+overlay/ 274 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= xenial/ 274 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=phone- armhf,release= vivid+overlay/ 275/console /unity8- jenkins. ubuntu. com/job/ build-0- fetch/712 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 730 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial/ 730 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 726 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 726/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial/ 726 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial/ 726/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 726 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 726/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial/ 726 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial/ 726/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 726 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 726/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial/ 726 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial/ 726/artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-unity8- 1-ci/524/ rebuild
https:/