Merge lp:~tpeeters/ubuntu-ui-toolkit/stack-tabs into lp:ubuntu-ui-toolkit

Proposed by Tim Peeters
Status: Merged
Approved by: Cris Dywan
Approved revision: 652
Merged at revision: 649
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/stack-tabs
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 223 lines (+141/-10)
5 files modified
modules/Ubuntu/Components/Tabs.qml (+54/-1)
tests/resources/pagestack/MyCustomPage.qml (+3/-5)
tests/resources/pagestack/PageStack.qml (+2/-2)
tests/resources/pagestack/StackWithTabs.qml (+61/-0)
tests/unit/tst_components/tst_pagestack.qml (+21/-2)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/stack-tabs
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Cris Dywan Approve
Review via email: mp+176152@code.launchpad.net

Commit message

Allow combination of Tabs and PageStack:
- Fix header behavior for Tabs inside PageStack
- Add test for Tabs inside PageStack header bug
- Replace deprecated components in PageStack tests
- Add test program that combines PageStack and Tabs
- Update Tabs documentation

Description of the change

Allow combination of Tabs and PageStack:
- Fix header behavior for Tabs inside PageStack
- Add test for Tabs inside PageStack header bug
- Replace deprecated components in PageStack tests
- Add test program that combines PageStack and Tabs
- Update Tabs documentation

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Cris Dywan (kalikiana) wrote :

"Please note that when a \l PageStack exists inside Tabs, the Tabs will remain active and thus control the header, even when a new \l Page is pushed to the internal \l PageStack."

I'm wondering if that section could be a little more actionable, say "You need to push()/pop() explicitly to switch to the page in that case".

Looks good in any case. I like that the real fix is so simple.

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

Lovely! Thanks a lot for the doc tweaks!

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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
Cris Dywan (kalikiana) wrote :

Seems this is blocking on lp:~juhapekka-piiroinen/ubuntu-ui-toolkit/fix-pep8-issues getting merged first.

652. By Tim Peeters

