Unity 8

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

  1. verticalJournalImprovementsV2
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Michał Sawicz
Approved revision: 628
Merged at revision: 607
Proposed branch: lp:~aacid/unity8/verticalJournalImprovementsV2
Merge into: lp:unity8
Diff against target: 83 lines (+52/-7)
2 files modified
plugins/DashViews/verticaljournal.cpp (+20/-7)
tests/plugins/DashViews/verticaljournaltest.cpp (+32/-0)
To merge this branch: bzr merge lp:~aacid/unity8/verticalJournalImprovementsV2
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Daniel d'Andrada (community) Approve
Review via email: mp+199671@code.launchpad.net

Commit message

Do not assert if the item we are removing was not created yet (because e.g. it's not in the viewport)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:627
http://jenkins.qa.ubuntu.com/job/unity8-ci/1936/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1871
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1788
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/739
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/459
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/460
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/460/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/459
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1650
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1871
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1871/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1788
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1788/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4278
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2534

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1936/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

53 + // This is the same than checkInitialPositions but

s/same than/same as

Revision history for this message
Albert Astals Cid (aacid) wrote :

> 53 + // This is the same than checkInitialPositions but
>
> s/same than/same as

Done

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:628
http://jenkins.qa.ubuntu.com/job/unity8-ci/1939/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1891
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1808/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/743
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/462
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/463
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/463/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/462
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1668
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1891
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1891/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1808
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1808/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4295/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2562

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1939/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Looks good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/887/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3725
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1907
    ABORTED: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1824/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/748
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/273
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/273
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/273/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/273
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1682
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1907
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1907/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1824
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1824/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/893/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3737
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1935
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1849/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/757
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/279
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/279
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/279/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/279
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1705
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1935
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1935/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1849
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1849/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4321/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2593

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/894/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3738
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1936
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1850
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/758
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/280
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/280
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/280/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/280
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1706
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1936
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1936/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1850
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1850/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4322
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2594

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/DashViews/verticaljournal.cpp'
2--- plugins/DashViews/verticaljournal.cpp 2013-12-19 13:56:38 +0000
3+++ plugins/DashViews/verticaljournal.cpp 2013-12-20 08:18:57 +0000
4@@ -516,16 +516,29 @@
5 for (int i = remove.count - 1; i >= 0; --i) {
6 const int indexToRemove = remove.index + i;
7 // Since we only support removing from the end, indexToRemove
8- // must refer to the last item of one of the columns.
9+ // must refer to the last item of one of the columns or
10+ // be bigger than them (because it's not in the viewport and
11+ // thus we have not created a delegate for it)
12 bool found = false;
13+ int lastCreatedIndex = INT_MIN;
14 for (int i = 0; !found && i < m_columnVisibleItems.count(); ++i) {
15 QList<ViewItem> &column = m_columnVisibleItems[i];
16- if (!column.isEmpty() && column.last().m_modelIndex == indexToRemove) {
17- releaseItem(column.takeLast());
18- found = true;
19- }
20- }
21- Q_ASSERT(found);
22+ if (!column.isEmpty()) {
23+ const int lastColumnIndex = column.last().m_modelIndex;
24+ if (lastColumnIndex == indexToRemove) {
25+ releaseItem(column.takeLast());
26+ found = true;
27+ }
28+ lastCreatedIndex = qMax(lastCreatedIndex, lastColumnIndex);
29+ }
30+ }
31+ if (!found) {
32+ if (indexToRemove < lastCreatedIndex) {
33+ qFatal("VerticalJournal only supports removal from the end of the model");
34+ } else {
35+ m_implicitHeightDirty = true;
36+ }
37+ }
38 }
39 }
40 }
41
42=== modified file 'tests/plugins/DashViews/verticaljournaltest.cpp'
43--- tests/plugins/DashViews/verticaljournaltest.cpp 2013-12-18 11:45:57 +0000
44+++ tests/plugins/DashViews/verticaljournaltest.cpp 2013-12-20 08:18:57 +0000
45@@ -359,6 +359,38 @@
46 verifyItem(vj->m_columnVisibleItems[1][1], 3, 160, 60, true);
47 }
48
49+ void testModelRemoveLastNonVisible()
50+ {
51+ model->removeLast();
52+
53+ // This is the same as checkInitialPositions but
54+ // with a different implicitHeight since there's an item less
55+ // in the model
56+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
57+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 5);
58+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 7);
59+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 6);
60+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
61+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
62+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
63+ verifyItem(vj->m_columnVisibleItems[1][1], 3, 160, 60, true);
64+ verifyItem(vj->m_columnVisibleItems[1][2], 4, 160, 80, true);
65+ verifyItem(vj->m_columnVisibleItems[0][1], 5, 0, 110, true);
66+ verifyItem(vj->m_columnVisibleItems[1][3], 6, 160, 130, true);
67+ verifyItem(vj->m_columnVisibleItems[2][1], 7, 320, 135, true);
68+ verifyItem(vj->m_columnVisibleItems[0][2], 8, 0, 190, true);
69+ verifyItem(vj->m_columnVisibleItems[2][2], 9, 320, 255, true);
70+ verifyItem(vj->m_columnVisibleItems[2][3], 10, 320, 285, true);
71+ verifyItem(vj->m_columnVisibleItems[2][4], 11, 320, 315, true);
72+ verifyItem(vj->m_columnVisibleItems[1][4], 12, 160, 340, true);
73+ verifyItem(vj->m_columnVisibleItems[0][3], 13, 0, 360, true);
74+ verifyItem(vj->m_columnVisibleItems[2][5], 14, 320, 390, true);
75+ verifyItem(vj->m_columnVisibleItems[1][5], 15, 160, 430, false);
76+ verifyItem(vj->m_columnVisibleItems[0][4], 16, 0, 570, false);
77+ verifyItem(vj->m_columnVisibleItems[1][6], 17, 160, 570, false);
78+ QTRY_COMPARE(vj->implicitHeight(), 970. + 1. * 970. / 18.);
79+ }
80+
81 void testModelAppendRemoveLast()
82 {
83 QHeightModel *model2 = new QHeightModel();

Subscribers

People subscribed via source and target branches