Commit 3b80b000 authored by Nate Graham's avatar Nate Graham 🔩 Committed by Matthieu Gallien
Browse files

Replace homemade ScrollHelper with standard ScrollViews to hold view items

We created the ScrollHelper component to fix scrolling with touchpads,
which is quite terrible out of the box with QML's default flickables.

However this solution had some drawbacks:

- It broke touch scrolling
- It broke fast scrolling by holding down the shift key while scrolling
- It essentially re-wrote scroll handling locally, which is fragile
- It required a bunch of extra code to handle scrollbar visibility which
  introduced various minor visual bugs in a few places

In addition, this solution was not necessary since we can get the
same result by putting views inside ScrollView components, which is a
100% supported and standard paradigm. Doing so preserves the nice
touchpad scrolling we already have but fixes all of the above items
automatically.

BUG: 417859
BUG: 427967
FIXED-IN: 20.12
parent b49c0af4
Pipeline #40628 canceled with stage
......@@ -415,7 +415,6 @@ if (Qt5Quick_FOUND AND Qt5Widgets_FOUND)
qml/GridBrowserDelegate.qml
qml/ListBrowserView.qml
qml/ListBrowserDelegate.qml
qml/ScrollHelper.qml
qml/FlatButtonWithToolTip.qml
qml/HeaderFooterToolbar.qml
......
......@@ -175,46 +175,38 @@ FocusScope {
clip: true
GridView {
id: contentDirectoryView
property int availableWidth: scrollBar.visible ? width - scrollBar.width : width
ScrollView {
id: scrollView
activeFocusOnTab: true
keyNavigationEnabled: true
model: delegateModel
readonly property int scrollBarWidth: ScrollBar.vertical.visible ? ScrollBar.vertical.width : 0
readonly property int availableSpace: scrollView.width - scrollView.scrollBarWidth
anchors.fill: parent
anchors.leftMargin: (LayoutMirroring.enabled && scrollBar.visible) ? 0 : Kirigami.Units.largeSpacing
anchors.rightMargin: (!LayoutMirroring.enabled && scrollBar.visible) ? 0 : Kirigami.Units.largeSpacing
anchors.topMargin: Kirigami.Units.largeSpacing
anchors.bottomMargin: Kirigami.Units.largeSpacing
ScrollBar.vertical: ScrollBar {
id: scrollBar
}
boundsBehavior: Flickable.StopAtBounds
GridView {
id: contentDirectoryView
currentIndex: -1
clip: true
activeFocusOnTab: true
keyNavigationEnabled: true
Accessible.role: Accessible.List
Accessible.name: mainTitle
model: delegateModel
ScrollHelper {
id: scrollHelper
flickable: contentDirectoryView
anchors.fill: contentDirectoryView
}
currentIndex: -1
Kirigami.PlaceholderMessage {
anchors.centerIn: parent
width: parent.width - (Kirigami.Units.largeSpacing * 4)
visible: contentDirectoryView.count === 0 && !suppressNoDataPlaceholderMessage
text: i18n("Nothing to display")
}
Accessible.role: Accessible.List
Accessible.name: mainTitle
cellWidth: Math.floor(availableWidth / Math.max(Math.floor(availableWidth / elisaTheme.gridDelegateSize), 2))
cellHeight: elisaTheme.gridDelegateSize + Kirigami.Units.gridUnit * 2 + Kirigami.Units.largeSpacing
Kirigami.PlaceholderMessage {
anchors.centerIn: parent
width: parent.width - (Kirigami.Units.largeSpacing * 4)
visible: contentDirectoryView.count === 0 && !suppressNoDataPlaceholderMessage
text: i18n("Nothing to display")
}
cellWidth: Math.floor(scrollView.availableSpace / Math.max(Math.floor(scrollView.availableSpace / elisaTheme.gridDelegateSize), 2))
cellHeight: elisaTheme.gridDelegateSize + Kirigami.Units.gridUnit * 2 + Kirigami.Units.largeSpacing
}
}
}
}
......
......@@ -28,7 +28,7 @@ FocusScope {
property bool haveTreeModel: false
property alias showRating: navigationBar.showRating
property alias allowArtistNavigation: navigationBar.allowArtistNavigation
property var delegateWidth: scrollBar.visible ? contentDirectoryView.width - scrollBar.width : contentDirectoryView.width
property var delegateWidth: contentDirectoryView.width
property alias currentIndex: contentDirectoryView.currentIndex
property alias enableSorting: navigationBar.enableSorting
property alias sortRole: navigationBar.sortRole
......@@ -144,51 +144,43 @@ FocusScope {
Layout.fillWidth: true
Layout.margins: 2
ListView {
id: contentDirectoryView
ScrollView {
anchors.fill: parent
Accessible.role: Accessible.List
Accessible.name: mainTitle
Accessible.description: mainTitle
ListView {
id: contentDirectoryView
activeFocusOnTab: true
keyNavigationEnabled: true
Accessible.role: Accessible.List
Accessible.name: mainTitle
Accessible.description: mainTitle
model: delegateModel
clip: true
activeFocusOnTab: true
keyNavigationEnabled: true
currentIndex: -1
model: delegateModel
section.property: (showSection ? 'discNumber' : '')
section.criteria: ViewSection.FullString
section.labelPositioning: ViewSection.InlineLabels
section.delegate: TracksDiscHeader {
discNumber: section
width: scrollBar.visible ? (!LayoutMirroring.enabled ? contentDirectoryView.width - scrollBar.width : contentDirectoryView.width) : contentDirectoryView.width
}
currentIndex: -1
ScrollBar.vertical: ScrollBar {
id: scrollBar
}
boundsBehavior: Flickable.StopAtBounds
clip: true
section.property: (showSection ? 'discNumber' : '')
section.criteria: ViewSection.FullString
section.labelPositioning: ViewSection.InlineLabels
section.delegate: TracksDiscHeader {
discNumber: section
width: viewModeView
}
ScrollHelper {
id: scrollHelper
flickable: contentDirectoryView
anchors.fill: contentDirectoryView
}
Kirigami.PlaceholderMessage {
anchors.centerIn: parent
width: parent.width - (Kirigami.Units.largeSpacing * 4)
visible: contentDirectoryView.count === 0 && !suppressNoDataPlaceholderMessage
text: i18n("Nothing to display")
}
Kirigami.PlaceholderMessage {
anchors.centerIn: parent
width: parent.width - (Kirigami.Units.largeSpacing * 4)
visible: contentDirectoryView.count === 0 && !suppressNoDataPlaceholderMessage
text: i18n("Nothing to display")
onCountChanged: if (count === 0) {
currentIndex = -1;
}
}
onCountChanged: if (count === 0) {
currentIndex = -1;
}
}
}
}
......
......@@ -88,45 +88,37 @@ Window {
Layout.maximumWidth: elisaTheme.coverImageSize
}
ListView {
id: trackData
ScrollView {
Layout.fillWidth: true
Layout.fillHeight: true
Layout.leftMargin: 2 * Kirigami.Units.largeSpacing
focus: true
ListView {
id: trackData
ScrollBar.vertical: ScrollBar {
id: scrollBar
}
boundsBehavior: Flickable.StopAtBounds
clip: true
clip: true
focus: true
ScrollHelper {
id: scrollHelper
flickable: trackData
anchors.fill: trackData
}
model: realModel
model: realModel
Component {
id: metaDataDelegate
Component {
id: metaDataDelegate
MetaDataDelegate {
width: scrollBar.visible ? (!LayoutMirroring.enabled ? trackData.width - scrollBar.width : trackData.width) : trackData.width
MetaDataDelegate {
width: trackData.width
}
}
}
Component {
id: editableMetaDataDelegate
Component {
id: editableMetaDataDelegate
EditableMetaDataDelegate {
width: scrollBar.visible ? (!LayoutMirroring.enabled ? trackData.width - scrollBar.width : trackData.width) : trackData.width
EditableMetaDataDelegate {
width: trackData.width
}
}
}
delegate: editableMetadata ? editableMetaDataDelegate: metaDataDelegate
delegate: editableMetadata ? editableMetaDataDelegate: metaDataDelegate
}
}
}
......
......@@ -10,8 +10,8 @@ import QtQml.Models 2.1
import org.kde.elisa 1.0
ListView {
id: playListView
ScrollView {
id: scrollView
property alias playListModel: playListModelDelegate.model
property string title
......@@ -20,146 +20,139 @@ ListView {
signal pausePlayback()
signal displayError(var errorText)
focus: true
keyNavigationEnabled: true
activeFocusOnTab: true
ListView {
id: playListView
currentIndex: -1
focus: true
clip: true
keyNavigationEnabled: true
activeFocusOnTab: true
Accessible.role: Accessible.List
Accessible.name: title
currentIndex: -1
section.property: 'albumSection'
section.criteria: ViewSection.FullString
section.labelPositioning: ViewSection.InlineLabels
section.delegate: BasicPlayListAlbumHeader {
headerData: JSON.parse(section)
width: scrollBar.visible ? (!LayoutMirroring.enabled ? playListView.width - scrollBar.width : playListView.width) : playListView.width
}
ScrollBar.vertical: ScrollBar {
id: scrollBar
}
boundsBehavior: Flickable.StopAtBounds
clip: true
ScrollHelper {
id: scrollHelper
flickable: playListView
anchors.fill: playListView
}
/* currently disabled animations due to display corruption
because of https://bugreports.qt.io/browse/QTBUG-49868
causing https://bugs.kde.org/show_bug.cgi?id=406524
and https://bugs.kde.org/show_bug.cgi?id=398093
add: Transition {
NumberAnimation {
property: "opacity";
from: 0;
to: 1;
duration: Kirigami.Units.shortDuration }
}
Accessible.role: Accessible.List
Accessible.name: scrollView.title
populate: Transition {
NumberAnimation {
property: "opacity";
from: 0;
to: 1;
duration: Kirigami.Units.shortDuration }
}
section.property: 'albumSection'
section.criteria: ViewSection.FullString
section.labelPositioning: ViewSection.InlineLabels
section.delegate: BasicPlayListAlbumHeader {
headerData: JSON.parse(section)
width: playListView.width
}
remove: Transition {
NumberAnimation {
property: "opacity";
from: 1.0;
to: 0;
duration: Kirigami.Units.shortDuration }
}
/* currently disabled animations due to display corruption
because of https://bugreports.qt.io/browse/QTBUG-49868
causing https://bugs.kde.org/show_bug.cgi?id=406524
and https://bugs.kde.org/show_bug.cgi?id=398093
add: Transition {
NumberAnimation {
property: "opacity";
from: 0;
to: 1;
duration: Kirigami.Units.shortDuration }
}
displaced: Transition {
NumberAnimation {
properties: "x,y";
duration: Kirigami.Units.shortDuration
easing.type: Easing.InOutQuad }
}
*/
populate: Transition {
NumberAnimation {
property: "opacity";
from: 0;
to: 1;
duration: Kirigami.Units.shortDuration }
}
model: DelegateModel {
id: playListModelDelegate
remove: Transition {
NumberAnimation {
property: "opacity";
from: 1.0;
to: 0;
duration: Kirigami.Units.shortDuration }
}
groups: [
DelegateModelGroup { name: "selected" }
]
displaced: Transition {
NumberAnimation {
properties: "x,y";
duration: Kirigami.Units.shortDuration
easing.type: Easing.InOutQuad }
}
*/
delegate: DraggableItem {
id: item
placeholderHeight: topItem.placeholderHeight
model: DelegateModel {
id: playListModelDelegate
focus: true
groups: [
DelegateModelGroup { name: "selected" }
]
PlayListEntry {
id: entry
delegate: DraggableItem {
id: item
width: playListView.width
placeholderHeight: topItem.placeholderHeight
focus: true
width: scrollBar.visible ? (!LayoutMirroring.enabled ? playListView.width - scrollBar.width : playListView.width) : playListView.width
scrollBarWidth: scrollBar.visible ? scrollBar.width : 0
index: model.index
isAlternateColor: item.DelegateModel.itemsIndex % 2
isSelected: playListView.currentIndex === index
containsMouse: item.containsMouse
databaseId: model.databaseId ? model.databaseId : 0
entryType: model.entryType ? model.entryType : ElisaUtils.Unknown
title: model.title ? model.title : ''
artist: model.artist ? model.artist : ''
album: model.album ? model.album : ''
albumArtist: model.albumArtist ? model.albumArtist : ''
duration: model.duration ? model.duration : ''
fileName: model.trackResource ? model.trackResource : ''
imageUrl: model.imageUrl ? model.imageUrl : ''
trackNumber: model.trackNumber ? model.trackNumber : -1
discNumber: model.discNumber ? model.discNumber : -1
rating: model.rating ? model.rating : 0
isSingleDiscAlbum: model.isSingleDiscAlbum !== undefined ? model.isSingleDiscAlbum : true
isValid: model.isValid
isPlaying: model.isPlaying
onStartPlayback: playListView.startPlayback()
onPausePlayback: playListView.pausePlayback()
onRemoveFromPlaylist: playListView.playListModel.removeRow(trackIndex)
onSwitchToTrack: playListView.playListModel.switchTo(trackIndex)
onActiveFocusChanged: {
if (activeFocus && playListView.currentIndex !== index) {
playListView.currentIndex = index
PlayListEntry {
id: entry
focus: true
width: parent.width
index: model.index
isAlternateColor: item.DelegateModel.itemsIndex % 2
isSelected: playListView.currentIndex === index
containsMouse: item.containsMouse
databaseId: model.databaseId ? model.databaseId : 0
entryType: model.entryType ? model.entryType : ElisaUtils.Unknown
title: model.title ? model.title : ''
artist: model.artist ? model.artist : ''
album: model.album ? model.album : ''
albumArtist: model.albumArtist ? model.albumArtist : ''
duration: model.duration ? model.duration : ''
fileName: model.trackResource ? model.trackResource : ''
imageUrl: model.imageUrl ? model.imageUrl : ''
trackNumber: model.trackNumber ? model.trackNumber : -1
discNumber: model.discNumber ? model.discNumber : -1
rating: model.rating ? model.rating : 0
isSingleDiscAlbum: model.isSingleDiscAlbum !== undefined ? model.isSingleDiscAlbum : true
isValid: model.isValid
isPlaying: model.isPlaying
onStartPlayback: scrollView.startPlayback()
onPausePlayback: scrollView.pausePlayback()
onRemoveFromPlaylist: scrollView.playListModel.removeRow(trackIndex)
onSwitchToTrack: scrollView.playListModel.switchTo(trackIndex)
onActiveFocusChanged: {
if (activeFocus && playListView.currentIndex !== index) {
playListView.currentIndex = index
}
}
}
}
draggedItemParent: playListView
draggedItemParent: playListView
onClicked: {
playListView.currentIndex = index
entry.forceActiveFocus()
}
onClicked: {
playListView.currentIndex = index
entry.forceActiveFocus()
}
onDoubleClicked: {
if (model.isValid) {
playListView.playListModel.switchTo(model.index)
playListView.startPlayback()
onDoubleClicked: {
if (model.isValid) {
scrollView.playListModel.switchTo(model.index)
scrollView.startPlayback()
}
}
}
onMoveItemRequested: {
playListModel.moveRow(from, to);
onMoveItemRequested: {
playListModel.moveRow(from, to);
}
}
}
}
onCountChanged: if (count === 0) {
currentIndex = -1;
}
onCountChanged: if (count === 0) {
currentIndex = -1;
}
}
}
......@@ -35,7 +35,6 @@ FocusScope {
property int discNumber
property int rating
property bool hasValidDiscNumber: true
property int scrollBarWidth
property bool simpleMode: false
signal startPlayback()
......@@ -75,7 +74,6 @@ FocusScope {
id: entryBackground
anchors.fill: parent
anchors.rightMargin: LayoutMirroring.enabled ? scrollBarWidth : 0
z: 1
color: simpleMode ? "transparent" : myPalette.base
......@@ -102,7 +100,6 @@ FocusScope {
anchors.fill: parent
anchors.leftMargin: Kirigami.Units.largeSpacing
anchors.rightMargin: LayoutMirroring.enabled ? scrollBarWidth : 0
spacing: Kirigami.Units.smallSpacing / 2
......
/*
SPDX-FileCopyrightText: 2016 (c) Michael Bohlender <michael.bohlender@kdemail.net>
SPDX-FileCopyrightText: 2017 (c) Christian Mollekopf <mollekopf@kolabsystems.com>
SPDX-License-Identifier: GPL-2.0-or-later
*/
import QtQuick 2.7
import QtQuick.Controls 2.2
/*
* The MouseArea + interactive: false + maximumFlickVelocity are required
* to fix scrolling for desktop systems where we don't want flicking behaviour.
*
* See also:
* ScrollView.qml in qtquickcontrols
* qquickwheelarea.cpp in qtquickcontrols
*/
MouseArea {
id: root
propagateComposedEvents: true
property Flickable flickable
//Place the mouse area under the flickable
z: -1
onFlickableChanged: {
flickable.interactive = false
flickable.maximumFlickVelocity = 100000
flickable.boundsBehavior = Flickable.StopAtBounds
root.parent = flickable
}
function scrollByPixelDelta(flickableItem, pixelDelta) {
if (!pixelDelta) {
return flickableItem.contentY;
}
var minYExtent = flickableItem.originY + flickableItem.topMargin;
var maxYExtent = (flickableItem.contentHeight + flickableItem.bottomMargin + flickableItem.originY) - flickableItem.height;
if (typeof(flickableItem.headerItem) !== "undefined" && flickableItem.headerItem) {
minYExtent += flickableItem.headerItem.height
}
//Avoid overscrolling
return Math.max(minYExtent, Math.min(maxYExtent, flickableItem.contentY - pixelDelta));
}
function calculateNewPosition(flickableItem, wheel) {
//Nothing to scroll
if (flickableItem.contentHeight < flickableItem.height) {
return flickableItem.contentY;
}
//Ignore 0 events (happens at least with Christians trackpad)
if (wheel.pixelDelta.y === 0 && wheel.angleDelta.y === 0) {
return flickableItem.contentY;
}
//pixelDelta seems to be the same as angleDelta/8
var pixelDelta = 0
//The pixelDelta is a smaller number if both are provided, so pixelDelta can be 0 while angleDelta is still something. So we check the angleDelta
if (wheel.angleDelta.y) {
var wheelScrollLines = 3 //Default value of QApplication wheelScrollLines property
var pixelPerLine = 20 //Default value in Qt, originally comes from QTextEdit
var ticks = (wheel.angleDelta.y / 8) / 15.0 //Divide by 8 gives us pixels typically come in 15pixel steps.
pixelDelta = ticks * pixelPerLine * wheelScrollLines
} else {
pixelDelta = wheel.pixelDelta.y
}
return scrollByPixelDelta(flickableItem, pixelDelta);
}
function scrollDown() {
flickable.flick(0, 0);
flickable.contentY = scrollByPixelDelta(flickable, -60); //3 lines * 20 pixels
}
function scrollUp() {
flickable.flick(0, 0);
flickable.contentY = scrollByPixelDelta(flickable, 60); //3 lines * 20 pixels
}
onWheel: {
var newPos = calculateNewPosition(flickable, wheel);