Merge lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot into lp:ubuntu-ui-toolkit/staging
| Status: | Merged |
|---|---|
| Approved by: | Tim Peeters on 2017-01-18 |
| Approved revision: | 2070 |
| Merged at revision: | 2159 |
| Proposed branch: | lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot |
| Merge into: | lp:ubuntu-ui-toolkit/staging |
| Prerequisite: | lp:~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:~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot |
| Related bugs: |
| 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 (community) | 2017-01-13 | Needs Information on 2017-01-18 | |
|
Review via email:
|
|||
Commit Message
Add visualRoot property to MainWindow
FAILED: Continuous integration, rev:2066
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2066
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2066
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2066
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
| Tim Peeters (tpeeters) wrote : | # |
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.
- 2067. By Christian Dywan on 2017-01-16
- 2068. By Christian Dywan on 2017-01-16
-
Document the role of visualRoot in QuickUtils and popupUtils
FAILED: Continuous integration, rev:2068
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2068
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2068
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2068
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2068
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- 2069. By Christian Dywan on 2017-01-16
-
Tweak quickutils header include in tst_mainwindow
PASSED: Continuous integration, rev:2069
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2069
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2069
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2069
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2069
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
| Tim Peeters (tpeeters) wrote : | # |
some small suggestions for the docs in the inline comments below.
- 2070. By Christian Dywan on 2017-01-18
-
Reword documentation of visualRoot for clarity
| Christian Dywan (kalikiana) wrote : | # |
> > 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 *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.
| Zsombor Egri (zsombi) wrote : | # |
A small comment about the property, perhaps we would want to use the QQC2 ApplicationWindow's contentItem as property name?
http://
| Christian Dywan (kalikiana) wrote : | # |
> A small comment about the property, perhaps we would want to use the QQC2
> ApplicationWindow's contentItem as property name?
>
> http://
> #contentItem-prop
QQuickWindow already has a contentItem property and it's not the same as what we want for the visualItem to be.
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:2070
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:2066
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/