Unity 8

Merge lp:~aacid/unity8/sectionDelegateResizes into lp:unity8

sectionDelegateResizes
Merge into trunk

Proposed by Albert Astals Cid on 2016-02-29

Status:	Merged
Approved by:	Josh Arenson on 2016-03-21
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:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-04-07
Josh Arenson		2016-02-29	Approve on 2016-03-16
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/src/Dash/unity8-dash
* You get something like http://i.imgur.com/NEsRpqX.png
* Click on All and select Utilities
* You get something like http://i.imgur.com/jTPUd5l.png
* See the abnormal space on top
* With this patch you should get http://i.imgur.com/d8uU2rW.png that has no extra space on top

* 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 on 2016-02-29:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/524/rebuild

review: Needs Fixing (continuous-integration)

lp:~aacid/unity8/sectionDelegateResizes updated on 2016-03-14

2209. By Albert Astals Cid on 2016-03-14: Merge

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/701/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/726/rebuild

review: Needs Fixing (continuous-integration)

Josh Arenson (josharenson) wrote on 2016-03-15:

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.

review: Needs Information

Albert Astals Cid (aacid) wrote on 2016-03-15:

tsdgeos_work@xps:~$ apt-file search smart-scopes-proxy
libunity-scopes1.0: /usr/share/upstart/sessions/smart-scopes-proxy.conf

Maybe you didn't realize that "start smart-scopes-proxy" means actually typing "start smart-scopes-proxy"

Josh Arenson (josharenson) wrote on 2016-03-15:

This fixes the issue, but introduces a regression.

* Be on desktop
* start smart-scopes-proxy
* run builddir/src/Dash/unity8-dash
* 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.

review: Needs Fixing

lp:~aacid/unity8/sectionDelegateResizes updated on 2016-03-16

2210. By Albert Astals Cid on 2016-03-16

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 on 2016-03-16

Do not watch the geometry of the floating section item

Albert Astals Cid (aacid) wrote on 2016-03-16:

Should be better now, can you try again?

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-16:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/755/rebuild

review: Needs Fixing (continuous-integration)

Josh Arenson (josharenson) wrote on 2016-03-16:

* 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.

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-16:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/758/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-23:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/807/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/831/rebuild

review: Needs Fixing (continuous-integration)

lp:~aacid/unity8/sectionDelegateResizes updated on 2016-04-07

2212. By Albert Astals Cid on 2016-04-07: Merge unity8

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-07:

FAILED: Continuous integration, rev:2211
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/946/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/946/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-07:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/947/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

 === modified file 'plugins/Dash/listviewwithpageheader.cpp'
 --- plugins/Dash/listviewwithpageheader.cpp	2016-02-05 14:56:32 +0000
 +++ plugins/Dash/listviewwithpageheader.cpp	2016-04-07 07:36:14 +0000
@@ -285,7 +285,7 @@
          m_sectionDelegate = delegate;
--        m_topSectionItem = getSectionItem(QString());
++        m_topSectionItem = getSectionItem(QString(), false /*watchGeometry*/);
          m_topSectionItem->setZ(3);
          QQuickItemPrivate::get(m_topSectionItem)->setCulled(true);
          connect(m_topSectionItem, &QQuickItem::heightChanged, this, &ListViewWithPageHeader::stickyHeaderHeightChanged);
@@ -700,8 +700,10 @@
  void ListViewWithPageHeader::releaseItem(ListItem *listItem)
+ {
--    QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(listItem->m_item);
--    itemPrivate->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
++    QQuickItemPrivate::get(listItem->m_item)->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
++    if (listItem->sectionItem()) {
++        QQuickItemPrivate::get(listItem->sectionItem())->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
++    }
      m_itemsToRelease << listItem;
+ }
@@ -744,7 +746,7 @@
      return getSectionItem(section);
+ }
--QQuickItem *ListViewWithPageHeader::getSectionItem(const QString &sectionText)
++QQuickItem *ListViewWithPageHeader::getSectionItem(const QString &sectionText, bool watchGeometry)
+ {
      QQuickItem *sectionItem = nullptr;
@@ -768,7 +770,9 @@
+     }
      m_sectionDelegate->completeCreate();
