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

Proposed by Albert Astals Cid
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 620
Merged at revision: 599
Proposed branch: lp:~aacid/unity8/verticalJournal
Merge into: lp:unity8
Diff against target: 1632 lines (+1363/-28)
22 files modified
Dash/ScopeListView.qml (+1/-1)
debian/unity8-private.install (+1/-1)
plugins/CMakeLists.txt (+1/-1)
plugins/DashViews/CMakeLists.txt (+6/-5)
plugins/DashViews/plugin.cpp (+4/-2)
plugins/DashViews/plugin.h (+3/-3)
plugins/DashViews/qmldir (+2/-2)
plugins/DashViews/verticaljournal.cpp (+590/-0)
plugins/DashViews/verticaljournal.h (+182/-0)
tests/plugins/CMakeLists.txt (+1/-1)
tests/plugins/DashViews/CMakeLists.txt (+16/-5)
tests/plugins/DashViews/listviewwithpageheadertest.cpp (+1/-1)
tests/plugins/DashViews/listviewwithpageheadertest.qml (+1/-1)
tests/plugins/DashViews/listviewwithpageheadertestsection.cpp (+1/-1)
tests/plugins/DashViews/listviewwithpageheadertestsection.qml (+1/-1)
tests/plugins/DashViews/listviewwithpageheadertestsectionexternalmodel.cpp (+1/-1)
tests/plugins/DashViews/listviewwithpageheadertestsectionexternalmodel.qml (+1/-1)
tests/plugins/DashViews/tst_listviewwithpageheaderqml.qml (+1/-1)
tests/plugins/DashViews/verticaljournaltest.cpp (+315/-0)
tests/plugins/DashViews/verticaljournaltest.qml (+44/-0)
tests/plugins/DashViews/verticaljournaltry.cpp (+90/-0)
tests/plugins/DashViews/verticaljournaltry.qml (+100/-0)
To merge this branch: bzr merge lp:~aacid/unity8/verticalJournal
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Daniel d'Andrada (community) Approve
Review via email: mp+198897@code.launchpad.net

Commit message

Vertical journal

Comes from lp:~aacid/+junk/verticalJournal

To post a comment you must log in.
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

707 +class VerticalJournal : public QQuickItem

Would be lovely to have some short explanation about it. Maybe some copy&paste from the design doc.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

1274 + VerticalJournal *vj;

"Function names, variable names, and filenames should be descriptive; eschew abbreviation."

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

"Comes from lp:~aacid/+junk/verticalJournal"

What's the relevance of this info in the commit message?

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

> "Comes from lp:~aacid/+junk/verticalJournal"
>
> What's the relevance of this info in the commit message?

Let's you see the history of how the thing was built instead of a whole chunk of code in case you need to know why something was done one way or another.

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

> 1274 + VerticalJournal *vj;
>
> "Function names, variable names, and filenames should be descriptive; eschew
> abbreviation."

It's a test of VerticalJournal, if you really don't know that vj represents the VerticalJournal, you have a big problem.

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

FAILED: Continuous integration, rev:594
http://jenkins.qa.ubuntu.com/job/unity8-ci/1891/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1691
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1623
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/667
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/414
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/415
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/415/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/414
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1500
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1691
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1691/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1623
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1623/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4133
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2325

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

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

PASSED: Continuous integration, rev:595
http://jenkins.qa.ubuntu.com/job/unity8-ci/1893/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1697
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1628
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/670
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/416
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/417
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/417/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/416
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1505
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1697
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1697/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1628
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1628/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4138
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2330

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

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

PASSED: Continuous integration, rev:598
http://jenkins.qa.ubuntu.com/job/unity8-ci/1898/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1707
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1638
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/678
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/422
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/422/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1514
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1707
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1707/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1638
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1638/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4147
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2341

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

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :
Download full text (3.4 KiB)

> Q_PROPERTY(qreal horizontalSpacing READ horizontalSpacing WRITE setHorizontalSpacing NOTIFY horizontalSpacingChanged)
> [...]
> Q_PROPERTY(qreal delegateCreationBegin READ delegateCreationBegin WRITE setDelegateCreationBegin NOTIFY delegateCreationBeginChanged RESET resetDelegateCreationBegin)
> Q_PROPERTY(qreal delegateCreationEnd READ delegateCreationEnd WRITE setDelegateCreationEnd NOTIFY delegateCreationEndChanged RESET resetDelegateCreationEnd)

"Each line of text in your code should be at most 120 characters long. "

-----------------------

For test targets, the convention we use is "testFoo", not "testfoo".

Therefore I think the test should be called "testVerticalJournal", not "testverticaljournal".

-----------------------

I see that the vertical journal is not used anywhere yet. So there's no place to play with it. Would it be too much to ask for a "make tryVerticalJournal" target that brings up an interactive manual test so one could play with the VerticalJournal? I envision a window with a vertical journal on its left side and some controls on its right side. One could use those controls to manually add an item with a given size and see how the vertical journal places it. And click on a placed item to remove it and see now the vertical journal reorganizes itself. Similar to what we have with "make tryResponsiveGridView" and "make tryStage".

What do you think?

----------------------

In VerticalJournal::setModel(QAbstractItemModel *model):

"""
#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
        m_delegateModel->setModel(QVariant::fromValue<QAbstractItemModel *>(model));
#else
        m_delegateModel->setModel(QVariant::fromValue<QAbstractItemModel *>(model));
#endif
"""

Played "spot the difference" here and lost.

---------------------------

void VerticalJournal::createDelegateModel()
{
#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
    m_delegateModel = new QQuickVisualDataModel(qmlContext(this), this);
    connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)), this, SLOT(itemCreated(int,QQuickItem*)));
#else
    m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
    connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this, SLOT(itemCreated(int,QObject*)));
#endif

Could we use the QObject::connect version that checks signal-slot compatibility at compile time (as opposed to runtime)? Since we're not checking the connect() results here, which can fail with this runtime-check version, we are making room for sneaky bugs to slip through.

-------------------------------

    const int nColumns = ceil((double)(width() - m_horizontalSpacing) / (m_columnWidth + m_horizontalSpacing));

How have you come to this formula? My question also comes from the fact that I don't know what's your definition of horizontalSpacing, which should probably documented.

For instance, I don't know is there's horizontalSpacing to the left of the left-most column and to the right of the right-most column.
Or if the spacing applies exclusively *between* two columns.

I would help if you used the same nomenclature as the existing Grid component. i.e., s/horizontalSpacing/columnSpacing and s/verticalSpacing/rowSpac...

Read more...

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

> > Q_PROPERTY(qreal horizontalSpacing READ horizontalSpacing WRITE
> setHorizontalSpacing NOTIFY horizontalSpacingChanged)
> > [...]
> > Q_PROPERTY(qreal delegateCreationBegin READ delegateCreationBegin WRITE
> setDelegateCreationBegin NOTIFY delegateCreationBeginChanged RESET
> resetDelegateCreationBegin)
> > Q_PROPERTY(qreal delegateCreationEnd READ delegateCreationEnd WRITE
> setDelegateCreationEnd NOTIFY delegateCreationEndChanged RESET
> resetDelegateCreationEnd)
>
> "Each line of text in your code should be at most 120 characters long. "

Fixed

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

> -----------------------
>
> For test targets, the convention we use is "testFoo", not "testfoo".
>
> Therefore I think the test should be called "testVerticalJournal", not
> "testverticaljournal".
>
> -----------------------

Done.

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

>
> In VerticalJournal::setModel(QAbstractItemModel *model):
>
> """
> #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
> m_delegateModel->setModel(QVariant::fromValue<QAbstractItemModel
> *>(model));
> #else
> m_delegateModel->setModel(QVariant::fromValue<QAbstractItemModel
> *>(model));
> #endif
> """
>
> Played "spot the difference" here and lost.

Yep, that made sense at the begining but then it did not and i ended up with that silly ifdef.

Fixed

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

> ---------------------------
>
> void VerticalJournal::createDelegateModel()
> {
> #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
> m_delegateModel = new QQuickVisualDataModel(qmlContext(this), this);
> connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)), this,
> SLOT(itemCreated(int,QQuickItem*)));
> #else
> m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
> connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this,
> SLOT(itemCreated(int,QObject*)));
> #endif
>
>
> Could we use the QObject::connect version that checks signal-slot
> compatibility at compile time (as opposed to runtime)? Since we're not
> checking the connect() results here, which can fail with this runtime-check
> version, we are making room for sneaky bugs to slip through.

Don't worry, if the creation of items fails that is something that the unit test will quickly see.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> My screen fits 58 lines and 200 columns, yet the reviewer thinks it is better
> to waste the few vertical space avaiable so that we can all enjoy lots of empty
> space on the right of the screen

It's our coding style, not the reviewer's arbitrary preferences.

You cannot use split windows or have code side-by-side if there are 200 chars wide lines (not, auto-wrapped code is not readable) and one cannot assume that everybody uses the same screen resolution, font size, text editor etc as himself.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> > ---------------------------
> >
> > void VerticalJournal::createDelegateModel()
> > {
> > #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
> > m_delegateModel = new QQuickVisualDataModel(qmlContext(this), this);
> > connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)), this,
> > SLOT(itemCreated(int,QQuickItem*)));
> > #else
> > m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
> > connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this,
> > SLOT(itemCreated(int,QObject*)));
> > #endif
> >
> >
> > Could we use the QObject::connect version that checks signal-slot
> > compatibility at compile time (as opposed to runtime)? Since we're not
> > checking the connect() results here, which can fail with this runtime-check
> > version, we are making room for sneaky bugs to slip through.
>
> Don't worry, if the creation of items fails that is something that the unit
> test will quickly see.

Still, there's no benefit in having the code that way.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> const int nColumns = ceil((double)(width() - m_horizontalSpacing) /
> (m_columnWidth + m_horizontalSpacing));
>
> How have you come to this formula? [...] I would expect this expression to be:
>
> nColumns = floor((width+columnSpacing) / (columnWidth+columnSpacing))

Just noticed that they boil down to the same thing in the end, so nevermind.

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

> -------------------------------
>
> const int nColumns = ceil((double)(width() - m_horizontalSpacing) /
> (m_columnWidth + m_horizontalSpacing));
>
> How have you come to this formula? My question also comes from the fact that I
> don't know what's your definition of horizontalSpacing, which should probably
> documented.
>
> For instance, I don't know is there's horizontalSpacing to the left of the
> left-most column and to the right of the right-most column.
> Or if the spacing applies exclusively *between* two columns.
>
> I would help if you used the same nomenclature as the existing Grid component.
> i.e., s/horizontalSpacing/columnSpacing and s/verticalSpacing/rowSpacing.
>
> So, considering horizontalSpacing has the same meaning as Grid's
> columnSpacing, I would expect this expression to be:
>
> nColumns = floor((width+columnSpacing) / (columnWidth+columnSpacing))

You're right, needs to be floor and not ceil (as i was using it already in the other part of the code).

The formula is simple, think the other way around, if you only have 1 column, your "desired" with would be horizontalSpacing + columnWidth + horizontalSpacing, if you have 2 columns, it'd be horizontalSpacing + columnWidth + horizontalSpacing + columnWidth + horizontalSpacing, for 3 columns, horizontalSpacing + columnWidth + horizontalSpacing + columnWidth + horizontalSpacing + columnWidth + horizontalSpacing, so
desiredWidth=nColumns * (horizontalSpacing + columnWidth) + horizontalSpacing
i.e
desiredWidth - horizontalSpacing = nColumns * (horizontalSpacing + columnWidth)
i.e.
(desiredWidth - horizontalSpacing) / (horizontalSpacing + columnWidth) = nColumns
Which is what my formula is using.

Clearer now?

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

PASSED: Continuous integration, rev:600
http://jenkins.qa.ubuntu.com/job/unity8-ci/1902/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1750
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1674
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/688
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/425
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/426
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/426/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/425
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1544
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1750
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1750/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1674
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1674/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4177
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2388

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

review: Approve (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

> > > ---------------------------
> > >
> > > void VerticalJournal::createDelegateModel()
> > > {
> > > #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
> > > m_delegateModel = new QQuickVisualDataModel(qmlContext(this), this);
> > > connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)), this,
> > > SLOT(itemCreated(int,QQuickItem*)));
> > > #else
> > > m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
> > > connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this,
> > > SLOT(itemCreated(int,QObject*)));
> > > #endif
> > >
> > >
> > > Could we use the QObject::connect version that checks signal-slot
> > > compatibility at compile time (as opposed to runtime)? Since we're not
> > > checking the connect() results here, which can fail with this runtime-
> check
> > > version, we are making room for sneaky bugs to slip through.
> >
> > Don't worry, if the creation of items fails that is something that the unit
> > test will quickly see.
>
> Still, there's no benefit in having the code that way.

There's no benefit in doing it the other way either. The benefit of doing it this way is being consistent with what listviewwithpagehader.cpp does.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> > -------------------------------
> >
> > const int nColumns = ceil((double)(width() - m_horizontalSpacing) /
> > (m_columnWidth + m_horizontalSpacing));
> >
> > How have you come to this formula? My question also comes from the fact that
> I
> > don't know what's your definition of horizontalSpacing, which should
> probably
> > documented.
> >
> > For instance, I don't know is there's horizontalSpacing to the left of the
> > left-most column and to the right of the right-most column.
> > Or if the spacing applies exclusively *between* two columns.
> >
> > I would help if you used the same nomenclature as the existing Grid
> component.
> > i.e., s/horizontalSpacing/columnSpacing and s/verticalSpacing/rowSpacing.
> >
> > So, considering horizontalSpacing has the same meaning as Grid's
> > columnSpacing, I would expect this expression to be:
> >
> > nColumns = floor((width+columnSpacing) / (columnWidth+columnSpacing))
>
> You're right, needs to be floor and not ceil (as i was using it already in the
> other part of the code).
>
> The formula is simple, think the other way around, if you only have 1 column,
> your "desired" with would be horizontalSpacing + columnWidth +
> horizontalSpacing, if you have 2 columns, it'd be horizontalSpacing +
> columnWidth + horizontalSpacing + columnWidth + horizontalSpacing, for 3
> columns, horizontalSpacing + columnWidth + horizontalSpacing + columnWidth +
> horizontalSpacing + columnWidth + horizontalSpacing, so
> desiredWidth=nColumns * (horizontalSpacing + columnWidth) + horizontalSpacing
> i.e
> desiredWidth - horizontalSpacing = nColumns * (horizontalSpacing +
> columnWidth)
> i.e.
> (desiredWidth - horizontalSpacing) / (horizontalSpacing + columnWidth) =
> nColumns
> Which is what my formula is using.
>
> Clearer now?

Absolutely, thanks.

But I would prefer s/horizontalSpacing/columnSpacing because that margin can be done via other means, yielding more flexibility to the layout. You can use Item's native margin properties to achieve that spacing between the edge columns (leftmost and rightmost columns) and the item's borders.

And also mimicking an existing API (Qt's Grid) has the added benefit of having the overall API more uniform and fit better together. A developer used to work with Grid will have a much lower learning curve if his existing API expectations can be directly transfered when working with VerticalJournal.

Same applies for verticalSpacing property.

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

FAILED: Continuous integration, rev:602
http://jenkins.qa.ubuntu.com/job/unity8-ci/1903/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1751
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1675
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/689
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/427
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/427/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1545
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1751
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1751/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1675
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1675/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4178
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2389

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

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

> > > > ---------------------------
> > > >
> > > > void VerticalJournal::createDelegateModel()
> > > > {
> > > > #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
> > > > m_delegateModel = new QQuickVisualDataModel(qmlContext(this), this);
> > > > connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)), this,
> > > > SLOT(itemCreated(int,QQuickItem*)));
> > > > #else
> > > > m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
> > > > connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this,
> > > > SLOT(itemCreated(int,QObject*)));
> > > > #endif
> > > >
> > > >
> > > > Could we use the QObject::connect version that checks signal-slot
> > > > compatibility at compile time (as opposed to runtime)? Since we're not
> > > > checking the connect() results here, which can fail with this runtime-
> > check
> > > > version, we are making room for sneaky bugs to slip through.
> > >
> > > Don't worry, if the creation of items fails that is something that the
> unit
> > > test will quickly see.
> >
> > Still, there's no benefit in having the code that way.
>
> There's no benefit in doing it the other way either. The benefit of doing it
> this way is being consistent with what listviewwithpagehader.cpp does.

There is. Besides the compile-time compatibility check I already mentioned, it's also faster as there's no string parsing involved to find the signal and slot methods because you're already passing the methods addresses directly.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> Change name from testverticaljournal to testVerticalJournal
> The reviewer thinks that it is better to addhere to the global rules than to the folder rules

If there are other tests in this directory which have all-lowercase targets ("testfoo") they should also be updated as well (to the "testFoo" way). But not in this merge proposal, naturally.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> > Change name from testverticaljournal to testVerticalJournal
> > The reviewer thinks that it is better to addhere to the global rules than to
> the folder rules
>
> If there are other tests in this directory which have all-lowercase targets
> ("testfoo") they should also be updated as well (to the "testFoo" way). But
> not in this merge proposal, naturally.

The reasoning being that, when you want to test a single component/class in unity8, you go to the terminal and write (from its build dir):
$ make testComponentName
Likewise when you want to manually test some component:
$ make tryComponentName

That way you only have to know the name of the component. If the make targets don't follow any convention the poor developer will have to dig into the relevent CMakeLists.txt to find out how to summon the desired test.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

In VerticalJournal::setModel and VerticalJournal::setDelegate:

        if (!m_delegateModel) {
            [...]
        }
        [...]
        for (int i = 0; i < m_columnVisibleItems.count(); ++i)
        {
            [...]
        }

coding style: Let's be consistent with the placement of {}.

Seeing that you have the same code snippet in two functions makes me think that you could at least have a cleanupExistingItems() helper function with that chunk of code:

        for (int i = 0; i < m_columnVisibleItems.count(); ++i) {
            QList<ViewItem> &column = m_columnVisibleItems[i];
            Q_FOREACH(const ViewItem &item, column)
                releaseItem(item);
            column.clear();
        }
        m_indexColumnMap.clear();

---------------------------

"""
    // Check if we add it to the bottom of existing column items
    qreal columnToAddY = !m_columnVisibleItems[0].isEmpty() ? m_columnVisibleItems[0].last().y() + m_columnVisibleItems[0].last().height() : 0;
    int columnToAddTo = 0;
    for (int i = 1; i < m_columnVisibleItems.count(); ++i) {
        const qreal iY = !m_columnVisibleItems[i].isEmpty() ? m_columnVisibleItems[i].last().y() + m_columnVisibleItems[i].last().height() : 0;
        if (iY < columnToAddY) {
            columnToAddTo = i;
            columnToAddY = iY;
        }
    }
    if (m_columnVisibleItems[columnToAddTo].isEmpty() || m_columnVisibleItems[columnToAddTo].last().m_modelIndex < modelIndex) {
"""

Long lines ahoy!

----------------------------

Any particular reason why you're calling m_columnVisibleItems.resize(nColumns) in componentComplete() right before calling polish()? Because updatePolish() already does the very same thing. Looks like this is either a redundant leftover that should be removed or a workaround for some obscure behavior which should then be explained with a comment (for the unware developer reading this code, like myself).

----------------------------

"""
   * The first nColumns items are layouted at row 0 from column 0
   * to column nColumns-1 in order. After that every new item
   * is positioned in the column which provides the free topmost
   * position as possible. If more than one column tie in providing
   * the topmost free position the leftmost column will be used.
"""

Got some ASCII art for you!

+-----+ +-----+ +-----+
| | | 2 | | |
| | | | | |
| 1 | +-----+ | 3 |
| | +-----+ | |
| | | | +-----+
+-----+ | 4 | +-----+
+-----+ | | | 5 |
| 6 | +-----+ | |
| | +-----+
+-----+

----------------------------

void VerticalJournal::updatePolish()
{
[...]
    if (m_implicitHeightDirty) {
        [...]
    }
}

Could be refactored to

void VerticalJournal::updatePolish()
{
[...]
    if (m_implicitHeightDirty) {
        calculateImplicitHeight();
    }
}

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> Got some ASCII art for you!
>
> +-----+ +-----+ +-----+
> | | | 2 | | |
> | | | | | |
> | 1 | +-----+ | 3 |
> | | +-----+ | |
> | | | | +-----+
> +-----+ | 4 | +-----+
> +-----+ | | | 5 |
> | 6 | +-----+ | |
> | | +-----+
> +-----+

The non-borked version:
http://paste.ubuntu.com/6585270/

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

> In VerticalJournal::setModel and VerticalJournal::setDelegate:
>
> if (!m_delegateModel) {
> [...]
> }
> [...]
> for (int i = 0; i < m_columnVisibleItems.count(); ++i)
> {
> [...]
> }
>
> coding style: Let's be consistent with the placement of {}.
>
> Seeing that you have the same code snippet in two functions makes me think
> that you could at least have a cleanupExistingItems() helper function with
> that chunk of code:
>
> for (int i = 0; i < m_columnVisibleItems.count(); ++i) {
> QList<ViewItem> &column = m_columnVisibleItems[i];
> Q_FOREACH(const ViewItem &item, column)
> releaseItem(item);
> column.clear();
> }
> m_indexColumnMap.clear();
>
>
> ---------------------------

Done

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

>
> """
> * The first nColumns items are layouted at row 0 from column 0
> * to column nColumns-1 in order. After that every new item
> * is positioned in the column which provides the free topmost
> * position as possible. If more than one column tie in providing
> * the topmost free position the leftmost column will be used.
> """
>
> Got some ASCII art for you!
>
> +-----+ +-----+ +-----+
> | | | 2 | | |
> | | | | | |
> | 1 | +-----+ | 3 |
> | | +-----+ | |
> | | | | +-----+
> +-----+ | 4 | +-----+
> +-----+ | | | 5 |
> | 6 | +-----+ | |
> | | +-----+
> +-----+

Added!

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

> ----------------------------
>
> Any particular reason why you're calling m_columnVisibleItems.resize(nColumns)
> in componentComplete() right before calling polish()? Because updatePolish()
> already does the very same thing. Looks like this is either a redundant
> leftover that should be removed or a workaround for some obscure behavior
> which should then be explained with a comment (for the unware developer
> reading this code, like myself).

I'm voting leftover since can't think of any other reason.

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

> ----------------------------
>
> void VerticalJournal::updatePolish()
> {
> [...]
> if (m_implicitHeightDirty) {
> [...]
> }
> }
>
> Could be refactored to
>
> void VerticalJournal::updatePolish()
> {
> [...]
> if (m_implicitHeightDirty) {
> calculateImplicitHeight();
> }
> }

It could. And it has been.

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

> """
> // Check if we add it to the bottom of existing column items
> qreal columnToAddY = !m_columnVisibleItems[0].isEmpty() ?
> m_columnVisibleItems[0].last().y() + m_columnVisibleItems[0].last().height() :
> 0;
> int columnToAddTo = 0;
> for (int i = 1; i < m_columnVisibleItems.count(); ++i) {
> const qreal iY = !m_columnVisibleItems[i].isEmpty() ?
> m_columnVisibleItems[i].last().y() + m_columnVisibleItems[i].last().height() :
> 0;
> if (iY < columnToAddY) {
> columnToAddTo = i;
> columnToAddY = iY;
> }
> }
> if (m_columnVisibleItems[columnToAddTo].isEmpty() ||
> m_columnVisibleItems[columnToAddTo].last().m_modelIndex < modelIndex) {
> """
>
> Long lines ahoy!

They are now shorter

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

> > > > > ---------------------------
> > > > >
> > > > > void VerticalJournal::createDelegateModel()
> > > > > {
> > > > > #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
> > > > > m_delegateModel = new QQuickVisualDataModel(qmlContext(this),
> this);
> > > > > connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)),
> this,
> > > > > SLOT(itemCreated(int,QQuickItem*)));
> > > > > #else
> > > > > m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
> > > > > connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this,
> > > > > SLOT(itemCreated(int,QObject*)));
> > > > > #endif
> > > > >
> > > > >
> > > > > Could we use the QObject::connect version that checks signal-slot
> > > > > compatibility at compile time (as opposed to runtime)? Since we're not
> > > > > checking the connect() results here, which can fail with this runtime-
> > > check
> > > > > version, we are making room for sneaky bugs to slip through.
> > > >
> > > > Don't worry, if the creation of items fails that is something that the
> > unit
> > > > test will quickly see.
> > >
> > > Still, there's no benefit in having the code that way.
> >
> > There's no benefit in doing it the other way either. The benefit of doing it
> > this way is being consistent with what listviewwithpagehader.cpp does.
>
> There is. Besides the compile-time compatibility check I already mentioned,
> it's also faster as there's no string parsing involved to find the signal and
> slot methods because you're already passing the methods addresses directly.

We don't have a policy about this and we do have lots of other connects in the code like this one so to be honest i don't see the need to change.

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

PASSED: Continuous integration, rev:610
http://jenkins.qa.ubuntu.com/job/unity8-ci/1909/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1764
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1684
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/695
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/432
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/433
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/433/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/432
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1556
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1764
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1764/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1684
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1684/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4186
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2407

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

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

    *yPos = INT_MAX;

The better (as it's type-specific), C++ way, of doing this is:

    *yPos = std::numeric_limits<double>::max();

Same here:

    *modelIndex = INT_MAX;
    *yPos = INT_MIN;

---------------------

    double yPos;
    findBottomModelIndexToAdd(&modelIndex, &yPos);

    void findBottomModelIndexToAdd(int *modelIndex, double *yPos);
    void findTopModelIndexToAdd(int *modelIndex, double *yPos);

Any reason why you using double for yPos while everywhere else we are dealing with qreals?

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

> > > -------------------------------
> > >
> > > const int nColumns = ceil((double)(width() - m_horizontalSpacing) /
> > > (m_columnWidth + m_horizontalSpacing));
> > >
> > > How have you come to this formula? My question also comes from the fact
> that
> > I
> > > don't know what's your definition of horizontalSpacing, which should
> > probably
> > > documented.
> > >
> > > For instance, I don't know is there's horizontalSpacing to the left of the
> > > left-most column and to the right of the right-most column.
> > > Or if the spacing applies exclusively *between* two columns.
> > >
> > > I would help if you used the same nomenclature as the existing Grid
> > component.
> > > i.e., s/horizontalSpacing/columnSpacing and s/verticalSpacing/rowSpacing.
> > >
> > > So, considering horizontalSpacing has the same meaning as Grid's
> > > columnSpacing, I would expect this expression to be:
> > >
> > > nColumns = floor((width+columnSpacing) / (columnWidth+columnSpacing))
> >
> > You're right, needs to be floor and not ceil (as i was using it already in
> the
> > other part of the code).
> >
> > The formula is simple, think the other way around, if you only have 1
> column,
> > your "desired" with would be horizontalSpacing + columnWidth +
> > horizontalSpacing, if you have 2 columns, it'd be horizontalSpacing +
> > columnWidth + horizontalSpacing + columnWidth + horizontalSpacing, for 3
> > columns, horizontalSpacing + columnWidth + horizontalSpacing + columnWidth +
> > horizontalSpacing + columnWidth + horizontalSpacing, so
> > desiredWidth=nColumns * (horizontalSpacing + columnWidth) +
> horizontalSpacing
> > i.e
> > desiredWidth - horizontalSpacing = nColumns * (horizontalSpacing +
> > columnWidth)
> > i.e.
> > (desiredWidth - horizontalSpacing) / (horizontalSpacing + columnWidth) =
> > nColumns
> > Which is what my formula is using.
> >
> > Clearer now?
>
> Absolutely, thanks.
>
> But I would prefer s/horizontalSpacing/columnSpacing because that margin can
> be done via other means, yielding more flexibility to the layout. You can use
> Item's native margin properties to achieve that spacing between the edge
> columns (leftmost and rightmost columns) and the item's borders.
>
> And also mimicking an existing API (Qt's Grid) has the added benefit of having
> the overall API more uniform and fit better together. A developer used to work
> with Grid will have a much lower learning curve if his existing API
> expectations can be directly transfered when working with VerticalJournal.
>
> Same applies for verticalSpacing property.

Done.

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

FAILED: Continuous integration, rev:611
http://jenkins.qa.ubuntu.com/job/unity8-ci/1912/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1770
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1690
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/699
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/435
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/436
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/436/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/435
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1562
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1770
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1770/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1690
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1690/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4193
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2416

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

> *yPos = INT_MAX;
>
> The better (as it's type-specific), C++ way, of doing this is:
>
> *yPos = std::numeric_limits<double>::max();
>
> Same here:
>
> *modelIndex = INT_MAX;
> *yPos = INT_MIN;
>
> ---------------------
>
> double yPos;
> findBottomModelIndexToAdd(&modelIndex, &yPos);
>
> void findBottomModelIndexToAdd(int *modelIndex, double *yPos);
> void findTopModelIndexToAdd(int *modelIndex, double *yPos);
>
> Any reason why you using double for yPos while everywhere else we are dealing
> with qreals?

Fixed.

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

FAILED: Continuous integration, rev:613
http://jenkins.qa.ubuntu.com/job/unity8-ci/1914/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1775
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1695
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/701
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/437
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/438
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/438/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/437
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1567
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1775
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1775/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1695
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1695/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4197
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2422

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

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

FAILED: Continuous integration, rev:614
http://jenkins.qa.ubuntu.com/job/unity8-ci/1915/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1779
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1699
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/702
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/438
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/439
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/439/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/438
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1571
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1779
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1779/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1699
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1699/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4201
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2426

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

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

Last bits. In verticaljournaltest.cpp:

s/testHorizontalSpacing/testColumnSpacing
s/testVerticalSpacing/testRowSpacing

--------------------

Tests are hard to read (it's a bunch of numbers and to check the correctness of them you have to crunch numbers yourself) but I don't see another way.

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

FAILED: Continuous integration, rev:615
http://jenkins.qa.ubuntu.com/job/unity8-ci/1916/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1784
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1703
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/703
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/439
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/440
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/440/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/439
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1575
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1784
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1784/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1703
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1703/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4205
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2433

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

> I see that the vertical journal is not used anywhere yet. So there's no place
> to play with it. Would it be too much to ask for a "make tryVerticalJournal"
> target that brings up an interactive manual test so one could play with the
> VerticalJournal? I envision a window with a vertical journal on its left side
> and some controls on its right side. One could use those controls to manually
> add an item with a given size and see how the vertical journal places it. And
> click on a placed item to remove it and see now the vertical journal
> reorganizes itself. Similar to what we have with "make tryResponsiveGridView"
> and "make tryStage".
>
> What do you think?

Added

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

> Last bits. In verticaljournaltest.cpp:
>
> s/testHorizontalSpacing/testColumnSpacing
> s/testVerticalSpacing/testRowSpacing
>

Done

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

PASSED: Continuous integration, rev:617
http://jenkins.qa.ubuntu.com/job/unity8-ci/1917/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1789
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1708
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/704
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/440
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/441
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/441/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/440
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1580
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1789
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1789/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1708
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1708/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4209
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2438

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

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

FAILED: Continuous integration, rev:618
http://jenkins.qa.ubuntu.com/job/unity8-ci/1921/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1795
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1714
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/708
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/444
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/445
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/445/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/444
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1584
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1795
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1795/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1714
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1714/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4213
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2442

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

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

Make tryVerticalJournal works nicely, thanks! Good to go.

But I still think you should be less stubborn and start using new the newer, better and more efficient, QObject::connect versions.

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

PASSED: Continuous integration, rev:620
http://jenkins.qa.ubuntu.com/job/unity8-ci/1926/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1817
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1734
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/714
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/449
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/450
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/450/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/449
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1604
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1817
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1817/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1734
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1734/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4230
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2470

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

review: Approve (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/871/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3667
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/1819
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/1736
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/716
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/257
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/257
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/257/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/257
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1606
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1819
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/1819/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1736
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/1736/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4232
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/2472

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 'Dash/ScopeListView.qml'
2--- Dash/ScopeListView.qml 2013-10-11 11:41:27 +0000
3+++ Dash/ScopeListView.qml 2013-12-18 09:51:17 +0000
4@@ -15,7 +15,7 @@
5 */
6
7 import QtQuick 2.0
8-import ListViewWithPageHeader 0.1
9+import DashViews 0.1
10
11 ListViewWithPageHeader {
12 maximumFlickVelocity: height * 10
13
14=== modified file 'debian/unity8-private.install'
15--- debian/unity8-private.install 2013-09-13 21:37:14 +0000
16+++ debian/unity8-private.install 2013-12-18 09:51:17 +0000
17@@ -1,7 +1,7 @@
18 usr/lib/*/unity8/qml/AccountsService
19 usr/lib/*/unity8/qml/HudClient
20 usr/lib/*/unity8/qml/LightDM
21-usr/lib/*/unity8/qml/ListViewWithPageHeader
22+usr/lib/*/unity8/qml/DashViews
23 usr/lib/*/unity8/qml/Powerd
24 usr/lib/*/unity8/qml/SessionBroadcast
25 usr/lib/*/unity8/qml/Ubuntu
26
27=== modified file 'plugins/CMakeLists.txt'
28--- plugins/CMakeLists.txt 2013-09-12 17:58:21 +0000
29+++ plugins/CMakeLists.txt 2013-12-18 09:51:17 +0000
30@@ -1,7 +1,7 @@
31 add_subdirectory(AccountsService)
32 add_subdirectory(HudClient)
33 add_subdirectory(LightDM)
34-add_subdirectory(ListViewWithPageHeader)
35+add_subdirectory(DashViews)
36 add_subdirectory(Powerd)
37 add_subdirectory(SessionBroadcast)
38 add_subdirectory(Ubuntu)
39
40=== renamed directory 'plugins/ListViewWithPageHeader' => 'plugins/DashViews'
41=== modified file 'plugins/DashViews/CMakeLists.txt'
42--- plugins/ListViewWithPageHeader/CMakeLists.txt 2013-07-29 11:21:37 +0000
43+++ plugins/DashViews/CMakeLists.txt 2013-12-18 09:51:17 +0000
44@@ -24,19 +24,20 @@
45 set(QMLPLUGIN_SRC
46 plugin.cpp
47 listviewwithpageheader.cpp
48+ verticaljournal.cpp
49 )
50
51-add_library(ListViewWithPageHeader-qml MODULE
52+add_library(DashViews-qml MODULE
53 ${QMLPLUGIN_SRC}
54 )
55
56-target_link_libraries(ListViewWithPageHeader-qml
57+target_link_libraries(DashViews-qml
58 ${Qt5Gui_LIBRARIES}
59 ${Qt5Quick_LIBRARIES}
60 )
61
62-qt5_use_modules(ListViewWithPageHeader-qml Qml Quick)
63+qt5_use_modules(DashViews-qml Qml Quick)
64
65 # export the qmldir qmltypes and plugin files
66-export_qmlfiles(ListViewWithPageHeader ListViewWithPageHeader)
67-export_qmlplugin(ListViewWithPageHeader 0.1 ListViewWithPageHeader TARGETS ListViewWithPageHeader-qml)
68+export_qmlfiles(DashViews DashViews)
69+export_qmlplugin(DashViews 0.1 DashViews TARGETS DashViews-qml)
70
71=== modified file 'plugins/DashViews/plugin.cpp'
72--- plugins/ListViewWithPageHeader/plugin.cpp 2013-06-07 14:42:51 +0000
73+++ plugins/DashViews/plugin.cpp 2013-12-18 09:51:17 +0000
74@@ -18,12 +18,14 @@
75 #include "plugin.h"
76
77 #include "listviewwithpageheader.h"
78+#include "verticaljournal.h"
79
80 #include <QAbstractItemModel>
81
82-void ListViewWithPageHeaderPlugin::registerTypes(const char *uri)
83+void DashViewsPlugin::registerTypes(const char *uri)
84 {
85- Q_ASSERT(uri == QLatin1String("ListViewWithPageHeader"));
86+ Q_ASSERT(uri == QLatin1String("DashViews"));
87 qmlRegisterType<QAbstractItemModel>();
88 qmlRegisterType<ListViewWithPageHeader>(uri, 0, 1, "ListViewWithPageHeader");
89+ qmlRegisterType<VerticalJournal>(uri, 0, 1, "VerticalJournal");
90 }
91
92=== modified file 'plugins/DashViews/plugin.h'
93--- plugins/ListViewWithPageHeader/plugin.h 2013-06-07 13:52:42 +0000
94+++ plugins/DashViews/plugin.h 2013-12-18 09:51:17 +0000
95@@ -15,13 +15,13 @@
96 *
97 */
98
99-#ifndef LISTVIEWWITHPAGEHEADER_PLUGIN_H
100-#define LISTVIEWWITHPAGEHEADER_PLUGIN_H
101+#ifndef DASHVIEWS_PLUGIN_H
102+#define DASHVIEWS_PLUGIN_H
103
104 #include <QtQml/QQmlEngine>
105 #include <QtQml/QQmlExtensionPlugin>
106
107-class ListViewWithPageHeaderPlugin : public QQmlExtensionPlugin
108+class DashViewsPlugin : public QQmlExtensionPlugin
109 {
110 Q_OBJECT
111 Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QQmlExtensionInterface")
112
113=== modified file 'plugins/DashViews/qmldir'
114--- plugins/ListViewWithPageHeader/qmldir 2013-07-04 09:32:44 +0000
115+++ plugins/DashViews/qmldir 2013-12-18 09:51:17 +0000
116@@ -1,3 +1,3 @@
117-module ListViewWithPageHeader
118-plugin ListViewWithPageHeader-qml
119+module DashViews
120+plugin DashViews-qml
121 typeinfo plugin.qmltypes
122
123=== added file 'plugins/DashViews/verticaljournal.cpp'
124--- plugins/DashViews/verticaljournal.cpp 1970-01-01 00:00:00 +0000
125+++ plugins/DashViews/verticaljournal.cpp 2013-12-18 09:51:17 +0000
126@@ -0,0 +1,590 @@
127+/*
128+ * Copyright (C) 2013 Canonical, Ltd.
129+ *
130+ * This program is free software; you can redistribute it and/or modify
131+ * it under the terms of the GNU General Public License as published by
132+ * the Free Software Foundation; version 3.
133+ *
134+ * This program is distributed in the hope that it will be useful,
135+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
136+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
137+ * GNU General Public License for more details.
138+ *
139+ * You should have received a copy of the GNU General Public License
140+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
141+ */
142+
143+/*
144+ * The implementation is centered around m_columnVisibleItems
145+ * that holds a vector of lists. There's a list for each of the
146+ * columns the view has. In the list the items of the column are
147+ * ordered as they appear topdown in the view. m_indexColumnMap is
148+ * used when re-building the list up since given a position
149+ * in the middle of the list and the need to create the previous does
150+ * not give us enough information to know in which column we have
151+ * to position the item so that when we reach the item the view is
152+ * correctly layouted at 0 for all the columns
153+ */
154+#include "verticaljournal.h"
155+
156+#include <qqmlengine.h>
157+#pragma GCC diagnostic push
158+#pragma GCC diagnostic ignored "-pedantic"
159+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
160+#include <private/qquickvisualdatamodel_p.h>
161+#else
162+#include <private/qqmldelegatemodel_p.h>
163+#include <qqmlinfo.h>
164+#endif
165+#include <private/qquickitem_p.h>
166+#pragma GCC diagnostic pop
167+
168+static const qreal bufferRatio = 0.5;
169+
170+VerticalJournal::VerticalJournal()
171+ : m_delegateModel(nullptr)
172+ , m_asyncRequestedIndex(-1)
173+ , m_columnWidth(0)
174+ , m_columnSpacing(0)
175+ , m_rowSpacing(0)
176+ , m_delegateCreationBegin(0)
177+ , m_delegateCreationEnd(0)
178+ , m_delegateCreationBeginValid(false)
179+ , m_delegateCreationEndValid(false)
180+ , m_needsRelayout(false)
181+ , m_delegateValidated(false)
182+ , m_implicitHeightDirty(false)
183+{
184+ connect(this, SIGNAL(widthChanged()), this, SLOT(relayout()));
185+ connect(this, SIGNAL(heightChanged()), this, SLOT(onHeightChanged()));
186+}
187+
188+QAbstractItemModel *VerticalJournal::model() const
189+{
190+ return m_delegateModel ? m_delegateModel->model().value<QAbstractItemModel *>() : nullptr;
191+}
192+
193+void VerticalJournal::setModel(QAbstractItemModel *model)
194+{
195+ if (model != this->model()) {
196+ if (!m_delegateModel) {
197+ createDelegateModel();
198+ } else {
199+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
200+ disconnect(m_delegateModel, SIGNAL(modelUpdated(QQuickChangeSet,bool)), this, SLOT(onModelUpdated(QQuickChangeSet,bool)));
201+ }
202+ m_delegateModel->setModel(QVariant::fromValue<QAbstractItemModel *>(model));
203+ connect(m_delegateModel, SIGNAL(modelUpdated(QQuickChangeSet,bool)), this, SLOT(onModelUpdated(QQuickChangeSet,bool)));
204+#else
205+ disconnect(m_delegateModel, SIGNAL(modelUpdated(QQmlChangeSet,bool)), this, SLOT(onModelUpdated(QQmlChangeSet,bool)));
206+ }
207+ m_delegateModel->setModel(QVariant::fromValue<QAbstractItemModel *>(model));
208+ connect(m_delegateModel, SIGNAL(modelUpdated(QQmlChangeSet,bool)), this, SLOT(onModelUpdated(QQmlChangeSet,bool)));
209+#endif
210+
211+ cleanupExistingItems();
212+
213+ Q_EMIT modelChanged();
214+ polish();
215+ }
216+}
217+
218+QQmlComponent *VerticalJournal::delegate() const
219+{
220+ return m_delegateModel ? m_delegateModel->delegate() : nullptr;
221+}
222+
223+void VerticalJournal::setDelegate(QQmlComponent *delegate)
224+{
225+ if (delegate != this->delegate()) {
226+ if (!m_delegateModel) {
227+ createDelegateModel();
228+ }
229+
230+ cleanupExistingItems();
231+
232+ m_delegateModel->setDelegate(delegate);
233+
234+ Q_EMIT delegateChanged();
235+ m_delegateValidated = false;
236+ polish();
237+ }
238+}
239+
240+qreal VerticalJournal::columnWidth() const
241+{
242+ return m_columnWidth;
243+}
244+
245+void VerticalJournal::setColumnWidth(qreal columnWidth)
246+{
247+ if (columnWidth != m_columnWidth) {
248+ m_columnWidth = columnWidth;
249+ Q_EMIT columnWidthChanged();
250+
251+ if (isComponentComplete()) {
252+ Q_FOREACH(const auto &column, m_columnVisibleItems) {
253+ Q_FOREACH(const ViewItem &item, column) {
254+ item.m_item->setWidth(columnWidth);
255+ }
256+ }
257+ relayout();
258+ }
259+ }
260+}
261+
262+qreal VerticalJournal::columnSpacing() const
263+{
264+ return m_columnSpacing;
265+}
266+
267+void VerticalJournal::setColumnSpacing(qreal columnSpacing)
268+{
269+ if (columnSpacing != m_columnSpacing) {
270+ m_columnSpacing = columnSpacing;
271+ Q_EMIT columnSpacingChanged();
272+
273+ if (isComponentComplete()) {
274+ relayout();
275+ }
276+ }
277+}
278+
279+qreal VerticalJournal::rowSpacing() const
280+{
281+ return m_rowSpacing;
282+}
283+
284+void VerticalJournal::setRowSpacing(qreal rowSpacing)
285+{
286+ if (rowSpacing != m_rowSpacing) {
287+ m_rowSpacing = rowSpacing;
288+ Q_EMIT rowSpacingChanged();
289+
290+ if (isComponentComplete()) {
291+ relayout();
292+ }
293+ }
294+}
295+
296+qreal VerticalJournal::delegateCreationBegin() const
297+{
298+ return m_delegateCreationBegin;
299+}
300+
301+void VerticalJournal::setDelegateCreationBegin(qreal begin)
302+{
303+ m_delegateCreationBeginValid = true;
304+ if (m_delegateCreationBegin == begin)
305+ return;
306+ m_delegateCreationBegin = begin;
307+ if (isComponentComplete()) {
308+ polish();
309+ }
310+ emit delegateCreationBeginChanged();
311+}
312+
313+void VerticalJournal::resetDelegateCreationBegin()
314+{
315+ m_delegateCreationBeginValid = false;
316+ if (m_delegateCreationBegin == 0)
317+ return;
318+ m_delegateCreationBegin = 0;
319+ if (isComponentComplete()) {
320+ polish();
321+ }
322+ emit delegateCreationBeginChanged();
323+}
324+
325+qreal VerticalJournal::delegateCreationEnd() const
326+{
327+ return m_delegateCreationEnd;
328+}
329+
330+void VerticalJournal::setDelegateCreationEnd(qreal end)
331+{
332+ m_delegateCreationEndValid = true;
333+ if (m_delegateCreationEnd == end)
334+ return;
335+ m_delegateCreationEnd = end;
336+ if (isComponentComplete()) {
337+ polish();
338+ }
339+ emit delegateCreationEndChanged();
340+}
341+
342+void VerticalJournal::resetDelegateCreationEnd()
343+{
344+ m_delegateCreationEndValid = false;
345+ if (m_delegateCreationEnd == 0)
346+ return;
347+ m_delegateCreationEnd = 0;
348+ if (isComponentComplete()) {
349+ polish();
350+ }
351+ emit delegateCreationEndChanged();
352+}
353+
354+void VerticalJournal::createDelegateModel()
355+{
356+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
357+ m_delegateModel = new QQuickVisualDataModel(qmlContext(this), this);
358+ connect(m_delegateModel, SIGNAL(createdItem(int,QQuickItem*)), this, SLOT(itemCreated(int,QQuickItem*)));
359+#else
360+ m_delegateModel = new QQmlDelegateModel(qmlContext(this), this);
361+ connect(m_delegateModel, SIGNAL(createdItem(int,QObject*)), this, SLOT(itemCreated(int,QObject*)));
362+#endif
363+ if (isComponentComplete())
364+ m_delegateModel->componentComplete();
365+}
366+
367+void VerticalJournal::refill()
368+{
369+ if (!isComponentComplete()) {
370+ return;
371+ }
372+
373+ const bool delegateRangesValid = m_delegateCreationBeginValid && m_delegateCreationEndValid;
374+ const qreal from = delegateRangesValid ? m_delegateCreationBegin : 0;
375+ const qreal to = delegateRangesValid ? m_delegateCreationEnd : from + height();
376+ const qreal buffer = (to - from) * bufferRatio;
377+ const qreal bufferFrom = from - buffer;
378+ const qreal bufferTo = to + buffer;
379+
380+ bool added = addVisibleItems(from, to, false);
381+ bool removed = removeNonVisibleItems(bufferFrom, bufferTo);
382+ added |= addVisibleItems(bufferFrom, bufferTo, true);
383+
384+ if (added || removed) {
385+ m_implicitHeightDirty = true;
386+ }
387+}
388+
389+void VerticalJournal::findBottomModelIndexToAdd(int *modelIndex, qreal *yPos)
390+{
391+ *modelIndex = 0;
392+ *yPos = std::numeric_limits<qreal>::max();
393+
394+ Q_FOREACH(const auto &column, m_columnVisibleItems) {
395+ if (!column.isEmpty()) {
396+ const ViewItem &item = column.last();
397+ *yPos = qMin(*yPos, item.y() + item.height() + m_rowSpacing);
398+ *modelIndex = qMax(*modelIndex, item.m_modelIndex + 1);
399+ } else {
400+ *yPos = 0;
401+ }
402+ }
403+}
404+
405+void VerticalJournal::findTopModelIndexToAdd(int *modelIndex, qreal *yPos)
406+{
407+ *modelIndex = INT_MAX;
408+ *yPos = std::numeric_limits<qreal>::lowest();
409+ int columnToAddTo = -1;
410+
411+ // Find the topmost free column
412+ for (int i = 0; i < m_columnVisibleItems.count(); ++i) {
413+ const auto &column = m_columnVisibleItems[i];
414+ if (!column.isEmpty()) {
415+ const ViewItem &item = column.first();
416+ const auto itemTopPos = item.y() - m_rowSpacing;
417+ if (itemTopPos > *yPos) {
418+ *yPos = itemTopPos;
419+ *modelIndex = item.m_modelIndex - 1;
420+ columnToAddTo = i;
421+ }
422+ }
423+ }
424+
425+ if (*modelIndex > 0) {
426+ Q_ASSERT(m_indexColumnMap.contains(*modelIndex));
427+ while (m_indexColumnMap[*modelIndex] != columnToAddTo) {
428+ // We found out that we have to add to columnToAddTo
429+ // and thought that we had to add *modelIndex, but history tells
430+ // it is not correct, so find up from *modelIndex until we found the index
431+ // that has to end up in columnToAddTo
432+ *modelIndex = *modelIndex - 1;
433+ Q_ASSERT(m_indexColumnMap.contains(*modelIndex));
434+ }
435+ }
436+}
437+
438+bool VerticalJournal::addVisibleItems(qreal fillFrom, qreal fillTo, bool asynchronous)
439+{
440+ if (!delegate())
441+ return false;
442+
443+ if (m_delegateModel->count() == 0)
444+ return false;
445+
446+ int modelIndex;
447+ qreal yPos;
448+ findBottomModelIndexToAdd(&modelIndex, &yPos);
449+ bool changed = false;
450+ while (modelIndex < m_delegateModel->count() && yPos <= fillTo) {
451+ if (!createItem(modelIndex, asynchronous))
452+ break;
453+
454+ changed = true;
455+ findBottomModelIndexToAdd(&modelIndex, &yPos);
456+ }
457+
458+ findTopModelIndexToAdd(&modelIndex, &yPos);
459+ while (modelIndex >= 0 && yPos > fillFrom) {
460+ if (!createItem(modelIndex, asynchronous))
461+ break;
462+
463+ changed = true;
464+ findTopModelIndexToAdd(&modelIndex, &yPos);
465+ }
466+
467+ return changed;
468+}
469+
470+bool VerticalJournal::removeNonVisibleItems(qreal bufferFrom, qreal bufferTo)
471+{
472+ bool changed = false;
473+
474+ for (int i = 0; i < m_columnVisibleItems.count(); ++i) {
475+ QList<ViewItem> &column = m_columnVisibleItems[i];
476+ while (!column.isEmpty() && column.first().y() + column.first().height() < bufferFrom) {
477+ releaseItem(column.takeFirst());
478+ changed = true;
479+ }
480+
481+ while (!column.isEmpty() && column.last().y() > bufferTo) {
482+ releaseItem(column.takeLast());
483+ changed = true;
484+ }
485+ }
486+
487+ return changed;
488+}
489+
490+QQuickItem *VerticalJournal::createItem(int modelIndex, bool asynchronous)
491+{
492+ if (asynchronous && m_asyncRequestedIndex != -1)
493+ return nullptr;
494+
495+ m_asyncRequestedIndex = -1;
496+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
497+ QQuickItem *item = m_delegateModel->item(modelIndex, asynchronous);
498+#else
499+ QObject* object = m_delegateModel->object(modelIndex, asynchronous);
500+ QQuickItem *item = qmlobject_cast<QQuickItem*>(object);
501+#endif
502+ if (!item) {
503+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
504+ m_asyncRequestedIndex = modelIndex;
505+#else
506+ if (object) {
507+ m_delegateModel->release(object);
508+ if (!m_delegateValidated) {
509+ m_delegateValidated = true;
510+ QObject* delegateObj = delegate();
511+ qmlInfo(delegateObj ? delegateObj : this) << "Delegate must be of Item type";
512+ }
513+ } else {
514+ m_asyncRequestedIndex = modelIndex;
515+ }
516+#endif
517+ return nullptr;
518+ } else {
519+ if (item->width() != m_columnWidth) {
520+ qWarning() << "Item" << modelIndex << "width is not the one that the columnWidth mandates, resetting it";
521+ item->setWidth(m_columnWidth);
522+ }
523+ positionItem(modelIndex, item);
524+ return item;
525+ }
526+}
527+
528+void VerticalJournal::positionItem(int modelIndex, QQuickItem *item)
529+{
530+ // Check if we add it to the bottom of existing column items
531+ const QList<ViewItem> &firstColumn = m_columnVisibleItems[0];
532+ qreal columnToAddY = !firstColumn.isEmpty() ? firstColumn.last().y() + firstColumn.last().height() : -m_rowSpacing;
533+ int columnToAddTo = 0;
534+ for (int i = 1; i < m_columnVisibleItems.count(); ++i) {
535+ const QList<ViewItem> &column = m_columnVisibleItems[i];
536+ const qreal iY = !column.isEmpty() ? column.last().y() + column.last().height() : -m_rowSpacing;
537+ if (iY < columnToAddY) {
538+ columnToAddTo = i;
539+ columnToAddY = iY;
540+ }
541+ }
542+
543+ const QList<ViewItem> &columnToAdd = m_columnVisibleItems[columnToAddTo];
544+ if (columnToAdd.isEmpty() || columnToAdd.last().m_modelIndex < modelIndex) {
545+ item->setX(columnToAddTo * (m_columnWidth + m_columnSpacing));
546+ item->setY(columnToAddY + m_rowSpacing);
547+
548+ m_columnVisibleItems[columnToAddTo] << ViewItem(item, modelIndex);
549+ m_indexColumnMap[modelIndex] = columnToAddTo;
550+ } else {
551+ Q_ASSERT(m_indexColumnMap.contains(modelIndex));
552+ columnToAddTo = m_indexColumnMap[modelIndex];
553+ columnToAddY = m_columnVisibleItems[columnToAddTo].first().y();
554+
555+ item->setX(columnToAddTo * (m_columnWidth + m_columnSpacing));
556+ item->setY(columnToAddY - m_rowSpacing - item->height());
557+
558+ m_columnVisibleItems[columnToAddTo].prepend(ViewItem(item, modelIndex));
559+ }
560+}
561+
562+void VerticalJournal::cleanupExistingItems()
563+{
564+ // Cleanup the existing items
565+ for (int i = 0; i < m_columnVisibleItems.count(); ++i) {
566+ QList<ViewItem> &column = m_columnVisibleItems[i];
567+ Q_FOREACH(const ViewItem &item, column)
568+ releaseItem(item);
569+ column.clear();
570+ }
571+ m_indexColumnMap.clear();
572+}
573+
574+void VerticalJournal::releaseItem(const ViewItem &item)
575+{
576+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
577+ QQuickVisualModel::ReleaseFlags flags = m_delegateModel->release(item.m_item);
578+ if (flags & QQuickVisualModel::Destroyed) {
579+#else
580+ QQmlDelegateModel::ReleaseFlags flags = m_delegateModel->release(item.m_item);
581+ if (flags & QQmlDelegateModel::Destroyed) {
582+#endif
583+ item.m_item->setParentItem(nullptr);
584+ }
585+}
586+
587+void VerticalJournal::calculateImplicitHeight()
588+{
589+ m_implicitHeightDirty = false;
590+
591+ int lastModelIndex = -1;
592+ qreal bottomMostY = 0;
593+ Q_FOREACH(const auto &column, m_columnVisibleItems) {
594+ if (!column.isEmpty()) {
595+ const ViewItem &item = column.last();
596+ lastModelIndex = qMax(lastModelIndex, item.m_modelIndex);
597+ bottomMostY = qMax(bottomMostY, item.y() + item.height());
598+ }
599+ }
600+ if (lastModelIndex >= 0) {
601+ const double averageHeight = bottomMostY / (lastModelIndex + 1);
602+ setImplicitHeight(bottomMostY + averageHeight * (model()->rowCount() - lastModelIndex - 1));
603+ } else {
604+ setImplicitHeight(0);
605+ }
606+}
607+
608+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
609+void VerticalJournal::itemCreated(int modelIndex, QQuickItem *item)
610+{
611+#else
612+void VerticalJournal::itemCreated(int modelIndex, QObject *object)
613+{
614+ QQuickItem *item = qmlobject_cast<QQuickItem*>(object);
615+ if (!item) {
616+ qWarning() << "VerticalJournal::itemCreated got a non item for index" << modelIndex;
617+ return;
618+ }
619+#endif
620+ item->setParentItem(this);
621+ if (modelIndex == m_asyncRequestedIndex) {
622+ createItem(modelIndex, false);
623+ m_implicitHeightDirty = true;
624+ polish();
625+ }
626+}
627+
628+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
629+void VerticalJournal::onModelUpdated(const QQuickChangeSet & /*changeSet*/, bool reset)
630+#else
631+void VerticalJournal::onModelUpdated(const QQmlChangeSet & /*changeSet*/, bool reset)
632+#endif
633+{
634+ // TODO Add support for additions/removals at end that are not reset calls
635+ // TODO Add a test for reset
636+ if (reset) {
637+ cleanupExistingItems();
638+ }
639+ polish();
640+}
641+
642+
643+void VerticalJournal::relayout()
644+{
645+ m_needsRelayout = true;
646+ polish();
647+}
648+
649+void VerticalJournal::onHeightChanged()
650+{
651+ polish();
652+}
653+
654+void VerticalJournal::updatePolish()
655+{
656+ if (!model())
657+ return;
658+
659+ if (m_needsRelayout) {
660+ m_implicitHeightDirty = true;
661+
662+ QList<ViewItem> allItems;
663+ Q_FOREACH(const auto &column, m_columnVisibleItems)
664+ allItems << column;
665+
666+ qSort(allItems);
667+
668+ const int nColumns = qMax(1., floor((double)(width() + m_columnSpacing) / (m_columnWidth + m_columnSpacing)));
669+ m_columnVisibleItems.resize(nColumns);
670+ m_indexColumnMap.clear();
671+ for (int i = 0; i < nColumns; ++i)
672+ m_columnVisibleItems[i].clear();
673+
674+ // If the first of allItems doesn't contain index 0 we need to drop them
675+ // all since we can't consistently relayout without the first item being there
676+
677+ if (!allItems.isEmpty()) {
678+ if (allItems.first().m_modelIndex == 0) {
679+ Q_FOREACH(const ViewItem &item, allItems)
680+ positionItem(item.m_modelIndex, item.m_item);
681+ } else {
682+ Q_FOREACH(const ViewItem &item, allItems)
683+ releaseItem(item);
684+ }
685+ }
686+
687+ m_needsRelayout = false;
688+ }
689+
690+ refill();
691+
692+ const bool delegateRangesValid = m_delegateCreationBeginValid && m_delegateCreationEndValid;
693+ const qreal from = delegateRangesValid ? m_delegateCreationBegin : 0;
694+ const qreal to = delegateRangesValid ? m_delegateCreationEnd : from + height();
695+ Q_FOREACH(const auto &column, m_columnVisibleItems) {
696+ Q_FOREACH(const ViewItem &item, column) {
697+ QQuickItemPrivate::get(item.m_item)->setCulled(item.y() + item.height() <= from || item.y() >= to);
698+ }
699+ }
700+
701+ if (m_implicitHeightDirty) {
702+ calculateImplicitHeight();
703+ }
704+}
705+
706+void VerticalJournal::componentComplete()
707+{
708+ if (m_delegateModel)
709+ m_delegateModel->componentComplete();
710+
711+ QQuickItem::componentComplete();
712+
713+ m_needsRelayout = true;
714+
715+ polish();
716+}
717
718=== added file 'plugins/DashViews/verticaljournal.h'
719--- plugins/DashViews/verticaljournal.h 1970-01-01 00:00:00 +0000
720+++ plugins/DashViews/verticaljournal.h 2013-12-18 09:51:17 +0000
721@@ -0,0 +1,182 @@
722+/*
723+ * Copyright (C) 2013 Canonical, Ltd.
724+ *
725+ * This program is free software; you can redistribute it and/or modify
726+ * it under the terms of the GNU General Public License as published by
727+ * the Free Software Foundation; version 3.
728+ *
729+ * This program is distributed in the hope that it will be useful,
730+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
731+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
732+ * GNU General Public License for more details.
733+ *
734+ * You should have received a copy of the GNU General Public License
735+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
736+ */
737+
738+#ifndef VERTICALJOURNAL_H
739+#define VERTICALJOURNAL_H
740+
741+#include <QQuickItem>
742+
743+class QAbstractItemModel;
744+class QQmlComponent;
745+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
746+class QQuickChangeSet;
747+class QQuickVisualDataModel;
748+#else
749+class QQmlChangeSet;
750+class QQmlDelegateModel;
751+#endif
752+
753+ /** A vertical journal is a view that creates delegates
754+ * based on a model and layouts them in columns following
755+ * a top-left most position rule.
756+ *
757+ * The number of columns is calculated using the width of
758+ * the view itself, the columnWidth (i.e. the width of each individual delegate)
759+ * and the columnSpacing between columns.
760+ *
761+ * All delegates are forced to columnWidth if they don't have it.
762+ *
763+ * The first nColumns items are layouted at row 0 from column 0
764+ * to column nColumns-1 in order. After that every new item
765+ * is positioned in the column which provides the free topmost
766+ * position as possible. If more than one column tie in providing
767+ * the topmost free position the leftmost column will be used.
768+ *
769+ * Example:
770+ *
771+ * +-----+ +-----+ +-----+
772+ * | | | 2 | | |
773+ * | | | | | |
774+ * | 1 | +-----+ | 3 |
775+ * | | +-----+ | |
776+ * | | | | +-----+
777+ * +-----+ | 4 | +-----+
778+ * +-----+ | | | 5 |
779+ * | 6 | +-----+ | |
780+ * | | +-----+
781+ * +-----+
782+ *
783+ */
784+ class VerticalJournal : public QQuickItem
785+{
786+ Q_OBJECT
787+
788+ Q_PROPERTY(QAbstractItemModel *model READ model WRITE setModel NOTIFY modelChanged)
789+ Q_PROPERTY(QQmlComponent *delegate READ delegate WRITE setDelegate NOTIFY delegateChanged)
790+ Q_PROPERTY(qreal columnWidth READ columnWidth WRITE setColumnWidth NOTIFY columnWidthChanged)
791+ Q_PROPERTY(qreal columnSpacing READ columnSpacing WRITE setColumnSpacing NOTIFY columnSpacingChanged)
792+ Q_PROPERTY(qreal rowSpacing READ rowSpacing WRITE setRowSpacing NOTIFY rowSpacingChanged)
793+ Q_PROPERTY(qreal delegateCreationBegin READ delegateCreationBegin
794+ WRITE setDelegateCreationBegin
795+ NOTIFY delegateCreationBeginChanged
796+ RESET resetDelegateCreationBegin)
797+ Q_PROPERTY(qreal delegateCreationEnd READ delegateCreationEnd
798+ WRITE setDelegateCreationEnd
799+ NOTIFY delegateCreationEndChanged
800+ RESET resetDelegateCreationEnd)
801+
802+friend class VerticalJournalTest;
803+
804+public:
805+ VerticalJournal();
806+
807+ QAbstractItemModel *model() const;
808+ void setModel(QAbstractItemModel *model);
809+
810+ QQmlComponent *delegate() const;
811+ void setDelegate(QQmlComponent *delegate);
812+
813+ qreal columnWidth() const;
814+ void setColumnWidth(qreal columnWidth);
815+
816+ qreal columnSpacing() const;
817+ void setColumnSpacing(qreal columnSpacing);
818+
819+ qreal rowSpacing() const;
820+ void setRowSpacing(qreal rowSpacing);
821+
822+ qreal delegateCreationBegin() const;
823+ void setDelegateCreationBegin(qreal);
824+ void resetDelegateCreationBegin();
825+
826+ qreal delegateCreationEnd() const;
827+ void setDelegateCreationEnd(qreal);
828+ void resetDelegateCreationEnd();
829+
830+Q_SIGNALS:
831+ void modelChanged();
832+ void delegateChanged();
833+ void columnWidthChanged();
834+ void columnSpacingChanged();
835+ void rowSpacingChanged();
836+ void delegateCreationBeginChanged();
837+ void delegateCreationEndChanged();
838+
839+protected:
840+ void updatePolish();
841+ void componentComplete();
842+
843+private Q_SLOTS:
844+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
845+ void itemCreated(int modelIndex, QQuickItem *item);
846+ void onModelUpdated(const QQuickChangeSet &changeSet, bool reset);
847+#else
848+ void itemCreated(int modelIndex, QObject *object);
849+ void onModelUpdated(const QQmlChangeSet &changeSet, bool reset);
850+#endif
851+ void relayout();
852+ void onHeightChanged();
853+
854+private:
855+ class ViewItem
856+ {
857+ public:
858+ ViewItem(QQuickItem *item, int modelIndex) : m_item(item), m_modelIndex(modelIndex) {}
859+ qreal x() const { return m_item->x(); }
860+ qreal y() const { return m_item->y(); }
861+ qreal height() const { return m_item->height(); }
862+ bool operator<(const ViewItem &v) const { return m_modelIndex < v.m_modelIndex; }
863+
864+ QQuickItem *m_item;
865+ int m_modelIndex;
866+ };
867+
868+ void createDelegateModel();
869+ void refill();
870+ void findBottomModelIndexToAdd(int *modelIndex, qreal *yPos);
871+ void findTopModelIndexToAdd(int *modelIndex, qreal *yPos);
872+ bool addVisibleItems(qreal fillFrom, qreal fillTo, bool asynchronous);
873+ bool removeNonVisibleItems(qreal bufferFrom, qreal bufferTo);
874+ QQuickItem *createItem(int modelIndex, bool asynchronous);
875+ void positionItem(int modelIndex, QQuickItem *item);
876+ void cleanupExistingItems();
877+ void releaseItem(const ViewItem &item);
878+ void calculateImplicitHeight();
879+
880+#if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
881+ QQuickVisualDataModel *m_delegateModel;
882+#else
883+ QQmlDelegateModel *m_delegateModel;
884+#endif
885+
886+ // Index we are waiting because we requested it asynchronously
887+ int m_asyncRequestedIndex;
888+
889+ QVector<QList<ViewItem>> m_columnVisibleItems;
890+ QHash<int, int> m_indexColumnMap;
891+ int m_columnWidth;
892+ int m_columnSpacing;
893+ int m_rowSpacing;
894+ qreal m_delegateCreationBegin;
895+ qreal m_delegateCreationEnd;
896+ bool m_delegateCreationBeginValid;
897+ bool m_delegateCreationEndValid;
898+ bool m_needsRelayout;
899+ bool m_delegateValidated;
900+ bool m_implicitHeightDirty;
901+};
902+
903+#endif
904
905=== modified file 'tests/plugins/CMakeLists.txt'
906--- tests/plugins/CMakeLists.txt 2013-12-03 11:43:15 +0000
907+++ tests/plugins/CMakeLists.txt 2013-12-18 09:51:17 +0000
908@@ -1,6 +1,6 @@
909 add_subdirectory(AccountsService)
910 add_subdirectory(LightDM)
911-add_subdirectory(ListViewWithPageHeader)
912+add_subdirectory(DashViews)
913 add_subdirectory(Ubuntu)
914 add_subdirectory(Unity)
915 add_subdirectory(Utils)
916
917=== renamed directory 'tests/plugins/ListViewWithPageHeader' => 'tests/plugins/DashViews'
918=== modified file 'tests/plugins/DashViews/CMakeLists.txt'
919--- tests/plugins/ListViewWithPageHeader/CMakeLists.txt 2013-11-14 10:38:25 +0000
920+++ tests/plugins/DashViews/CMakeLists.txt 2013-12-18 09:51:17 +0000
921@@ -16,30 +16,40 @@
922 ${Qt5Quick_INCLUDE_DIRS}
923 ${Qt5Quick_PRIVATE_INCLUDE_DIRS}
924 ${Qt5V8_PRIVATE_INCLUDE_DIR}
925- ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/ListViewWithPageHeader
926+ ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/DashViews
927 ${CMAKE_CURRENT_BINARY_DIR}
928 )
929
930 remove_definitions(-DQT_NO_KEYWORDS)
931
932-add_definitions(-DLISTVIEWWITHPAGEHEADER_FOLDER="${CMAKE_CURRENT_SOURCE_DIR}")
933+add_definitions(-DDASHVIEWSTEST_FOLDER="${CMAKE_CURRENT_SOURCE_DIR}")
934 add_definitions(-DBUILT_PLUGINS_DIR="${CMAKE_BINARY_DIR}/plugins")
935
936-add_executable(listviewwithpageheadertestExec listviewwithpageheadertest.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/ListViewWithPageHeader/listviewwithpageheader.cpp)
937+add_executable(listviewwithpageheadertestExec listviewwithpageheadertest.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/DashViews/listviewwithpageheader.cpp)
938 qt5_use_modules(listviewwithpageheadertestExec Test Core Qml)
939 target_link_libraries(listviewwithpageheadertestExec ${Qt5Gui_LIBRARIES} ${Qt5Quick_LIBRARIES})
940 add_custom_target(testlistviewwithpageheader ${CMAKE_CURRENT_BINARY_DIR}/listviewwithpageheadertestExec -o ${CMAKE_BINARY_DIR}/testlistviewwithpageheader.xml,xunitxml -o -,txt)
941
942-add_executable(listviewwithpageheadertestsectionExec listviewwithpageheadertestsection.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/ListViewWithPageHeader/listviewwithpageheader.cpp)
943+add_executable(listviewwithpageheadertestsectionExec listviewwithpageheadertestsection.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/DashViews/listviewwithpageheader.cpp)
944 qt5_use_modules(listviewwithpageheadertestsectionExec Test Core Qml)
945 target_link_libraries(listviewwithpageheadertestsectionExec ${Qt5Gui_LIBRARIES} ${Qt5Quick_LIBRARIES})
946 add_custom_target(testlistviewwithpageheadersection ${CMAKE_CURRENT_BINARY_DIR}/listviewwithpageheadertestsectionExec -o ${CMAKE_BINARY_DIR}/testlistviewwithpageheadersection.xml,xunitxml -o -,txt)
947
948-add_executable(listviewwithpageheadertestsectionexternalmodelExec listviewwithpageheadertestsectionexternalmodel.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/ListViewWithPageHeader/listviewwithpageheader.cpp)
949+add_executable(listviewwithpageheadertestsectionexternalmodelExec listviewwithpageheadertestsectionexternalmodel.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/DashViews/listviewwithpageheader.cpp)
950 qt5_use_modules(listviewwithpageheadertestsectionexternalmodelExec Test Core Qml)
951 target_link_libraries(listviewwithpageheadertestsectionexternalmodelExec ${Qt5Gui_LIBRARIES} ${Qt5Quick_LIBRARIES})
952 add_custom_target(testlistviewwithpageheadersectionexternalmodel ${CMAKE_CURRENT_BINARY_DIR}/listviewwithpageheadertestsectionexternalmodelExec -o ${CMAKE_BINARY_DIR}/testlistviewwithpageheadersectionexternalmodel.xml,xunitxml -o -,txt)
953
954+add_executable(verticaljournaltestExec verticaljournaltest.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/DashViews/verticaljournal.cpp)
955+qt5_use_modules(verticaljournaltestExec Test Core Qml)
956+target_link_libraries(verticaljournaltestExec ${Qt5Gui_LIBRARIES} ${Qt5Quick_LIBRARIES})
957+add_custom_target(testVerticalJournal ${CMAKE_CURRENT_BINARY_DIR}/verticaljournaltestExec -o ${CMAKE_BINARY_DIR}/testVerticalJournal.xml,xunitxml -o -,txt)
958+
959+add_executable(verticaljournaltryExec verticaljournaltry.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../../../plugins/DashViews/verticaljournal.cpp)
960+qt5_use_modules(verticaljournaltryExec Test Core Qml)
961+target_link_libraries(verticaljournaltryExec ${Qt5Gui_LIBRARIES} ${Qt5Quick_LIBRARIES})
962+add_custom_target(tryVerticalJournal ${CMAKE_CURRENT_BINARY_DIR}/verticaljournaltryExec -o ${CMAKE_BINARY_DIR}/testVerticalJournal.xml,xunitxml -o -,txt)
963+
964 set(qmltest_DEFAULT_NO_ADD_TEST TRUE)
965 add_qml_test(. listviewwithpageheaderqml IMPORT_PATHS ${CMAKE_BINARY_DIR}/plugins)
966
967@@ -47,3 +57,4 @@
968 add_dependencies(qmluitests testlistviewwithpageheadersection)
969 add_dependencies(qmluitests testlistviewwithpageheadersectionexternalmodel)
970 add_dependencies(qmluitests testlistviewwithpageheaderqml)
971+add_dependencies(qmluitests testVerticalJournal)
972
973=== modified file 'tests/plugins/DashViews/listviewwithpageheadertest.cpp'
974--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-11-19 08:36:34 +0000
975+++ tests/plugins/DashViews/listviewwithpageheadertest.cpp 2013-12-18 09:51:17 +0000
976@@ -118,7 +118,7 @@
977 {
978 view = new QQuickView();
979 view->engine()->addImportPath(BUILT_PLUGINS_DIR);
980- view->setSource(QUrl::fromLocalFile(LISTVIEWWITHPAGEHEADER_FOLDER "/test.qml"));
981+ view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/listviewwithpageheadertest.qml"));
982 lvwph = dynamic_cast<ListViewWithPageHeader*>(view->rootObject()->findChild<QQuickFlickable*>());
983 #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
984 model = view->rootObject()->findChild<QQuickListModel*>();
985
986=== renamed file 'tests/plugins/ListViewWithPageHeader/test.qml' => 'tests/plugins/DashViews/listviewwithpageheadertest.qml'
987--- tests/plugins/ListViewWithPageHeader/test.qml 2013-06-21 16:13:51 +0000
988+++ tests/plugins/DashViews/listviewwithpageheadertest.qml 2013-12-18 09:51:17 +0000
989@@ -15,7 +15,7 @@
990 */
991
992 import QtQuick 2.0
993-import ListViewWithPageHeader 0.1
994+import DashViews 0.1
995
996 Rectangle {
997 width: 300
998
999=== modified file 'tests/plugins/DashViews/listviewwithpageheadertestsection.cpp'
1000--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp 2013-11-05 10:35:41 +0000
1001+++ tests/plugins/DashViews/listviewwithpageheadertestsection.cpp 2013-12-18 09:51:17 +0000
1002@@ -133,7 +133,7 @@
1003 {
1004 view = new QQuickView();
1005 view->engine()->addImportPath(BUILT_PLUGINS_DIR);
1006- view->setSource(QUrl::fromLocalFile(LISTVIEWWITHPAGEHEADER_FOLDER "/test_section.qml"));
1007+ view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/listviewwithpageheadertestsection.qml"));
1008 lvwph = dynamic_cast<ListViewWithPageHeader*>(view->rootObject()->findChild<QQuickFlickable*>());
1009 #if (QT_VERSION < QT_VERSION_CHECK(5, 1, 0))
1010 model = view->rootObject()->findChild<QQuickListModel*>();
1011
1012=== renamed file 'tests/plugins/ListViewWithPageHeader/test_section.qml' => 'tests/plugins/DashViews/listviewwithpageheadertestsection.qml'
1013--- tests/plugins/ListViewWithPageHeader/test_section.qml 2013-06-21 16:13:51 +0000
1014+++ tests/plugins/DashViews/listviewwithpageheadertestsection.qml 2013-12-18 09:51:17 +0000
1015@@ -15,7 +15,7 @@
1016 */
1017
1018 import QtQuick 2.0
1019-import ListViewWithPageHeader 0.1
1020+import DashViews 0.1
1021
1022 Rectangle {
1023 width: 300
1024
1025=== modified file 'tests/plugins/DashViews/listviewwithpageheadertestsectionexternalmodel.cpp'
1026--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsectionexternalmodel.cpp 2013-09-02 13:59:38 +0000
1027+++ tests/plugins/DashViews/listviewwithpageheadertestsectionexternalmodel.cpp 2013-12-18 09:51:17 +0000
1028@@ -173,7 +173,7 @@
1029 {
1030 view = new QQuickView();
1031 view->engine()->addImportPath(BUILT_PLUGINS_DIR);
1032- view->setSource(QUrl::fromLocalFile(LISTVIEWWITHPAGEHEADER_FOLDER "/test_sectionexternalmodel.qml"));
1033+ view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/listviewwithpageheadertestsectionexternalmodel.qml"));
1034 lvwph = dynamic_cast<ListViewWithPageHeader*>(view->rootObject()->findChild<QQuickFlickable*>());
1035 QVERIFY(lvwph);
1036 view->show();
1037
1038=== renamed file 'tests/plugins/ListViewWithPageHeader/test_sectionexternalmodel.qml' => 'tests/plugins/DashViews/listviewwithpageheadertestsectionexternalmodel.qml'
1039--- tests/plugins/ListViewWithPageHeader/test_sectionexternalmodel.qml 2013-09-02 13:59:38 +0000
1040+++ tests/plugins/DashViews/listviewwithpageheadertestsectionexternalmodel.qml 2013-12-18 09:51:17 +0000
1041@@ -15,7 +15,7 @@
1042 */
1043
1044 import QtQuick 2.0
1045-import ListViewWithPageHeader 0.1
1046+import DashViews 0.1
1047
1048 Rectangle {
1049 width: 300
1050
1051=== modified file 'tests/plugins/DashViews/tst_listviewwithpageheaderqml.qml'
1052--- tests/plugins/ListViewWithPageHeader/tst_listviewwithpageheaderqml.qml 2013-11-14 10:42:51 +0000
1053+++ tests/plugins/DashViews/tst_listviewwithpageheaderqml.qml 2013-12-18 09:51:17 +0000
1054@@ -16,7 +16,7 @@
1055
1056 import QtQuick 2.0
1057 import QtTest 1.0
1058-import ListViewWithPageHeader 0.1
1059+import DashViews 0.1
1060
1061 Rectangle {
1062 width: 300
1063
1064=== added file 'tests/plugins/DashViews/verticaljournaltest.cpp'
1065--- tests/plugins/DashViews/verticaljournaltest.cpp 1970-01-01 00:00:00 +0000
1066+++ tests/plugins/DashViews/verticaljournaltest.cpp 2013-12-18 09:51:17 +0000
1067@@ -0,0 +1,315 @@
1068+/*
1069+ * Copyright (C) 2013 Canonical, Ltd.
1070+ *
1071+ * This program is free software; you can redistribute it and/or modify
1072+ * it under the terms of the GNU General Public License as published by
1073+ * the Free Software Foundation; version 3.
1074+ *
1075+ * This program is distributed in the hope that it will be useful,
1076+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
1077+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1078+ * GNU General Public License for more details.
1079+ *
1080+ * You should have received a copy of the GNU General Public License
1081+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
1082+ */
1083+
1084+#include <QQuickView>
1085+#include <QtTestGui>
1086+#include <QDebug>
1087+#include <QGuiApplication>
1088+#include <QQuickView>
1089+#include <QtQml/qqml.h>
1090+#include <QStringListModel>
1091+#include <QQmlContext>
1092+#include <QQmlEngine>
1093+#pragma GCC diagnostic push
1094+#pragma GCC diagnostic ignored "-pedantic"
1095+#include <private/qquickitem_p.h>
1096+#pragma GCC diagnostic pop
1097+
1098+#include "verticaljournal.h"
1099+
1100+class QHeightModel : public QStringListModel {
1101+public:
1102+ QHash<int, QByteArray> roleNames() const
1103+ {
1104+ QHash<int, QByteArray> roles;
1105+ roles.insert(Qt::DisplayRole, "modelHeight");
1106+ return roles;
1107+ }
1108+};
1109+
1110+class VerticalJournalTest : public QObject
1111+{
1112+ Q_OBJECT
1113+
1114+private:
1115+ void verifyItem(const VerticalJournal::ViewItem &item, int modelIndex, qreal x, qreal y, bool visible)
1116+ {
1117+ QTRY_COMPARE(item.m_modelIndex, modelIndex);
1118+ QTRY_COMPARE(item.x(), x);
1119+ QTRY_COMPARE(item.y(), y);
1120+ QTRY_COMPARE(item.height(), heightList[modelIndex].toDouble());
1121+ QTRY_COMPARE(QQuickItemPrivate::get(item.m_item)->culled, !visible);
1122+ }
1123+
1124+ void checkInitialPositions()
1125+ {
1126+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
1127+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 5);
1128+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 7);
1129+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 6);
1130+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
1131+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
1132+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
1133+ verifyItem(vj->m_columnVisibleItems[1][1], 3, 160, 60, true);
1134+ verifyItem(vj->m_columnVisibleItems[1][2], 4, 160, 80, true);
1135+ verifyItem(vj->m_columnVisibleItems[0][1], 5, 0, 110, true);
1136+ verifyItem(vj->m_columnVisibleItems[1][3], 6, 160, 130, true);
1137+ verifyItem(vj->m_columnVisibleItems[2][1], 7, 320, 135, true);
1138+ verifyItem(vj->m_columnVisibleItems[0][2], 8, 0, 190, true);
1139+ verifyItem(vj->m_columnVisibleItems[2][2], 9, 320, 255, true);
1140+ verifyItem(vj->m_columnVisibleItems[2][3], 10, 320, 285, true);
1141+ verifyItem(vj->m_columnVisibleItems[2][4], 11, 320, 315, true);
1142+ verifyItem(vj->m_columnVisibleItems[1][4], 12, 160, 340, true);
1143+ verifyItem(vj->m_columnVisibleItems[0][3], 13, 0, 360, true);
1144+ verifyItem(vj->m_columnVisibleItems[2][5], 14, 320, 390, true);
1145+ verifyItem(vj->m_columnVisibleItems[1][5], 15, 160, 430, false);
1146+ verifyItem(vj->m_columnVisibleItems[0][4], 16, 0, 570, false);
1147+ verifyItem(vj->m_columnVisibleItems[1][6], 17, 160, 570, false);
1148+ QCOMPARE(vj->implicitHeight(), 970. + 2. * 970. / 18.);
1149+ }
1150+
1151+private Q_SLOTS:
1152+ void init()
1153+ {
1154+ view = new QQuickView();
1155+ view->setResizeMode(QQuickView::SizeRootObjectToView);
1156+ view->engine()->addImportPath(BUILT_PLUGINS_DIR);
1157+
1158+ model = new QHeightModel();
1159+ heightList.clear();
1160+ heightList << "100" << "50" << "125" << "10" << "40" << "70" << "200" << "110" << "160" << "20" << "20" << "65" << "80" << "200" << "300" << "130" << "400" << "300" << "500" << "10";
1161+ model->setStringList(heightList);
1162+
1163+ view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/verticaljournaltest.qml"));
1164+
1165+ view->show();
1166+ view->resize(470, 400);
1167+ QTest::qWaitForWindowExposed(view);
1168+
1169+ vj = dynamic_cast<VerticalJournal*>(view->rootObject()->findChild<QObject*>("vj"));
1170+ vj->setModel(model);
1171+
1172+ checkInitialPositions();
1173+ }
1174+
1175+ void cleanup()
1176+ {
1177+ delete view;
1178+ delete model;
1179+ }
1180+
1181+ void testWidthResize()
1182+ {
1183+ view->resize(629, 400);
1184+ QTRY_COMPARE(vj->width(), 629.);
1185+
1186+ // This is exactly the same block as above as nothing changed, just white space on the right
1187+ checkInitialPositions();
1188+
1189+ view->resize(630, 400);
1190+
1191+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 4);
1192+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 4);
1193+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 7);
1194+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 4);
1195+ QTRY_COMPARE(vj->m_columnVisibleItems[3].count(), 5);
1196+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
1197+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
1198+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
1199+ verifyItem(vj->m_columnVisibleItems[3][0], 3, 480, 0, true);
1200+ verifyItem(vj->m_columnVisibleItems[3][1], 4, 480, 20, true);
1201+ verifyItem(vj->m_columnVisibleItems[1][1], 5, 160, 60, true);
1202+ verifyItem(vj->m_columnVisibleItems[3][2], 6, 480, 70, true);
1203+ verifyItem(vj->m_columnVisibleItems[0][1], 7, 0, 110, true);
1204+ verifyItem(vj->m_columnVisibleItems[2][1], 8, 320, 135, true);
1205+ verifyItem(vj->m_columnVisibleItems[1][2], 9, 160, 140, true);
1206+ verifyItem(vj->m_columnVisibleItems[1][3], 10, 160, 170, true);
1207+ verifyItem(vj->m_columnVisibleItems[1][4], 11, 160, 200, true);
1208+ verifyItem(vj->m_columnVisibleItems[0][2], 12, 0, 230, true);
1209+ verifyItem(vj->m_columnVisibleItems[1][5], 13, 160, 275, true);
1210+ verifyItem(vj->m_columnVisibleItems[3][3], 14, 480, 280, true);
1211+ verifyItem(vj->m_columnVisibleItems[2][2], 15, 320, 305, true);
1212+ verifyItem(vj->m_columnVisibleItems[0][3], 16, 0, 320, true);
1213+ verifyItem(vj->m_columnVisibleItems[2][3], 17, 320, 445, false);
1214+ verifyItem(vj->m_columnVisibleItems[1][6], 18, 160, 485, false);
1215+ verifyItem(vj->m_columnVisibleItems[3][4], 19, 480, 590, false);
1216+ QTRY_COMPARE(vj->implicitHeight(), 985.);
1217+
1218+ view->resize(470, 400);
1219+
1220+ // This is exactly the same block as the first again
1221+ checkInitialPositions();
1222+ }
1223+
1224+ void testColumnSpacing()
1225+ {
1226+ vj->setColumnSpacing(11);
1227+
1228+ QTRY_COMPARE(vj->m_needsRelayout, false);
1229+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 2);
1230+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 7);
1231+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 7);
1232+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
1233+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 161, 0, true);
1234+ verifyItem(vj->m_columnVisibleItems[1][1], 2, 161, 60, true);
1235+ verifyItem(vj->m_columnVisibleItems[0][1], 3, 0, 110, true);
1236+ verifyItem(vj->m_columnVisibleItems[0][2], 4, 0, 130, true);
1237+ verifyItem(vj->m_columnVisibleItems[0][3], 5, 0, 180, true);
1238+ verifyItem(vj->m_columnVisibleItems[1][2], 6, 161, 195, true);
1239+ verifyItem(vj->m_columnVisibleItems[0][4], 7, 0, 260, true);
1240+ verifyItem(vj->m_columnVisibleItems[0][5], 8, 0, 380, true);
1241+ verifyItem(vj->m_columnVisibleItems[1][3], 9, 161, 405, false);
1242+ verifyItem(vj->m_columnVisibleItems[1][4], 10, 161, 435, false);
1243+ verifyItem(vj->m_columnVisibleItems[1][5], 11, 161, 465, false);
1244+ verifyItem(vj->m_columnVisibleItems[1][6], 12, 161, 540, false);
1245+ verifyItem(vj->m_columnVisibleItems[0][6], 13, 0, 550, false);
1246+ QCOMPARE(vj->implicitHeight(), 750. + 6. * 750. / 14.);
1247+
1248+ vj->setColumnSpacing(10);
1249+ QTRY_COMPARE(vj->m_needsRelayout, false);
1250+
1251+ // This is exactly the same block as the first again
1252+ checkInitialPositions();
1253+ }
1254+
1255+ void testRowSpacing()
1256+ {
1257+ vj->setRowSpacing(11);
1258+
1259+ QTRY_COMPARE(vj->m_needsRelayout, false);
1260+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
1261+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 5);
1262+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 7);
1263+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 6);
1264+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
1265+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
1266+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
1267+ verifyItem(vj->m_columnVisibleItems[1][1], 3, 160, 61, true);
1268+ verifyItem(vj->m_columnVisibleItems[1][2], 4, 160, 82, true);
1269+ verifyItem(vj->m_columnVisibleItems[0][1], 5, 0, 111, true);
1270+ verifyItem(vj->m_columnVisibleItems[1][3], 6, 160, 133, true);
1271+ verifyItem(vj->m_columnVisibleItems[2][1], 7, 320, 136, true);
1272+ verifyItem(vj->m_columnVisibleItems[0][2], 8, 0, 192, true);
1273+ verifyItem(vj->m_columnVisibleItems[2][2], 9, 320, 257, true);
1274+ verifyItem(vj->m_columnVisibleItems[2][3], 10, 320, 288, true);
1275+ verifyItem(vj->m_columnVisibleItems[2][4], 11, 320, 319, true);
1276+ verifyItem(vj->m_columnVisibleItems[1][4], 12, 160, 344, true);
1277+ verifyItem(vj->m_columnVisibleItems[0][3], 13, 0, 363, true);
1278+ verifyItem(vj->m_columnVisibleItems[2][5], 14, 320, 395, true);
1279+ verifyItem(vj->m_columnVisibleItems[1][5], 15, 160, 435, false);
1280+ verifyItem(vj->m_columnVisibleItems[0][4], 16, 0, 574, false);
1281+ verifyItem(vj->m_columnVisibleItems[1][6], 17, 160, 576, false);
1282+ QCOMPARE(vj->implicitHeight(), 974. + 2. * 974. / 18.);
1283+
1284+ vj->setRowSpacing(10);
1285+ QTRY_COMPARE(vj->m_needsRelayout, false);
1286+
1287+ // This is exactly the same block as the first again
1288+ checkInitialPositions();
1289+ }
1290+
1291+ void testDelegateCreationRanges()
1292+ {
1293+ vj->setDelegateCreationBegin(200);
1294+ vj->setDelegateCreationEnd(view->height());
1295+
1296+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
1297+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 4);
1298+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 4);
1299+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 6);
1300+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, false);
1301+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, false);
1302+ verifyItem(vj->m_columnVisibleItems[1][0], 4, 160, 80, false);
1303+ verifyItem(vj->m_columnVisibleItems[0][1], 5, 0, 110, false);
1304+ verifyItem(vj->m_columnVisibleItems[1][1], 6, 160, 130, true);
1305+ verifyItem(vj->m_columnVisibleItems[2][1], 7, 320, 135, true);
1306+ verifyItem(vj->m_columnVisibleItems[0][2], 8, 0, 190, true);
1307+ verifyItem(vj->m_columnVisibleItems[2][2], 9, 320, 255, true);
1308+ verifyItem(vj->m_columnVisibleItems[2][3], 10, 320, 285, true);
1309+ verifyItem(vj->m_columnVisibleItems[2][4], 11, 320, 315, true);
1310+ verifyItem(vj->m_columnVisibleItems[1][2], 12, 160, 340, true);
1311+ verifyItem(vj->m_columnVisibleItems[0][3], 13, 0, 360, true);
1312+ verifyItem(vj->m_columnVisibleItems[2][5], 14, 320, 390, true);
1313+ verifyItem(vj->m_columnVisibleItems[1][3], 15, 160, 430, false);
1314+ QCOMPARE(vj->implicitHeight(), 690. + 4. * 690. / 16.);
1315+
1316+ vj->resetDelegateCreationBegin();
1317+ vj->resetDelegateCreationEnd();
1318+
1319+ // This is exactly the same block as the first again
1320+ checkInitialPositions();
1321+ }
1322+
1323+
1324+ void testColumnWidthChange()
1325+ {
1326+ vj->setColumnWidth(200);
1327+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 2);
1328+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 7);
1329+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 7);
1330+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
1331+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 210, 0, true);
1332+ verifyItem(vj->m_columnVisibleItems[1][1], 2, 210, 60, true);
1333+ verifyItem(vj->m_columnVisibleItems[0][1], 3, 0, 110, true);
1334+ verifyItem(vj->m_columnVisibleItems[0][2], 4, 0, 130, true);
1335+ verifyItem(vj->m_columnVisibleItems[0][3], 5, 0, 180, true);
1336+ verifyItem(vj->m_columnVisibleItems[1][2], 6, 210, 195, true);
1337+ verifyItem(vj->m_columnVisibleItems[0][4], 7, 0, 260, true);
1338+ verifyItem(vj->m_columnVisibleItems[0][5], 8, 0, 380, true);
1339+ verifyItem(vj->m_columnVisibleItems[1][3], 9, 210, 405, false);
1340+ verifyItem(vj->m_columnVisibleItems[1][4], 10, 210, 435, false);
1341+ verifyItem(vj->m_columnVisibleItems[1][5], 11, 210, 465, false);
1342+ verifyItem(vj->m_columnVisibleItems[1][6], 12, 210, 540, false);
1343+ verifyItem(vj->m_columnVisibleItems[0][6], 13, 0, 550, false);
1344+ QCOMPARE(vj->implicitHeight(), 750. + 6. * 750. / 14.);
1345+
1346+ vj->setColumnWidth(150);
1347+
1348+ // This is exactly the same block as the first again
1349+ checkInitialPositions();
1350+ }
1351+
1352+ void testChangeModel()
1353+ {
1354+ QHeightModel model2;
1355+ QStringList list2;
1356+ list2 << "100" << "50" << "25" << "25" << "50" << "50";
1357+ model2.setStringList(list2);
1358+ vj->setModel(&model2);
1359+ heightList = list2;
1360+
1361+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
1362+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 1);
1363+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 2);
1364+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 3);
1365+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
1366+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
1367+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
1368+ verifyItem(vj->m_columnVisibleItems[2][1], 3, 320, 35, true);
1369+ verifyItem(vj->m_columnVisibleItems[1][1], 4, 160, 60, true);
1370+ verifyItem(vj->m_columnVisibleItems[2][2], 5, 320, 70, true);
1371+ }
1372+
1373+private:
1374+ QQuickView *view;
1375+ VerticalJournal *vj;
1376+ QStringList heightList;
1377+ QHeightModel *model;
1378+};
1379+
1380+QTEST_MAIN(VerticalJournalTest)
1381+
1382+#include "verticaljournaltest.moc"
1383
1384=== added file 'tests/plugins/DashViews/verticaljournaltest.qml'
1385--- tests/plugins/DashViews/verticaljournaltest.qml 1970-01-01 00:00:00 +0000
1386+++ tests/plugins/DashViews/verticaljournaltest.qml 2013-12-18 09:51:17 +0000
1387@@ -0,0 +1,44 @@
1388+/*
1389+ * Copyright (C) 2013 Canonical, Ltd.
1390+ *
1391+ * This program is free software; you can redistribute it and/or modify
1392+ * it under the terms of the GNU General Public License as published by
1393+ * the Free Software Foundation; version 3.
1394+ *
1395+ * This program is distributed in the hope that it will be useful,
1396+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
1397+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1398+ * GNU General Public License for more details.
1399+ *
1400+ * You should have received a copy of the GNU General Public License
1401+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
1402+ */
1403+
1404+import QtQuick 2.1
1405+import DashViews 0.1
1406+
1407+Item {
1408+
1409+ VerticalJournal {
1410+ id: vj
1411+ objectName: "vj"
1412+ anchors.fill: parent
1413+ columnWidth: 150
1414+ columnSpacing: 10
1415+ rowSpacing: 10
1416+
1417+ delegate: Rectangle {
1418+ property real randomValue: Math.random()
1419+ width: 150
1420+ color: randomValue < 0.3 ? "green" : randomValue < 0.6 ? "blue" : "red";
1421+ height: modelHeight
1422+ border.width: 3
1423+
1424+ Text {
1425+ text: index + "\ny: " + parent.y + "\nheight: " + parent.height
1426+ x: 10
1427+ y: 10
1428+ }
1429+ }
1430+ }
1431+}
1432
1433=== added file 'tests/plugins/DashViews/verticaljournaltry.cpp'
1434--- tests/plugins/DashViews/verticaljournaltry.cpp 1970-01-01 00:00:00 +0000
1435+++ tests/plugins/DashViews/verticaljournaltry.cpp 2013-12-18 09:51:17 +0000
1436@@ -0,0 +1,90 @@
1437+/*
1438+ * Copyright (C) 2013 Canonical, Ltd.
1439+ *
1440+ * This program is free software; you can redistribute it and/or modify
1441+ * it under the terms of the GNU General Public License as published by
1442+ * the Free Software Foundation; version 3.
1443+ *
1444+ * This program is distributed in the hope that it will be useful,
1445+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
1446+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1447+ * GNU General Public License for more details.
1448+ *
1449+ * You should have received a copy of the GNU General Public License
1450+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
1451+ */
1452+
1453+#include <QQuickView>
1454+#include <QDebug>
1455+#include <QGuiApplication>
1456+#include <QQuickView>
1457+#include <QtQml/qqml.h>
1458+#include <QStringListModel>
1459+#include <QQmlContext>
1460+#include <QQmlEngine>
1461+#include <QGuiApplication>
1462+
1463+#include "verticaljournal.h"
1464+
1465+class QHeightModel : public QStringListModel {
1466+ Q_OBJECT
1467+public:
1468+ QHash<int, QByteArray> roleNames() const
1469+ {
1470+ QHash<int, QByteArray> roles;
1471+ roles.insert(Qt::DisplayRole, "modelHeight");
1472+ return roles;
1473+ }
1474+
1475+public slots:
1476+ void add(int height)
1477+ {
1478+ if (height <= 0) {
1479+ height = 50 + 50. * qrand() / RAND_MAX ;
1480+ }
1481+ QStringList sl = stringList();
1482+ sl << QString::number(height);
1483+ setStringList(sl);
1484+ }
1485+
1486+ void remove()
1487+ {
1488+ QStringList sl = stringList();
1489+ if (!sl.isEmpty()) {
1490+ sl.removeLast();
1491+ setStringList(sl);
1492+ }
1493+ }
1494+};
1495+
1496+int main(int argc, char *argv[])
1497+{
1498+ QGuiApplication a(argc, argv);
1499+
1500+ QQuickView *view = new QQuickView();
1501+ view->setResizeMode(QQuickView::SizeRootObjectToView);
1502+ view->engine()->addImportPath(BUILT_PLUGINS_DIR);
1503+
1504+ QHeightModel model;
1505+ QStringList heightList;
1506+ heightList << "100" << "50" << "125";
1507+ model.setStringList(heightList);
1508+
1509+ QStringListModel listModel;
1510+ listModel.setStringList(QStringList() << QString());
1511+
1512+ view->rootContext()->setContextProperty("listModel", &listModel);
1513+ view->rootContext()->setContextProperty("vjModel", &model);
1514+
1515+ view->setSource(QUrl::fromLocalFile(DASHVIEWSTEST_FOLDER "/verticaljournaltry.qml"));
1516+
1517+ view->show();
1518+ view->resize(530, 400);
1519+
1520+ QObject::connect(view->rootObject(), SIGNAL(add(int)), &model, SLOT(add(int)));
1521+ QObject::connect(view->rootObject(), SIGNAL(remove()), &model, SLOT(remove()));
1522+
1523+ return a.exec();
1524+}
1525+
1526+#include "verticaljournaltry.moc"
1527
1528=== added file 'tests/plugins/DashViews/verticaljournaltry.qml'
1529--- tests/plugins/DashViews/verticaljournaltry.qml 1970-01-01 00:00:00 +0000
1530+++ tests/plugins/DashViews/verticaljournaltry.qml 2013-12-18 09:51:17 +0000
1531@@ -0,0 +1,100 @@
1532+/*
1533+ * Copyright (C) 2013 Canonical, Ltd.
1534+ *
1535+ * This program is free software; you can redistribute it and/or modify
1536+ * it under the terms of the GNU General Public License as published by
1537+ * the Free Software Foundation; version 3.
1538+ *
1539+ * This program is distributed in the hope that it will be useful,
1540+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
1541+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1542+ * GNU General Public License for more details.
1543+ *
1544+ * You should have received a copy of the GNU General Public License
1545+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
1546+ */
1547+
1548+import QtQuick 2.1
1549+import DashViews 0.1
1550+
1551+Item {
1552+ id: root
1553+ signal add(int height)
1554+ signal remove()
1555+
1556+ ListViewWithPageHeader {
1557+ id: lvwph
1558+ model: listModel
1559+ height: parent.height
1560+ width: parent.width - controls.width
1561+
1562+ delegate: VerticalJournal {
1563+ id: vj
1564+ model: vjModel
1565+ columnWidth: 100
1566+ columnSpacing: 10
1567+ rowSpacing: 10
1568+ width: parent.width
1569+ height: implicitHeight > 100 ? implicitHeight : 100
1570+ delegateCreationBegin: lvwph.contentY
1571+ delegateCreationEnd: lvwph.contentY + lvwph.height
1572+
1573+ delegate: Rectangle {
1574+ width: 100
1575+ color: "red";
1576+ height: modelHeight
1577+ border.width: 3
1578+
1579+ Text {
1580+ text: index + "\ny: " + parent.y + "\nheight: " + parent.height
1581+ x: 10
1582+ y: 10
1583+ }
1584+ }
1585+ }
1586+ }
1587+
1588+ Rectangle {
1589+ id: controls
1590+ height: parent.height
1591+ width: 100
1592+ anchors.right: parent.right
1593+ color: "gray"
1594+ opacity: 0.4
1595+
1596+ Rectangle {
1597+ id: addButton
1598+ height: 50
1599+ width: parent.width
1600+ color: "blue"
1601+ Text {
1602+ text: "Add"
1603+ anchors.centerIn: parent
1604+ }
1605+ MouseArea {
1606+ anchors.fill: parent
1607+ onClicked: root.add(addField.text)
1608+ }
1609+ }
1610+ TextInput {
1611+ id: addField
1612+ anchors.top: addButton.bottom
1613+ height: 30
1614+ width: parent.width
1615+ }
1616+ Rectangle {
1617+ anchors.top: addField.bottom
1618+ height: 50
1619+ width: parent.width
1620+ color: "red"
1621+ Text {
1622+ text: "Remove"
1623+ anchors.centerIn: parent
1624+ }
1625+ MouseArea {
1626+ anchors.fill: parent
1627+ onClicked: root.remove()
1628+ }
1629+ }
1630+ }
1631+}
1632\ No newline at end of file

Subscribers

People subscribed via source and target branches