Merge lp:~zsombi/ubuntu-ui-toolkit/multicolumnview into lp:ubuntu-ui-toolkit/staging
| Status: | Merged |
|---|---|
| Merge reported by: | Christian Dywan |
| Merged at revision: | not available |
| Proposed branch: | lp:~zsombi/ubuntu-ui-toolkit/multicolumnview |
| Merge into: | lp:ubuntu-ui-toolkit/staging |
| Diff against target: |
1172 lines (+1033/-4) 14 files modified
components.api (+15/-0) modules/Ubuntu/Components/1.2/stack.js (+1/-0) modules/Ubuntu/Components/1.3/ColumnMetrics.qml (+52/-0) modules/Ubuntu/Components/1.3/MultiColumnView.qml (+569/-0) modules/Ubuntu/Components/1.3/Page.qml (+1/-0) modules/Ubuntu/Components/1.3/PageHeadConfiguration.qml (+2/-0) modules/Ubuntu/Components/1.3/PageWrapper.qml (+37/-0) modules/Ubuntu/Components/1.3/stack.js (+75/-0) modules/Ubuntu/Components/Themes/Ambiance/1.3/PageHeadStyle.qml (+2/-2) modules/Ubuntu/Components/qmldir (+2/-0) tests/resources/navigation/MyCustomPage.qml (+1/-1) tests/resources/navigation/SplitViewStack.qml (+137/-0) tests/unit_x11/tst_components/tst_components.pro (+2/-1) tests/unit_x11/tst_components/tst_multicolumnview.qml (+137/-0) |
| To merge this branch: | bzr merge lp:~zsombi/ubuntu-ui-toolkit/multicolumnview |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Tim Peeters | 2015-06-11 | Needs Fixing on 2015-06-18 | |
| PS Jenkins bot | continuous-integration | Approve on 2015-06-17 | |
|
Review via email:
|
|||
Commit Message
MultiColumnView component
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1538
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1540
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Tim Peeters (tpeeters) wrote : | # |
23 + default readonly property QtObject data
why?
| Tim Peeters (tpeeters) wrote : | # |
41 === modified file 'modules/
42 --- modules/
43 +++ modules/
44 @@ -41,4 +41,5 @@
45 if (this.size() < 1) return undefined;
46 return elements[
47 }
48 +
49 }
undo
| Tim Peeters (tpeeters) wrote : | # |
82 + /*!
83 + 1-based value identifying the column the metrics to be applied to.
84 + */
85 + property int column
Why not start at 0, as is common?
| Tim Peeters (tpeeters) wrote : | # |
87 + /*!
88 + Specifies whether the width of the column should fill the available space
89 + of the MultiColumnView column or not. Defaults to \a false.
90 + */
91 + property bool fillWidth: false
if fillWidth = true, then defaultColumnWidth will be used (if that is within the borders of the min and max width for the colum)? Can you describe that explicitly here?
| Tim Peeters (tpeeters) wrote : | # |
144 + MultiColumnView stores pages added in a tree. Pages are added relative to a
145 + given page, either as sibling (\l addPageToCurren
146 + (\l addPageToNextCo
147 + tree will remove all its children from the page tree.
I don't agree with the terminology here. They are both children, in either of the columns.
| Tim Peeters (tpeeters) wrote : | # |
152 + the column next to the source page. Giving a null value to the source page will
153 + add the page to the leftmost column of the view.
No, adding a null value is forbidden.
| Tim Peeters (tpeeters) wrote : | # |
160 + \note Unlike PageStack, the component does not fill its parent content.
why not?
| Tim Peeters (tpeeters) wrote : | # |
I'm reading this - http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1543
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Zsombor Egri (zsombi) wrote : | # |
> 23 + default readonly property QtObject data
>
> why?
That wasn't me, it is the qmlapicheck, sorry.
| Zsombor Egri (zsombi) wrote : | # |
> 41 === modified file 'modules/
> 42 --- modules/
> +0000
> 43 +++ modules/
> +0000
> 44 @@ -41,4 +41,5 @@
> 45 if (this.size() < 1) return undefined;
> 46 return elements[
> 47 }
> 48 +
> 49 }
>
> undo
ehh... will undo soon.
| Zsombor Egri (zsombi) wrote : | # |
> 82 + /*!
> 83 + 1-based value identifying the column the metrics to be applied
> to.
> 84 + */
> 85 + property int column
>
> Why not start at 0, as is common?
Well, doesn't matter, we can start from 0 then.
| Zsombor Egri (zsombi) wrote : | # |
> 87 + /*!
> 88 + Specifies whether the width of the column should fill the
> available space
> 89 + of the MultiColumnView column or not. Defaults to \a false.
> 90 + */
> 91 + property bool fillWidth: false
>
> if fillWidth = true, then defaultColumnWidth will be used (if that is within
> the borders of the min and max width for the colum)? Can you describe that
> explicitly here?
if fillWidth == true => every value is omitted, just like in Layout.
| Zsombor Egri (zsombi) wrote : | # |
> 160 + \note Unlike PageStack, the component does not fill its parent
> content.
>
> why not?
This was something people complained a lot in PageStack. if you want different aching, you have to set anchor.fill: undefined first, then change the anchors.
| Zsombor Egri (zsombi) wrote : | # |
> I'm reading this - http://
> they have GridLayout, RowLayout and ColumnLayout. I'm wondering whether we
> should name our component MultiColumnView (similar to our MainView), or we
> adapt naming similar to that used in Layouts, so MultiColumnLayout?
Not a bad idea!!!!!
| Tim Peeters (tpeeters) wrote : | # |
293 + The proeprty can hold either a Page instance, a component holding a Page
proeprty
| Tim Peeters (tpeeters) wrote : | # |
293 + The proeprty can hold either a Page instance, a component holding a Page
294 + or a QML document defining the Page. The property cannot be changed after
295 + component completion.
296 + */
297 + property var primaryPage
Why? What is the use case for this flexibility? So we just make it property Item primaryPage or even property Page primaryPage.
| Tim Peeters (tpeeters) wrote : | # |
300 + Specifies the number of columns in the view. A condition must be set to
301 + control the number of columns depending on the space available.
It is not a requirement that a condition is set.
| Tim Peeters (tpeeters) wrote : | # |
311 + /*!
312 + The property specifies the default width of each column. The property is
313 + applied on each column. If the \a minimumWidth specified for the column is
Not on the last column. That will use full width.
314 + bigger than this value, the minimum width will be applied.
315 + */
316 + property real defaultColumnWidth: units.gu(40)
| Tim Peeters (tpeeters) wrote : | # |
323 + is empty. Only columns requiring special handling than the default should
324 + be specified.
325 + */
+different from
| Tim Peeters (tpeeters) wrote : | # |
329 + \qmlmethod Item addPageToCurren
this doesn't seem to do anything in the generated docs.
| Tim Peeters (tpeeters) wrote : | # |
330 + Adds a \c page to the column the \c sourcePage resides in. If \c sourcePage
331 + is null, the \c page will be added to the leftmost column. \c page can be a
332
Null is not allowed. Update docs.
| Tim Peeters (tpeeters) wrote : | # |
336 + function addPageToCurren
363 + function addPageToNextCo
These share a lot of implementation. Add an internal addPageToColumn(i, sourcePage, page, properties) function that they call.
| Tim Peeters (tpeeters) wrote : | # |
409 + console.warn("There must me a minimum of one column set.");
BE

FAILED: Continuous integration, rev:1536 jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- ci/1886/ jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- vivid-touch/ 3192/console jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-amd64- ci/614/ console jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-armhf- ci/616/ console jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-i386- ci/613/ console jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- vivid-armhf/ 3190/console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/ubuntu- sdk-team- ubuntu- ui-toolkit- staging- ci/1886/ rebuild
http://