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

[Tim Peeters]

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

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

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:455
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~tpeeters/ubuntu-ui-toolkit/tabfix/+merge/161902/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1112/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/./build=panda-pbuilder,distribution=quantal,flavor=armhf/1112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/./build=pbuilder-master,distribution=quantal,flavor=amd64/1112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/./build=pbuilder-master,distribution=quantal,flavor=i386/1112

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/1112/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:457
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/8/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/8/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/8/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/8/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:458
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/9/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/9/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/9/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/9/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:460
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/10/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/10/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/10/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/10/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:461
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/11/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/11/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/11/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/11/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:463
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/13/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/13/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/13/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/13/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:464
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/16

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/16/rebuild

review: Approve (continuous-integration)

[Zsombor Egri (zsombi)]

You have not documented the Repeater limitation in the Tabs.

review: Needs Fixing

[Zsombor Egri (zsombi)]

good to go now

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:467
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/32/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/32
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/32/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/32/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
Unapproved changes made after approval.
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-autolanding/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-autolanding/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-autolanding/22

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:469
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/36/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/36
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/36

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/36/rebuild

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/Tab.qml'
--- modules/Ubuntu/Components/Tab.qml	2013-04-03 12:21:17 +0000
+++ modules/Ubuntu/Components/Tab.qml	2013-05-07 11:26:29 +0000
@@ -59,4 +59,6 @@
      */
     active: parentNode && parentNode.active &&
             parentNode.hasOwnProperty("selectedTab") && parentNode.selectedTab === tab
+
+    visible: active
 }
 
=== modified file 'modules/Ubuntu/Components/Tabs.qml'
--- modules/Ubuntu/Components/Tabs.qml	2013-04-08 13:32:34 +0000
+++ modules/Ubuntu/Components/Tabs.qml	2013-05-07 11:26:29 +0000
@@ -90,6 +90,10 @@
 
     \endqml
     As the example above shows, an external \l Page inside a \l Tab can be loaded using a Loader.
+
+    It is possible to use a Repeater to generate tabs, but when doing so, ensure that the Repeater
+    is declared inside the Tabs at the end, because otherwise the shuffling of
+    the order of children by the Repeater can cause incorrect ordering of the tabs.
 */
 
 PageTreeNode {
@@ -104,13 +108,13 @@
       The first tab is 0, and -1 means that no tab is selected.
       The initial value is 0 if Tabs has contents, or -1 otherwise.
      */
-    property int selectedTabIndex: tabsModel.count > 0 ? 0 : -1
+    property int selectedTabIndex: tabs.__tabs.length > 0 ? 0 : -1
 
     /*!
       \preliminary
       The currently selected tab.
      */
-    readonly property Tab selectedTab: (selectedTabIndex < 0) || (tabsModel.count <= selectedTabIndex) ?
+    readonly property Tab selectedTab: (selectedTabIndex < 0) || (__tabs.length <= selectedTabIndex) ?
                                            null : __tabs[selectedTabIndex]
 
     /*!
@@ -139,21 +143,56 @@
                           "Pages will automatically update the toolbar when activated. "+
                           "See CHANGES file, and use toolbar.tools instead when needed.");
 
-
-    // FIXME: Using the VisualItemModel as a workaround for this bug:
-    //  "theming: contentItem does work when it is a VisualItemModel"
-    //  https://bugs.launchpad.net/tavastia/+bug/1080330
-    //  The workaround does not break the regular TabsDelegate.
-    /*! \internal */
-    default property alias __tabs: tabsModel.children
+    /*!
+      \internal
+      Used by the delegate to create the tabs header.
+    */
+    property alias __tabs: tabsModel.tabList
+
+    /*!
+      Children are placed in a separate item that has functionality to extract the Tab items.
+      \qmlproperty list<Item> tabChildren
+     */
+    default property alias tabChildren: tabsModel.children
+
+    /*!
+      Used by the tabs delegate to update the tabs header with the titles of all the tabs.
+      This signal is used in an intermediate step in transitioning the tabs to a new
+      implementation and may be removed in the future.
+     */
+    signal modelChanged()
 
     /*!
       \internal
       required by NewTabsDelegate
      */
-    property alias __tabsModel: tabsModel
-    VisualItemModel {
+    Item {
+        anchors.fill: parent
         id: tabsModel
+
+        property var tabList: []
+        onChildrenChanged: {
+            updateTabList();
+        }
+
+        function updateTabList() {
+            var list = [];
+            for (var i=0; i < children.length; i++) {
+                if (isTab(tabsModel.children[i])) list.push(tabsModel.children[i]);
+            }
+            tabList = list;
+            tabs.modelChanged();
+        }
+
+        function isTab(item) {
+            if (item && item.hasOwnProperty("__isPageTreeNode")
+                    && item.__isPageTreeNode && item.hasOwnProperty("title")
+                    && item.hasOwnProperty("page")) {
+                return true;
+            } else {
+                return false;
+            }
+        }
     }
 
     /*! \internal */
 
=== added directory 'tests/resources/tabs'
=== added file 'tests/resources/tabs/MyCustomPage.qml'
--- tests/resources/tabs/MyCustomPage.qml	1970-01-01 00:00:00 +0000
+++ tests/resources/tabs/MyCustomPage.qml	2013-05-07 11:26:29 +0000
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import Ubuntu.Components 0.1
+
+Page {
+    title: i18n.tr("My custom page")
+
+    Flickable {
+        anchors.fill: parent
+        contentHeight: parent.height + units.gu(10)
+        Label {
+            anchors {
+                top: parent.top
+                topMargin: units.gu(16)
+                horizontalCenter: parent.horizontalCenter
+            }
+
+            text: i18n.tr("This is an external page\nwith a locked toolbar.")
+            color: "#757373"
+        }
+    }
+
+    tools: ToolbarActions {
+        Action {
+            text: "action 1"
+            iconSource: Qt.resolvedUrl("call_icon.png")
+        }
+        Action {
+            text: "action 2"
+            iconSource: Qt.resolvedUrl("call_icon.png")
+        }
+        opened: true
+        locked: true
+    }
+}
 
=== added file 'tests/resources/tabs/Tabs.qml'
--- tests/resources/tabs/Tabs.qml	1970-01-01 00:00:00 +0000
+++ tests/resources/tabs/Tabs.qml	2013-05-07 11:26:29 +0000
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import Ubuntu.Components 0.1
+import Ubuntu.Components.ListItems 0.1 as ListItem
+
+MainView {
+    width: 800
+    height: 600
+    Tabs {
+        id: tabs
+        selectedTabIndex: 0
+        Item {
+            // does this mess up stuff? nope.
+        }
+
+        Tab {
+            title: i18n.tr("Simple page")
+            page: Page {
+                Label {
+                    id: label
+                    anchors.centerIn: parent
+                    text: "A centered label"
+                }
+                tools: ToolbarActions {
+                    Action {
+                        text: "action"
+                        iconSource: "call_icon.png"
+                        onTriggered: print("action triggered")
+                    }
+                }
+            }
+        }
+        Repeater {
+            model: 3
+            Tab {
+                title: "Extra " + index
+                page: Page {
+                    Column {
+                        anchors.centerIn: parent
+                        width: units.gu(40)
+                        Label {
+                            anchors {
+                                left: parent.left
+                                right: parent.right
+                            }
+                            text: "Extra tab number "+index
+                        }
+                        Button {
+                            anchors {
+                                left: parent.left
+                                right: parent.right
+                            }
+                            text: "Previous"
+                            onClicked: if (tabs.selectedTabIndex > 0) tabs.selectedTabIndex--
+                        }
+                    }
+                }
+            }
+        }
+        Tab {
+            id: externalTab
+            title: i18n.tr("External")
+            page: Loader {
+                parent: externalTab
+                anchors.fill: parent
+                source: (tabs.selectedTab === externalTab) ? Qt.resolvedUrl("MyCustomPage.qml") : ""
+            }
+        }
+        Tab {
+            title: i18n.tr("List view")
+            page: Page {
+                ListView {
+                    clip: true
+                    anchors.fill: parent
+                    model: 20
+                    delegate: ListItem.Standard {
+                        icon: Qt.resolvedUrl("call_icon.png")
+                        text: "Item "+modelData
+                    }
+                }
+            }
+        }
+    }
+}
 
=== added file 'tests/resources/tabs/[e-mail]'
 Binary files tests/resources/tabs/[e-mail]	1970-01-01 00:00:00 +0000 and tests/resources/tabs/[e-mail]	2013-05-07 11:26:29 +0000 differ
=== modified file 'themes/Ambiance/qmltheme/NewTabBar.qml'
--- themes/Ambiance/qmltheme/NewTabBar.qml	2013-05-02 00:57:01 +0000
+++ themes/Ambiance/qmltheme/NewTabBar.qml	2013-05-07 11:26:29 +0000
@@ -66,6 +66,7 @@
     Connections {
         target: tabs
         onSelectedTabIndexChanged: buttonView.selectButton(tabs.selectedTabIndex)
+        onModelChanged: buttonView.selectButton(tabs.selectedTabIndex)
     }
 
     Component {
@@ -79,9 +80,17 @@
             width: childrenRect.width
             property int rowNumber: modelData
 
+            Component.onCompleted: {
+                if (rowNumber === 0) {
+                    buttonView.buttonRow1 = theRow;
+                } else {
+                    buttonView.buttonRow2 = theRow;
+                }
+            }
+
             Repeater {
                 id: repeater
-                model: tabs.__tabsModel.children
+                model: tabs.__tabs
 
                 AbstractButton {
                     id: button
@@ -97,7 +106,6 @@
                     property bool selected: (tabBar.active && buttonView.needsScrolling) ? tabs.selectedTabIndex === index : buttonView.selectedButtonIndex === button.buttonIndex
                     property real offset: theRow.rowNumber + 1 - button.x / theRow.width;
                     property int buttonIndex: index + theRow.rowNumber*repeater.count
-                    Component.onCompleted: buttonView.buttons.push(button)
 
                     // Use opacity 0 to hide instead of setting visibility to false in order to
                     // make fading work well, and not to mess up width/offset computations
@@ -109,7 +117,7 @@
 
                         // When we don't need scrolling, we want to avoid showing a button that is fading
                         // while sliding in from the right side when a new button was selected
-                        var numTabs = tabs.__tabsModel.count;
+                        var numTabs = tabs.__tabs.length;
                         var minimum = buttonView.selectedButtonIndex;
                         var maximum = buttonView.selectedButtonIndex + numTabs - 1;
                         if (MathUtils.clamp(buttonIndex, minimum, maximum) === buttonIndex) return true;
@@ -176,7 +184,7 @@
                     // Select this button
                     function select() {
                         buttonView.selectedButtonIndex = button.buttonIndex;
-                        buttonView.updateOffset();
+                        buttonView.updateOffset(button.offset);
                     }
                 }
             }
@@ -192,12 +200,14 @@
         }
 
         // set to the width of one tabButtonRow in Component.onCompleted.
-        property real buttonRowWidth
+        property real buttonRowWidth: buttonRow1 ? buttonRow1.width : 0
 
-        property var buttons: []
+        // set by the delegate when the components are completed.
+        property Row buttonRow1
+        property Row buttonRow2
 
         // Track which button was last clicked
-        property int selectedButtonIndex: 0
+        property int selectedButtonIndex: -1
 
         delegate: tabButtonRow
         model: 2 // The second buttonRow shows the buttons that disappear on the left
@@ -222,21 +232,23 @@
 
         // Select the closest of the two buttons that represent the given tab index
         function selectButton(tabIndex) {
-            var b1 = buttons[tabIndex];
-            var b2 = buttons[tabIndex + tabs.__tabsModel.children.length];
+            if (tabIndex < 0 || tabIndex >= tabs.__tabs.length) return;
+            if (buttonView.buttonRow1 && buttonView.buttonRow2) {
+                var b1 = buttonView.buttonRow1.children[tabIndex];
+                var b2 = buttonView.buttonRow2.children[tabIndex];
 
-            // find the button with the nearest offset
-            var d1 = cyclicDistance(b1.offset, buttonView.offset, 2);
-            var d2 = cyclicDistance(b2.offset, buttonView.offset, 2);
-            if (d1 < d2) {
-                b1.select();
-            } else {
-                b2.select();
+                // find the button with the nearest offset
+                var d1 = cyclicDistance(b1.offset, buttonView.offset, 2);
+                var d2 = cyclicDistance(b2.offset, buttonView.offset, 2);
+                if (d1 < d2) {
+                    b1.select();
+                } else {
+                    b2.select();
+                }
             }
         }
 
-        function updateOffset() {
-            var newOffset = buttonView.buttons[buttonView.selectedButtonIndex].offset;
+        function updateOffset(newOffset) {
             if (offset - newOffset < -1) newOffset = newOffset - 2;
             offset = newOffset;
         }
@@ -250,7 +262,6 @@
 
         Component.onCompleted: {
             selectButton(tabs.selectedTabIndex);
-            buttonRowWidth = currentItem.width;
         }
 
         onDragEnded: activatingTimer.stop()
 
=== modified file 'themes/Ambiance/qmltheme/NewTabsDelegate.qml'
--- themes/Ambiance/qmltheme/NewTabsDelegate.qml	2013-04-30 23:55:32 +0000
+++ themes/Ambiance/qmltheme/NewTabsDelegate.qml	2013-05-07 11:26:29 +0000
@@ -32,22 +32,23 @@
       the next/previous tab.
      */
     property bool swipeToSwitchTabs
-    onSwipeToSwitchTabsChanged: print("swipeToSwitchTabs property is DEPRECATED. Please do not rely on swiping the Page's contents to switch tabs, this functionality will be removed.")
+    /*!
+      \deprecated
+      \internal
+     */
+    onSwipeToSwitchTabsChanged: print("swipeToSwitchTabs property is DEPRECATED.")
 
     property color headerTextColor
     property color headerTextSelectedColor
     property real headerTextOpacity
     property real headerTextSelectedOpacity
-
     property int headerTextFadeDuration
     property string headerFontSize
     property int headerFontWeight
     property real headerTextLeftMargin
     property real headerTextRightMargin
     property real headerTextBottomMargin
-
     property url indicatorImageSource
-
     property real tabBarHeight
 
     /*!
@@ -64,8 +65,6 @@
     id: tabsDelegate
     anchors.fill: parent
 
-    property VisualItemModel tabModel: item.__tabsModel
-
     // use theTabs property because item gives problems in the loader
     property Tabs theTabs: item
     property Component headerContents: Component {
@@ -94,54 +93,7 @@
         }
     }
 
-    ListView {
-        id: tabView
-        anchors.fill: parent
-
-        interactive: tabsDelegate.swipeToSwitchTabs
-        model: tabsDelegate.tabModel
-        onModelChanged: tabView.updatePages()
-        currentIndex: item.selectedTabIndex
-        onCurrentIndexChanged: if (item.__tabsModel.count > 0) item.selectedTabIndex = tabView.currentIndex
-
-        orientation: ListView.Horizontal
-        snapMode: ListView.SnapOneItem
-        boundsBehavior: Flickable.DragOverBounds
-        highlightFollowsCurrentItem: true
-        highlightRangeMode: ListView.StrictlyEnforceRange
-
-        function updatePages() {
-            if (!tabsDelegate.tabModel) return; // not initialized yet
-
-            var tabList = tabsDelegate.tabModel.children
-            var tab;
-            for (var i=0; i < tabList.length; i++) {
-                tab = tabList[i];
-                tab.anchors.fill = undefined;
-                tab.width = tabView.width;
-                tab.height = tabView.height
-            }
-            tabView.updateSelectedTabIndex();
-        }
-
-        function updateSelectedTabIndex() {
-            if (tabView.currentIndex === item.selectedTabIndex) return;
-            // The view is automatically updated, because highlightFollowsCurrentItem
-            tabView.currentIndex = item.selectedTabIndex;
-        }
-    }
-
-    Connections {
-        target: item
-        onSelectedTabIndexChanged: {
-            tabView.updateSelectedTabIndex();
-        }
-    }
-
-    onWidthChanged: tabView.updatePages();
-    onHeightChanged: tabView.updatePages();
     Component.onCompleted: {
         item.__headerContents = headerContents;
-        tabView.updatePages();
     }
 }