Merge lp:~tpeeters/ubuntu-ui-toolkit/tabfix into lp:ubuntu-ui-toolkit

Proposed by Tim Peeters on 2013-05-01
Status: Merged
Approved by: Zsombor Egri on 2013-05-07
Approved revision: 469
Merged at revision: 480
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/tabfix
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 483 lines (+236/-83)
6 files modified
modules/Ubuntu/Components/Tab.qml (+2/-0)
modules/Ubuntu/Components/Tabs.qml (+50/-11)
tests/resources/tabs/MyCustomPage.qml (+50/-0)
tests/resources/tabs/Tabs.qml (+99/-0)
themes/Ambiance/qmltheme/NewTabBar.qml (+30/-19)
themes/Ambiance/qmltheme/NewTabsDelegate.qml (+5/-53)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/tabfix
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2013-05-07
Zsombor Egri 2013-05-01 Approve on 2013-05-07
Review via email: mp+161902@code.launchpad.net

Commit message

Remove VisualItemModel from Tabs internals, and make it work with a Repeater to define the tabs.

Description of the change

Remove VisualItemModel from Tabs internals, and make it work with a Repeater to define the tabs.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Zsombor Egri (zsombi) wrote :

You have not documented the Repeater limitation in the Tabs.

review: Needs Fixing
Zsombor Egri (zsombi) wrote :

good to go now

review: Approve
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
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/Tab.qml'
2--- modules/Ubuntu/Components/Tab.qml 2013-04-03 12:21:17 +0000
3+++ modules/Ubuntu/Components/Tab.qml 2013-05-07 11:26:29 +0000
4@@ -59,4 +59,6 @@
5 */
6 active: parentNode && parentNode.active &&
7 parentNode.hasOwnProperty("selectedTab") && parentNode.selectedTab === tab
8+
9+ visible: active
10 }
11
12=== modified file 'modules/Ubuntu/Components/Tabs.qml'
13--- modules/Ubuntu/Components/Tabs.qml 2013-04-08 13:32:34 +0000
14+++ modules/Ubuntu/Components/Tabs.qml 2013-05-07 11:26:29 +0000
15@@ -90,6 +90,10 @@
16
17 \endqml
18 As the example above shows, an external \l Page inside a \l Tab can be loaded using a Loader.
19+
20+ It is possible to use a Repeater to generate tabs, but when doing so, ensure that the Repeater
21+ is declared inside the Tabs at the end, because otherwise the shuffling of
22+ the order of children by the Repeater can cause incorrect ordering of the tabs.
23 */
24
25 PageTreeNode {
26@@ -104,13 +108,13 @@
27 The first tab is 0, and -1 means that no tab is selected.
28 The initial value is 0 if Tabs has contents, or -1 otherwise.
29 */
30- property int selectedTabIndex: tabsModel.count > 0 ? 0 : -1
31+ property int selectedTabIndex: tabs.__tabs.length > 0 ? 0 : -1
32
33 /*!
34 \preliminary
35 The currently selected tab.
36 */
37- readonly property Tab selectedTab: (selectedTabIndex < 0) || (tabsModel.count <= selectedTabIndex) ?
38+ readonly property Tab selectedTab: (selectedTabIndex < 0) || (__tabs.length <= selectedTabIndex) ?
39 null : __tabs[selectedTabIndex]
40
41 /*!
42@@ -139,21 +143,56 @@
43 "Pages will automatically update the toolbar when activated. "+
44 "See CHANGES file, and use toolbar.tools instead when needed.");
45
46-
47- // FIXME: Using the VisualItemModel as a workaround for this bug:
48- // "theming: contentItem does work when it is a VisualItemModel"
49- // https://bugs.launchpad.net/tavastia/+bug/1080330
50- // The workaround does not break the regular TabsDelegate.
51- /*! \internal */
52- default property alias __tabs: tabsModel.children
53+ /*!
54+ \internal
55+ Used by the delegate to create the tabs header.
56+ */
57+ property alias __tabs: tabsModel.tabList
58+
59+ /*!
60+ Children are placed in a separate item that has functionality to extract the Tab items.
61+ \qmlproperty list<Item> tabChildren
62+ */
63+ default property alias tabChildren: tabsModel.children
64+
65+ /*!
66+ Used by the tabs delegate to update the tabs header with the titles of all the tabs.
67+ This signal is used in an intermediate step in transitioning the tabs to a new
68+ implementation and may be removed in the future.
69+ */
70+ signal modelChanged()
71
72 /*!
73 \internal
74 required by NewTabsDelegate
75 */
76- property alias __tabsModel: tabsModel
77- VisualItemModel {
78+ Item {
79+ anchors.fill: parent
80 id: tabsModel
81+
82+ property var tabList: []
83+ onChildrenChanged: {
84+ updateTabList();
85+ }
86+
87+ function updateTabList() {
88+ var list = [];
89+ for (var i=0; i < children.length; i++) {
90+ if (isTab(tabsModel.children[i])) list.push(tabsModel.children[i]);
91+ }
92+ tabList = list;
93+ tabs.modelChanged();
94+ }
95+
96+ function isTab(item) {
97+ if (item && item.hasOwnProperty("__isPageTreeNode")
98+ && item.__isPageTreeNode && item.hasOwnProperty("title")
99+ && item.hasOwnProperty("page")) {
100+ return true;
101+ } else {
102+ return false;
103+ }
104+ }
105 }
106
107 /*! \internal */
108
109=== added directory 'tests/resources/tabs'
110=== added file 'tests/resources/tabs/MyCustomPage.qml'
111--- tests/resources/tabs/MyCustomPage.qml 1970-01-01 00:00:00 +0000
112+++ tests/resources/tabs/MyCustomPage.qml 2013-05-07 11:26:29 +0000
113@@ -0,0 +1,50 @@
114+/*
115+ * Copyright 2012 Canonical Ltd.
116+ *
117+ * This program is free software; you can redistribute it and/or modify
118+ * it under the terms of the GNU Lesser General Public License as published by
119+ * the Free Software Foundation; version 3.
120+ *
121+ * This program is distributed in the hope that it will be useful,
122+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
123+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
124+ * GNU Lesser General Public License for more details.
125+ *
126+ * You should have received a copy of the GNU Lesser General Public License
127+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
128+ */
129+
130+import QtQuick 2.0
131+import Ubuntu.Components 0.1
132+
133+Page {
134+ title: i18n.tr("My custom page")
135+
136+ Flickable {
137+ anchors.fill: parent
138+ contentHeight: parent.height + units.gu(10)
139+ Label {
140+ anchors {
141+ top: parent.top
142+ topMargin: units.gu(16)
143+ horizontalCenter: parent.horizontalCenter
144+ }
145+
146+ text: i18n.tr("This is an external page\nwith a locked toolbar.")
147+ color: "#757373"
148+ }
149+ }
150+
151+ tools: ToolbarActions {
152+ Action {
153+ text: "action 1"
154+ iconSource: Qt.resolvedUrl("call_icon.png")
155+ }
156+ Action {
157+ text: "action 2"
158+ iconSource: Qt.resolvedUrl("call_icon.png")
159+ }
160+ opened: true
161+ locked: true
162+ }
163+}
164
165=== added file 'tests/resources/tabs/Tabs.qml'
166--- tests/resources/tabs/Tabs.qml 1970-01-01 00:00:00 +0000
167+++ tests/resources/tabs/Tabs.qml 2013-05-07 11:26:29 +0000
168@@ -0,0 +1,99 @@
169+/*
170+ * Copyright 2012 Canonical Ltd.
171+ *
172+ * This program is free software; you can redistribute it and/or modify
173+ * it under the terms of the GNU Lesser General Public License as published by
174+ * the Free Software Foundation; version 3.
175+ *
176+ * This program is distributed in the hope that it will be useful,
177+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
178+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
179+ * GNU Lesser General Public License for more details.
180+ *
181+ * You should have received a copy of the GNU Lesser General Public License
182+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
183+ */
184+
185+import QtQuick 2.0
186+import Ubuntu.Components 0.1
187+import Ubuntu.Components.ListItems 0.1 as ListItem
188+
189+MainView {
190+ width: 800
191+ height: 600
192+ Tabs {
193+ id: tabs
194+ selectedTabIndex: 0
195+ Item {
196+ // does this mess up stuff? nope.
197+ }
198+
199+ Tab {
200+ title: i18n.tr("Simple page")
201+ page: Page {
202+ Label {
203+ id: label
204+ anchors.centerIn: parent
205+ text: "A centered label"
206+ }
207+ tools: ToolbarActions {
208+ Action {
209+ text: "action"
210+ iconSource: "call_icon.png"
211+ onTriggered: print("action triggered")
212+ }
213+ }
214+ }
215+ }
216+ Repeater {
217+ model: 3
218+ Tab {
219+ title: "Extra " + index
220+ page: Page {
221+ Column {
222+ anchors.centerIn: parent
223+ width: units.gu(40)
224+ Label {
225+ anchors {
226+ left: parent.left
227+ right: parent.right
228+ }
229+ text: "Extra tab number "+index
230+ }
231+ Button {
232+ anchors {
233+ left: parent.left
234+ right: parent.right
235+ }
236+ text: "Previous"
237+ onClicked: if (tabs.selectedTabIndex > 0) tabs.selectedTabIndex--
238+ }
239+ }
240+ }
241+ }
242+ }
243+ Tab {
244+ id: externalTab
245+ title: i18n.tr("External")
246+ page: Loader {
247+ parent: externalTab
248+ anchors.fill: parent
249+ source: (tabs.selectedTab === externalTab) ? Qt.resolvedUrl("MyCustomPage.qml") : ""
250+ }
251+ }
252+ Tab {
253+ title: i18n.tr("List view")
254+ page: Page {
255+ ListView {
256+ clip: true
257+ anchors.fill: parent
258+ model: 20
259+ delegate: ListItem.Standard {
260+ icon: Qt.resolvedUrl("call_icon.png")
261+ text: "Item "+modelData
262+ }
263+ }
264+ }
265+ }
266+ }
267+}
268
269=== added file 'tests/resources/tabs/call_icon@8.png'
270Binary files tests/resources/tabs/call_icon@8.png 1970-01-01 00:00:00 +0000 and tests/resources/tabs/call_icon@8.png 2013-05-07 11:26:29 +0000 differ
271=== modified file 'themes/Ambiance/qmltheme/NewTabBar.qml'
272--- themes/Ambiance/qmltheme/NewTabBar.qml 2013-05-02 00:57:01 +0000
273+++ themes/Ambiance/qmltheme/NewTabBar.qml 2013-05-07 11:26:29 +0000
274@@ -66,6 +66,7 @@
275 Connections {
276 target: tabs
277 onSelectedTabIndexChanged: buttonView.selectButton(tabs.selectedTabIndex)
278+ onModelChanged: buttonView.selectButton(tabs.selectedTabIndex)
279 }
280
281 Component {
282@@ -79,9 +80,17 @@
283 width: childrenRect.width
284 property int rowNumber: modelData
285
286+ Component.onCompleted: {
287+ if (rowNumber === 0) {
288+ buttonView.buttonRow1 = theRow;
289+ } else {
290+ buttonView.buttonRow2 = theRow;
291+ }
292+ }
293+
294 Repeater {
295 id: repeater
296- model: tabs.__tabsModel.children
297+ model: tabs.__tabs
298
299 AbstractButton {
300 id: button
301@@ -97,7 +106,6 @@
302 property bool selected: (tabBar.active && buttonView.needsScrolling) ? tabs.selectedTabIndex === index : buttonView.selectedButtonIndex === button.buttonIndex
303 property real offset: theRow.rowNumber + 1 - button.x / theRow.width;
304 property int buttonIndex: index + theRow.rowNumber*repeater.count
305- Component.onCompleted: buttonView.buttons.push(button)
306
307 // Use opacity 0 to hide instead of setting visibility to false in order to
308 // make fading work well, and not to mess up width/offset computations
309@@ -109,7 +117,7 @@
310
311 // When we don't need scrolling, we want to avoid showing a button that is fading
312 // while sliding in from the right side when a new button was selected
313- var numTabs = tabs.__tabsModel.count;
314+ var numTabs = tabs.__tabs.length;
315 var minimum = buttonView.selectedButtonIndex;
316 var maximum = buttonView.selectedButtonIndex + numTabs - 1;
317 if (MathUtils.clamp(buttonIndex, minimum, maximum) === buttonIndex) return true;
318@@ -176,7 +184,7 @@
319 // Select this button
320 function select() {
321 buttonView.selectedButtonIndex = button.buttonIndex;
322- buttonView.updateOffset();
323+ buttonView.updateOffset(button.offset);
324 }
325 }
326 }
327@@ -192,12 +200,14 @@
328 }
329
330 // set to the width of one tabButtonRow in Component.onCompleted.
331- property real buttonRowWidth
332+ property real buttonRowWidth: buttonRow1 ? buttonRow1.width : 0
333
334- property var buttons: []
335+ // set by the delegate when the components are completed.
336+ property Row buttonRow1
337+ property Row buttonRow2
338
339 // Track which button was last clicked
340- property int selectedButtonIndex: 0
341+ property int selectedButtonIndex: -1
342
343 delegate: tabButtonRow
344 model: 2 // The second buttonRow shows the buttons that disappear on the left
345@@ -222,21 +232,23 @@
346
347 // Select the closest of the two buttons that represent the given tab index
348 function selectButton(tabIndex) {
349- var b1 = buttons[tabIndex];
350- var b2 = buttons[tabIndex + tabs.__tabsModel.children.length];
351+ if (tabIndex < 0 || tabIndex >= tabs.__tabs.length) return;
352+ if (buttonView.buttonRow1 && buttonView.buttonRow2) {
353+ var b1 = buttonView.buttonRow1.children[tabIndex];
354+ var b2 = buttonView.buttonRow2.children[tabIndex];
355
356- // find the button with the nearest offset
357- var d1 = cyclicDistance(b1.offset, buttonView.offset, 2);
358- var d2 = cyclicDistance(b2.offset, buttonView.offset, 2);
359- if (d1 < d2) {
360- b1.select();
361- } else {
362- b2.select();
363+ // find the button with the nearest offset
364+ var d1 = cyclicDistance(b1.offset, buttonView.offset, 2);
365+ var d2 = cyclicDistance(b2.offset, buttonView.offset, 2);
366+ if (d1 < d2) {
367+ b1.select();
368+ } else {
369+ b2.select();
370+ }
371 }
372 }
373
374- function updateOffset() {
375- var newOffset = buttonView.buttons[buttonView.selectedButtonIndex].offset;
376+ function updateOffset(newOffset) {
377 if (offset - newOffset < -1) newOffset = newOffset - 2;
378 offset = newOffset;
379 }
380@@ -250,7 +262,6 @@
381
382 Component.onCompleted: {
383 selectButton(tabs.selectedTabIndex);
384- buttonRowWidth = currentItem.width;
385 }
386
387 onDragEnded: activatingTimer.stop()
388
389=== modified file 'themes/Ambiance/qmltheme/NewTabsDelegate.qml'
390--- themes/Ambiance/qmltheme/NewTabsDelegate.qml 2013-04-30 23:55:32 +0000
391+++ themes/Ambiance/qmltheme/NewTabsDelegate.qml 2013-05-07 11:26:29 +0000
392@@ -32,22 +32,23 @@
393 the next/previous tab.
394 */
395 property bool swipeToSwitchTabs
396- onSwipeToSwitchTabsChanged: print("swipeToSwitchTabs property is DEPRECATED. Please do not rely on swiping the Page's contents to switch tabs, this functionality will be removed.")
397+ /*!
398+ \deprecated
399+ \internal
400+ */
401+ onSwipeToSwitchTabsChanged: print("swipeToSwitchTabs property is DEPRECATED.")
402
403 property color headerTextColor
404 property color headerTextSelectedColor
405 property real headerTextOpacity
406 property real headerTextSelectedOpacity
407-
408 property int headerTextFadeDuration
409 property string headerFontSize
410 property int headerFontWeight
411 property real headerTextLeftMargin
412 property real headerTextRightMargin
413 property real headerTextBottomMargin
414-
415 property url indicatorImageSource
416-
417 property real tabBarHeight
418
419 /*!
420@@ -64,8 +65,6 @@
421 id: tabsDelegate
422 anchors.fill: parent
423
424- property VisualItemModel tabModel: item.__tabsModel
425-
426 // use theTabs property because item gives problems in the loader
427 property Tabs theTabs: item
428 property Component headerContents: Component {
429@@ -94,54 +93,7 @@
430 }
431 }
432
433- ListView {
434- id: tabView
435- anchors.fill: parent
436-
437- interactive: tabsDelegate.swipeToSwitchTabs
438- model: tabsDelegate.tabModel
439- onModelChanged: tabView.updatePages()
440- currentIndex: item.selectedTabIndex
441- onCurrentIndexChanged: if (item.__tabsModel.count > 0) item.selectedTabIndex = tabView.currentIndex
442-
443- orientation: ListView.Horizontal
444- snapMode: ListView.SnapOneItem
445- boundsBehavior: Flickable.DragOverBounds
446- highlightFollowsCurrentItem: true
447- highlightRangeMode: ListView.StrictlyEnforceRange
448-
449- function updatePages() {
450- if (!tabsDelegate.tabModel) return; // not initialized yet
451-
452- var tabList = tabsDelegate.tabModel.children
453- var tab;
454- for (var i=0; i < tabList.length; i++) {
455- tab = tabList[i];
456- tab.anchors.fill = undefined;
457- tab.width = tabView.width;
458- tab.height = tabView.height
459- }
460- tabView.updateSelectedTabIndex();
461- }
462-
463- function updateSelectedTabIndex() {
464- if (tabView.currentIndex === item.selectedTabIndex) return;
465- // The view is automatically updated, because highlightFollowsCurrentItem
466- tabView.currentIndex = item.selectedTabIndex;
467- }
468- }
469-
470- Connections {
471- target: item
472- onSelectedTabIndexChanged: {
473- tabView.updateSelectedTabIndex();
474- }
475- }
476-
477- onWidthChanged: tabView.updatePages();
478- onHeightChanged: tabView.updatePages();
479 Component.onCompleted: {
480 item.__headerContents = headerContents;
481- tabView.updatePages();
482 }
483 }

Subscribers

People subscribed via source and target branches

to status/vote changes: