Ubuntu UI Toolkit

Merge lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot into lp://staging/ubuntu-ui-toolkit/staging

visualRoot
Merge into staging

Proposed by Cris Dywan on 2017-01-13

Status:

Merged

Approved by:

Tim Peeters on 2017-01-18

Approved revision:

2070

Merged at revision:

2159

Proposed branch:

lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot

Merge into:

lp://staging/ubuntu-ui-toolkit/staging

Prerequisite:

lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/outTheWindow

Diff against target:

218 lines (+99/-2)

7 files modified

src/UbuntuToolkit/quickutils.cpp (+9/-0)
src/UbuntuToolkit/ucmainwindow.cpp (+26/-0)
src/UbuntuToolkit/ucmainwindow_p.h (+7/-0)
src/UbuntuToolkit/ucmainwindow_p_p.h (+1/-0)
src/imports/Components/Popups/1.3/popupUtils.js (+6/-1)
tests/unit/mainwindow/VisualRoot.qml (+36/-0)
tests/unit/mainwindow/tst_mainwindow.cpp (+14/-1)

To merge this branch:

bzr merge lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot

Related bugs:

Bug #1504551: Text input popover and selection cursors not oriented properly in shell	Medium	Fix Released
Bug #1656857: Provide a way of specifying a parent for Dialogs/Popups different than the root object when using PopupUtils.open	Undecided	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
ubuntu-sdk-build-bot	continuous-integration		Approve on 2017-01-18
Tim Peeters			Approve on 2017-01-18
Zsombor Egri		2017-01-13	Needs Information on 2017-01-18
Review via email: mp+314684@code.staging.launchpad.net

Commit message

Add visualRoot property to MainWindow

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2066
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1608/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7421/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1608/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2066
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1358/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7422/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1358/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2066
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/172/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7423/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/172/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2066
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1393/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7424/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1393/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/864/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Tim Peeters (tpeeters) wrote on 2017-01-16:

Let's add documentation in PopupUtils and the popup components on how to change the root item. Also an example/use case would be useful.

lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot updated on 2017-01-16

2067. By Cris Dywan on 2017-01-16: Merge lp:ubuntu-ui-toolkit/staging
2068. By Cris Dywan on 2017-01-16: Document the role of visualRoot in QuickUtils and popupUtils

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2068
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1359/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7426/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1359/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2068
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/173/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7427/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/173/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2068
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1609/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7428/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1609/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

FAILED: Continuous integration, rev:2068
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1394/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7429/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1394/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/865/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot updated on 2017-01-16

2069. By Cris Dywan on 2017-01-16: Tweak quickutils header include in tst_mainwindow

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

PASSED: Continuous integration, rev:2069
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1360/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7431/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1360/rebuild

review: Approve (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

PASSED: Continuous integration, rev:2069
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1395/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7432/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/1395/rebuild

review: Approve (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

PASSED: Continuous integration, rev:2069
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/174/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7433/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-yakkety/174/rebuild

review: Approve (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

PASSED: Continuous integration, rev:2069
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1610/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7434/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/1610/rebuild

review: Approve (continuous-integration)

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-16:

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/866/rebuild

review: Approve (continuous-integration)

Revision history for this message

Tim Peeters (tpeeters) wrote on 2017-01-18:

some small suggestions for the docs in the inline comments below.

lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot updated on 2017-01-18

2070. By Cris Dywan on 2017-01-18: Reword documentation of visualRoot for clarity

Revision history for this message

Cris Dywan (kalikiana) wrote on 2017-01-18:

> > The property holds the window's visual root,
> > as opposed to the root item.
> Should we prepend this sentence with 'If set,'?
> To make it clear that visualRoot is ignored
> when it is null? Or say 'set to null in order
> to use the root item....'.

I moved the If to the start of the sentence.

> > If set, popups (popovers, dialogs, menus) will
> > reparent to it when opened via
> They won't be reparented, they will be created
> as children of visualRoot (as you say in the
> docs of popupUtils).

Yes and no. popupUtils.open creates them initially with the parent. show() on the other hand does reparent after the fact if reparentToRootItem is true.

I made this more explicit now.

> > - QQuickItem *testItem(QQuickItem *that, const QString &identifier)
> > + QQuickItem *testItem(QObject *that, const QString &identifier)
> why does this have to be changed?

A QQuickWindow is not a QQuickItem. Properties and children are however implemented in QObject and aren't specific to items at all. So we can use them with a window just fine, so long as we don't try to cast it to something unrelated that it isn't.

Revision history for this message

Zsombor Egri (zsombi) wrote on 2017-01-18:

A small comment about the property, perhaps we would want to use the QQC2 ApplicationWindow's contentItem as property name?

http://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html#contentItem-prop

review: Needs Information

Revision history for this message

Cris Dywan (kalikiana) wrote on 2017-01-18:

> A small comment about the property, perhaps we would want to use the QQC2
> ApplicationWindow's contentItem as property name?
>
> http://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html
> #contentItem-prop

QQuickWindow already has a contentItem property and it's not the same as what we want for the visualItem to be.

Revision history for this message

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote on 2017-01-18:

PASSED: Continuous integration, rev:2070
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/1363/
Executed test runs:
None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7446/console