BasicTableView: Use ListView's overlay header for the table header

Previoulsy we manually positioned the table header over the table
contents, which required some manual tweakings. Since 5.4, ListView
support overlay headers that we can use them to clean our code in
BasicTableView.

There is, however, a small issue on Mac where the vertical scrollbar
(specially in non-transient mode) is displayed below the table header.
Since the header can't span beyond the ListView boundaries, we need
to account for the vertical scrollbar width in the ListView's content
width.

This patch also updates the Desktop style to set the transientScrollBars
property and some small tweaks to the Base style.

Change-Id: I4433d0703328f23eadb0ae4563fbef5bba0168db
Task-number: QTBUG-34344
Reviewed-by: Mitch Curtis <mitch.curtis@theqtcompany.com>

6 files changed, 57 insertions, 47 deletions

diff --git a/src/controls/Private/BasicTableView.qml b/src/controls/Private/BasicTableView.qml
index 5fd21167..822086ee 100644
--- a/src/controls/Private/BasicTableView.qml
+++ b/src/controls/Private/BasicTableView.qml
@@ -45,8 +45,8 @@
 // We mean it.
 //
 
-import QtQuick 2.4
-import QtQuick.Controls 1.3
+import QtQuick 2.6
+import QtQuick.Controls 1.5
 import QtQuick.Controls.Private 1.0
 import QtQuick.Controls.Styles 1.2
 import QtQuick.Window 2.2
@@ -325,7 +325,7 @@ ScrollView {
     function resizeColumnsToContents () {
         for (var i = 0; i < __columns.length; ++i) {
             var col = getColumn(i)
-            var header = repeater.itemAt(i)
+            var header = __listView.headerItem.headerRepeater.itemAt(i)
             if (col) {
                 col.__index = i
                 col.resizeToContents()
@@ -349,8 +349,8 @@ ScrollView {
     implicitHeight: 150
 
     frameVisible: true
-    __scrollBarTopMargin: (__style && __style.transientScrollBars || Qt.platform.os === "osx") ? headerrow.height : 0
-    __viewTopMargin: headerVisible ? headerrow.height : 0
+    __scrollBarTopMargin: headerVisible && (listView.transientScrollBars || Qt.platform.os === "osx")
+                          ? listView.headerItem.height : 0
 
     /*! \internal
         Use this to display user-friendly messages in TableView and TreeView common functions.
@@ -395,13 +395,32 @@ ScrollView {
         focus: true
         activeFocusOnTab: false
         Keys.forwardTo: [__mouseArea]
-        anchors.topMargin: headerVisible ? tableHeader.height : 0
         anchors.fill: parent
+        contentWidth: headerItem.headerRow.width + listView.vScrollbarPadding
+        // ### FIXME Late configuration of the header item requires
+        // this binding to get the header visible after creation
+        contentY: -headerItem.height
+
         currentIndex: -1
         visible: columnCount > 0
         interactive: Settings.hasTouchScreen
         property var rowItemStack: [] // Used as a cache for rowDelegates
 
+        readonly property bool transientScrollbars: __style && !!__style.transientScrollBars
+        readonly property real vScrollbarPadding: __scroller.verticalScrollBar.visible
+                                                  && !transientScrollbars && Qt.platform.os === "osx" ?
+                                                  __verticalScrollBar.width + __scroller.scrollBarSpacing + root.__style.padding.right : 0
+
+        Binding {
+            // On Mac, we reserve the vSB space in the contentItem because the vSB should
+            // appear under the header. Unfortunately, the ListView header won't expand
+            // beyond the ListView's boundaries, that's why we need to ressort to this.
+            target: root.__scroller
+            when: Qt.platform.os === "osx"
+            property: "verticalScrollbarOffset"
+            value: 0
+        }
+
         function incrementCurrentIndexBlocking() {
             var oldIndex = __listView.currentIndex
             __scroller.blockUpdates = true;
@@ -572,7 +591,6 @@ ScrollView {
                     id: itemrow
                     height: parent.height
                     Repeater {
-                        id: repeater
                         model: columnModel
 
                         delegate: __itemDelegateLoader
@@ -588,27 +606,17 @@ ScrollView {
             }
         }
 
-        Item {
+        headerPositioning: ListView.OverlayHeader
+        header: Item {
             id: tableHeader
-            clip: true
-            parent: __scroller
             visible: headerVisible
-            anchors.top: parent.top
-            anchors.topMargin: viewport.anchors.topMargin
-            anchors.leftMargin: viewport.anchors.leftMargin
-            anchors.margins: viewport.anchors.margins
-            anchors.rightMargin: (frameVisible ? __scroller.rightMargin : 0) +
-                                 (__scroller.outerFrame && __scrollBarTopMargin ? 0
-                                  : __verticalScrollBar.width + __scroller.scrollBarSpacing + root.__style.padding.right)
-
-            anchors.left: parent.left
-            anchors.right: parent.right
-
-            height: headerrow.height
+            width: Math.max(headerRow.width + listView.vScrollbarPadding, root.viewport.width)
+            height: visible ? headerRow.height : 0
 
+            property alias headerRow: row
+            property alias headerRepeater: repeater
             Row {
-                id: headerrow
-                x: -listView.contentX
+                id: row
 
                 Repeater {
                     id: repeater
@@ -632,8 +640,7 @@ ScrollView {
                         Loader {
                             id: headerStyle
                             sourceComponent: root.headerDelegate
-                            anchors.left: parent.left
-                            anchors.right: parent.right
+                            width: parent.width
                             property QtObject styleData: QtObject {
                                 readonly property string value: modelData.title
                                 readonly property bool pressed: headerClickArea.pressed
@@ -670,7 +677,7 @@ ScrollView {
                             onPositionChanged: {
                                 if (drag.active && modelData.movable && pressed && columnCount > 1) { // only do this while dragging
                                     for (var h = columnCount-1 ; h >= 0 ; --h) {
-                                        if (drag.target.x + listView.contentX + headerRowDelegate.width/2 > headerrow.children[h].x) {
+                                        if (drag.target.x + listView.contentX + headerRowDelegate.width/2 > headerRow.children[h].x) {
                                             repeater.targetIndex = h
                                             break
                                         }
@@ -742,12 +749,9 @@ ScrollView {
                         }
                     }
                 }
-
-                onWidthChanged: listView.contentWidth = width
             }
 
             Loader {
-                id: loader
                 property QtObject styleData: QtObject{
                     readonly property string value: ""
                     readonly property bool pressed: false
@@ -758,17 +762,17 @@ ScrollView {
 
                 anchors.top: parent.top
                 anchors.right: parent.right
-                anchors.bottom: headerrow.bottom
-                anchors.rightMargin: -2
+                anchors.bottom: headerRow.bottom
                 sourceComponent: root.headerDelegate
-                width: root.width - headerrow.width + 2
-                visible: root.columnCount
+                readonly property real __remainingWidth: parent.width - headerRow.width
+                visible: __remainingWidth > 0
+                width: __remainingWidth
                 z:-1
             }
         }
 
         function columnAt(offset) {
-            var item = headerrow.childAt(offset, 0)
+            var item = listView.headerItem.headerRow.childAt(offset, 0)
             return item ? item.column : -1
         }
     }
diff --git a/src/controls/ScrollView.qml b/src/controls/ScrollView.qml
index e6001b04..a6eaed82 100644
--- a/src/controls/ScrollView.qml
+++ b/src/controls/ScrollView.qml
@@ -164,7 +164,9 @@ FocusScope {
     default property Item contentItem
 
     /*! \internal */
-    property Item __scroller: scroller
+    property alias __scroller: scroller
+    /*! \internal */
+    property alias __verticalScrollbarOffset: scroller.verticalScrollbarOffset
     /*! \internal */
     property alias __wheelAreaScrollSpeed: wheelArea.scrollSpeed
     /*! \internal */
@@ -328,9 +330,9 @@ FocusScope {
             property bool outerFrame: !frameVisible || !(__style ? __style.__externalScrollBars : 0)
             property int scrollBarSpacing: outerFrame ? 0 : (__style ? __style.__scrollBarSpacing : 0)
             property int verticalScrollbarOffset: verticalScrollBar.visible && !verticalScrollBar.isTransient ?
-                                                      verticalScrollBar.width + scrollBarSpacing : 0
+                                                  verticalScrollBar.width + scrollBarSpacing : 0
             property int horizontalScrollbarOffset: horizontalScrollBar.visible && !horizontalScrollBar.isTransient ?
-                                                        horizontalScrollBar.height + scrollBarSpacing : 0
+                                                    horizontalScrollBar.height + scrollBarSpacing : 0
             Loader {
                 id: frameLoader
                 sourceComponent: __style ? __style.frame : null
diff --git a/src/controls/Styles/Base/BasicTableViewStyle.qml b/src/controls/Styles/Base/BasicTableViewStyle.qml
index c8d817c9..e0a33406 100644
--- a/src/controls/Styles/Base/BasicTableViewStyle.qml
+++ b/src/controls/Styles/Base/BasicTableViewStyle.qml
@@ -84,7 +84,7 @@ ScrollViewStyle {
         See qtquickcontrolsstyles-tableviewstyle.qdoc and qtquickcontrolsstyles-treeviewstyle.qdoc
     */
     property Component headerDelegate: BorderImage {
-        height: textItem.implicitHeight * 1.2
+        height: Math.round(textItem.implicitHeight * 1.2)
         source: "images/header.png"
         border.left: 4
         border.bottom: 2
@@ -102,12 +102,9 @@ ScrollViewStyle {
             renderType: Settings.isMobile ? Text.QtRendering : Text.NativeRendering
         }
         Rectangle {
-            anchors.right: parent.right
-            anchors.top: parent.top
-            anchors.bottom: parent.bottom
-            anchors.bottomMargin: 1
-            anchors.topMargin: 1
             width: 1
+            height: parent.height - 2
+            y: 1
             color: "#ccc"
         }
     }
diff --git a/src/controls/Styles/Desktop/ScrollViewStyle.qml b/src/controls/Styles/Desktop/ScrollViewStyle.qml
index 0ec5788f..bba99fde 100644
--- a/src/controls/Styles/Desktop/ScrollViewStyle.qml
+++ b/src/controls/Styles/Desktop/ScrollViewStyle.qml
@@ -71,6 +71,7 @@ Style {
     readonly property bool __externalScrollBars: __styleitem.styleHint("externalScrollBars")
     readonly property int __scrollBarSpacing: __styleitem.pixelMetric("scrollbarspacing")
     readonly property bool scrollToClickedPosition: __styleitem.styleHint("scrollToClickPosition") !== 0
+    property bool transientScrollBars: false
 
     readonly property int __wheelScrollLines: __styleitem.styleHint("wheelScrollLines")
 
@@ -88,6 +89,8 @@ Style {
 
         implicitWidth: horizontal ? 200 : pixelMetric("scrollbarExtent")
         implicitHeight: horizontal ? pixelMetric("scrollbarExtent") : 200
+
+        onIsTransientChanged: root.transientScrollBars = isTransient
     }
 
 }
diff --git a/tests/auto/controls/data/tableview/table_buttondelegate.qml b/tests/auto/controls/data/tableview/table_buttondelegate.qml
index 3f1be964..3c347b36 100644
--- a/tests/auto/controls/data/tableview/table_buttondelegate.qml
+++ b/tests/auto/controls/data/tableview/table_buttondelegate.qml
@@ -55,15 +55,16 @@ TableView {
     onClicked: ++clickCount
 
     TableViewColumn {
+        id: column
         width: 100
     }
     rowDelegate: Item {
         height: 40
-        width: parent.width
+        width: column.width
     }
     itemDelegate: Button {
         height: 40
-        width: parent.width
+        width: column.width
         text: styleData.value
         activeFocusOnPress: true
         onClicked: ++table.buttonClickCount
diff --git a/tests/auto/controls/data/tst_tableview.qml b/tests/auto/controls/data/tst_tableview.qml
index a01df56f..8a821661 100644
--- a/tests/auto/controls/data/tst_tableview.qml
+++ b/tests/auto/controls/data/tst_tableview.qml
@@ -91,7 +91,10 @@ TestCase {
                                                 model: 10; \n\
                                         }', testCase, '')
         wait(50);
-        verify(table.__viewTopMargin > 0)
+        testCase.visible = true // ### FIXME Why do I need this at all?
+        verify(table.visible)
+        verify(table.__listView.headerItem.visible)
+        verify(table.__listView.headerItem.height > 0)
         table.destroy()
     }