--    // TODO attach to sectionItem so we can accomodate to it changing its height
++    if (watchGeometry) {
++        QQuickItemPrivate::get(sectionItem)->addItemChangeListener(this, QQuickItemPrivate::Geometry);
++    }
      return sectionItem;
+ }
@@ -1126,15 +1130,23 @@
+     }
+ }
--void ListViewWithPageHeader::itemGeometryChanged(QQuickItem * /*item*/, const QRectF &newGeometry, const QRectF &oldGeometry)
++void ListViewWithPageHeader::itemGeometryChanged(QQuickItem *item, const QRectF &newGeometry, const QRectF &oldGeometry)
+ {
      const qreal heightDiff = newGeometry.height() - oldGeometry.height();
      if (heightDiff != 0) {
--        if (!m_inContentHeightKeepHeaderShown && oldGeometry.y() + oldGeometry.height() + m_clipItem->y() <= contentY() && !m_visibleItems.isEmpty()) {
++        if (!m_visibleItems.isEmpty()) {
              ListItem *firstItem = m_visibleItems.first();
--            firstItem->setY(firstItem->y() - heightDiff);
--            adjustMinYExtent();
--            layout();
++            const auto prevFirstItemY = firstItem->y();
++            if (!m_inContentHeightKeepHeaderShown && oldGeometry.y() + oldGeometry.height() + m_clipItem->y() <= contentY()) {
++                firstItem->setY(firstItem->y() - heightDiff);
++            } else if (item == firstItem->sectionItem()) {
++                firstItem->setY(firstItem->y() + heightDiff);
++            }
++
++            if (firstItem->y() != prevFirstItemY) {
++                adjustMinYExtent();
++                layout();
++            }
+         }
          refill();
          adjustMinYExtent();
 === modified file 'plugins/Dash/listviewwithpageheader.h'
 --- plugins/Dash/listviewwithpageheader.h	2016-02-05 14:56:32 +0000
 +++ plugins/Dash/listviewwithpageheader.h	2016-04-07 07:36:14 +0000
@@ -166,7 +166,7 @@
      void reallyReleaseItem(ListItem *item);
      void updateWatchedRoles();
      QQuickItem *getSectionItem(int modelIndex, bool alreadyInserted);
--    QQuickItem *getSectionItem(const QString &sectionText);
++    QQuickItem *getSectionItem(const QString &sectionText, bool watchGeometry = true);
      void updateSectionItem(int modelIndex);
      void initializeValuesForEmptyList();
 === modified file 'tests/plugins/Dash/listviewwithpageheadersectiontest.cpp'
 --- tests/plugins/Dash/listviewwithpageheadersectiontest.cpp	2015-11-26 13:50:56 +0000
 +++ tests/plugins/Dash/listviewwithpageheadersectiontest.cpp	2016-04-07 07:36:14 +0000
@@ -2180,6 +2180,21 @@
          verifyItem(2, 480., 390., false, "Agressive", false);
+     }
++    void firstItemSectionHeightChange()
++    {
++        QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 1), Q_ARG(QVariant, 5));
++        model->setProperty(0, "type", "halfheight");
++        verifyItem(0, 50., 170., false, "halfheight", false);
++    }
++
++    void secondItemSectionHeightChange()
++    {
++        QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 2), Q_ARG(QVariant, 4));
++        model->setProperty(1, "type", "halfheight");
++        verifyItem(0, 50., 190., false, "Agressive", false);
++        verifyItem(1, 240., 220., false, "halfheight", false);
++    }
++
  private:
      QQuickView *view;
      ListViewWithPageHeader *lvwph;
 === modified file 'tests/plugins/Dash/listviewwithpageheadertestsection.qml'
 --- tests/plugins/Dash/listviewwithpageheadertestsection.qml	2015-07-15 15:07:19 +0000
 +++ tests/plugins/Dash/listviewwithpageheadertestsection.qml	2016-04-07 07:36:14 +0000
@@ -98,7 +98,7 @@
          sectionDelegate: Component {
              Rectangle {
                  color: "green"
--                height: 40
++                height: section === "" ? 0 : section != "halfheight" ? 40 : 20;
                  Text { text: section; font.pixelSize: 34 }
                  anchors { left: parent.left; right: parent.right }
+             }