merge trunk

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'modules/Ubuntu/Components/Tabs.qml'
2--- modules/Ubuntu/Components/Tabs.qml 2013-07-10 15:59:20 +0000
3+++ modules/Ubuntu/Components/Tabs.qml 2013-07-25 08:32:24 +0000
4@@ -91,6 +91,59 @@
5 It is possible to use a Repeater to generate tabs, but when doing so, ensure that the Repeater
6 is declared inside the Tabs at the end, because otherwise the shuffling of
7 the order of children by the Repeater can cause incorrect ordering of the tabs.
8+
9+ The \l {http://design.ubuntu.com/apps/global-patterns/navigation}{Navigation Patterns} specify that
10+ a tabs header should never be combined with the back button of a \l PageStack. The only way to
11+ combine Tabs and \l PageStack that avoids this is by pushing the Tabs as the first page on the
12+ \l PageStack, and pushing other pages on top of that, as is shown in the following example:
13+
14+ \qml
15+ import QtQuick 2.0
16+ import Ubuntu.Components 0.1
17+
18+ MainView {
19+ id: mainView
20+ width: units.gu(38)
21+ height: units.gu(50)
22+
23+ PageStack {
24+ id: pageStack
25+ Component.onCompleted: push(tabs)
26+
27+ Tabs {
28+ id: tabs
29+ Tab {
30+ title: "Tab 1"
31+ page: Page {
32+ Button {
33+ anchors.centerIn: parent
34+ onClicked: pageStack.push(page3)
35+ text: "Press"
36+ }
37+ }
38+ }
39+ Tab {
40+ title: "Tab 2"
41+ page: Page {
42+ Label {
43+ anchors.centerIn: parent
44+ text: "Use header to navigate between tabs"
45+ }
46+ }
47+ }
48+ }
49+ Page {
50+ id: page3
51+ visible: false
52+ title: "Page on stack"
53+ Label {
54+ anchors.centerIn: parent
55+ text: "Press back to return to the tabs"
56+ }
57+ }
58+ }
59+ }
60+ \endqml
61 */
62
63 PageTreeNode {
64@@ -202,7 +255,7 @@
65 target: internal.header
66 property: "contents"
67 value: tabs.active ? tabs.__headerContents : null
68- when: internal.header
69+ when: internal.header && tabs.active
70 }
71
72 style: Theme.createStyleComponent("TabsStyle.qml", tabs)
73
74=== modified file 'tests/resources/pagestack/MyCustomPage.qml'
75--- tests/resources/pagestack/MyCustomPage.qml 2013-05-01 22:26:31 +0000
76+++ tests/resources/pagestack/MyCustomPage.qml 2013-07-25 08:32:24 +0000
77@@ -35,14 +35,12 @@
78 }
79 }
80
81- tools: ToolbarActions {
82- Action {
83+ tools: ToolbarItems {
84+ ToolbarButton {
85 text: "action 1"
86- iconSource: Qt.resolvedUrl("avatar_contacts_list.png")
87 }
88- Action {
89+ ToolbarButton {
90 text: "action 2"
91- iconSource: Qt.resolvedUrl("call_icon.png")
92 }
93 opened: true
94 locked: true
95
96=== modified file 'tests/resources/pagestack/PageStack.qml'
97--- tests/resources/pagestack/PageStack.qml 2013-05-02 19:18:09 +0000
98+++ tests/resources/pagestack/PageStack.qml 2013-07-25 08:32:24 +0000
99@@ -72,8 +72,8 @@
100 }
101 }
102
103- tools: ToolbarActions {
104- Action {
105+ tools: ToolbarItems {
106+ ToolbarButton {
107 text: "oh"
108 }
109 }
110
111=== added file 'tests/resources/pagestack/StackWithTabs.qml'
112--- tests/resources/pagestack/StackWithTabs.qml 1970-01-01 00:00:00 +0000
113+++ tests/resources/pagestack/StackWithTabs.qml 2013-07-25 08:32:24 +0000
114@@ -0,0 +1,61 @@
115+/*
116+ * Copyright 2012 Canonical Ltd.
117+ *
118+ * This program is free software; you can redistribute it and/or modify
119+ * it under the terms of the GNU Lesser General Public License as published by
120+ * the Free Software Foundation; version 3.
121+ *
122+ * This program is distributed in the hope that it will be useful,
123+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
124+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
125+ * GNU Lesser General Public License for more details.
126+ *
127+ * You should have received a copy of the GNU Lesser General Public License
128+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
129+ */
130+
131+import QtQuick 2.0
132+import Ubuntu.Components 0.1
133+
134+MainView {
135+ id: mainView
136+ width: units.gu(38)
137+ height: units.gu(50)
138+
139+ PageStack {
140+ id: pageStack
141+ Component.onCompleted: push(tabs)
142+
143+ Tabs {
144+ id: tabs
145+ Tab {
146+ title: "Tab 1"
147+ page: Page {
148+ Button {
149+ anchors.centerIn: parent
150+ onClicked: pageStack.push(page3)
151+ text: "Press"
152+ }
153+ }
154+ }
155+ Tab {
156+ title: "Tab 2"
157+ page: Page {
158+ Label {
159+ anchors.centerIn: parent
160+ text: "Use header to navigate between tabs"
161+ }
162+ }
163+ }
164+ }
165+ Page {
166+ id: page3
167+ visible: false
168+ title: "Page on stack"
169+ Label {
170+ anchors.centerIn: parent
171+ text: "Press back to return to the tabs"
172+ }
173+ }
174+ }
175+}
176
177=== modified file 'tests/unit/tst_components/tst_pagestack.qml'
178--- tests/unit/tst_components/tst_pagestack.qml 2013-05-22 09:38:09 +0000
179+++ tests/unit/tst_components/tst_pagestack.qml 2013-07-25 08:32:24 +0000
180@@ -81,6 +81,21 @@
181 pageStack.clear();
182 }
183
184+ function test_tabs_inside_stack_bug1187850() {
185+ pageStack.push(tabs);
186+ compare(pageStack.currentPage, tabs, "Tabs can be pushed on a PageStack");
187+ compare(tabs.active, true, "Tabs on top of a PageStack are active");
188+ compare(mainView.__propagated.header.contents, tabs.__headerContents, "Pushing Tabs on PageStack updates the header contents");
189+ pageStack.push(page1);
190+ compare(pageStack.currentPage, page1, "A page can be pushed on top of a Tabs");
191+ compare(tabs.active, false, "Tabs on a PageStack, but not on top, are inactive");
192+ compare(mainView.__propagated.header.contents, null, "Contents of inactive Tabs is not applied to header");
193+ pageStack.pop();
194+ compare(tabs.active, true, "Tabs on top of PageStack is active");
195+ compare(mainView.__propagated.header.contents, tabs.__headerContents, "Active Tabs controls header contents");
196+ pageStack.clear();
197+ }
198+
199 MainView {
200 id: mainView
201 PageStack {
202@@ -90,15 +105,19 @@
203 Page {
204 id: page1
205 title: "Title 1"
206- tools: ToolbarActions {
207+ tools: ToolbarItems {
208 id: tools1
209 }
210 }
211 Page {
212 id: page2
213 title: "Title 2"
214- tools: ToolbarActions {
215+ tools: ToolbarItems {
216 id: tools2
217 }
218 }
219+
220+ Tabs {
221+ id: tabs
222+ }
223 }

Subscribers

People subscribed via source and target branches

to status/vote changes: