From 9bf67d08cf32398b05c6bb24dc52c3b9e4cded81 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 17 Apr 2018 15:51:50 +0200 Subject: [PATCH 1/2] Use a default timeout of 250 ms between selection and activation --- src/library/baseplaylistfeature.cpp | 8 +------- src/library/crate/cratefeature.cpp | 4 +--- src/library/libraryfeature.cpp | 13 +++++++++++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/library/baseplaylistfeature.cpp b/src/library/baseplaylistfeature.cpp index c0506e458d..2cfd68e974 100644 --- a/src/library/baseplaylistfeature.cpp +++ b/src/library/baseplaylistfeature.cpp @@ -19,17 +19,11 @@ #include "widget/wlibrarytextbrowser.h" #include "util/assert.h" -namespace { - -const int kClickedChildActivationTimeoutMillis = 100; - -} // anonymous namespace - BasePlaylistFeature::BasePlaylistFeature(QObject* parent, UserSettingsPointer pConfig, TrackCollection* pTrackCollection, QString rootViewName) - : LibraryFeature(pConfig, kClickedChildActivationTimeoutMillis, parent), + : LibraryFeature(pConfig, parent), m_pTrackCollection(pTrackCollection), m_playlistDao(pTrackCollection->getPlaylistDAO()), m_trackDao(pTrackCollection->getTrackDAO()), diff --git a/src/library/crate/cratefeature.cpp b/src/library/crate/cratefeature.cpp index da75522e39..38c09b0eb8 100644 --- a/src/library/crate/cratefeature.cpp +++ b/src/library/crate/cratefeature.cpp @@ -29,8 +29,6 @@ namespace { -const int kClickedChildActivationTimeoutMillis = 100; - QString formatLabel( const CrateSummary& crateSummary) { return QString("%1 (%2) %3").arg( @@ -44,7 +42,7 @@ QString formatLabel( CrateFeature::CrateFeature(Library* pLibrary, TrackCollection* pTrackCollection, UserSettingsPointer pConfig) - : LibraryFeature(pConfig, kClickedChildActivationTimeoutMillis), + : LibraryFeature(pConfig), m_cratesIcon(":/images/library/ic_library_crates.png"), m_lockedCrateIcon(":/images/library/ic_library_locked.png"), m_pTrackCollection(pTrackCollection), diff --git a/src/library/libraryfeature.cpp b/src/library/libraryfeature.cpp index e582557370..701e17a513 100644 --- a/src/library/libraryfeature.cpp +++ b/src/library/libraryfeature.cpp @@ -7,10 +7,19 @@ // The reason for this is that LibraryFeature uses slots/signals and for this // to work the code has to be precompiles by moc +namespace { + +// The time between selecting and activating a feature item in the left +// pane. This is required to allow smooth and responsive scrolling through +// a list of items with an encoder! +const int kDefaultClickedChildActivationTimeoutMillis = 250; + +} // anonymous namespace + LibraryFeature::LibraryFeature( QObject *parent) : QObject(parent), - m_clickedChildActivationTimeoutMillis(0) { + m_clickedChildActivationTimeoutMillis(kDefaultClickedChildActivationTimeoutMillis) { } LibraryFeature::LibraryFeature( @@ -26,7 +35,7 @@ LibraryFeature::LibraryFeature( QObject* parent) : QObject(parent), m_pConfig(pConfig), - m_clickedChildActivationTimeoutMillis(0) { + m_clickedChildActivationTimeoutMillis(kDefaultClickedChildActivationTimeoutMillis) { } LibraryFeature::LibraryFeature( From c10c7be2890d0ede449ca32f25f6614e63db63af Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 17 Apr 2018 20:29:16 +0200 Subject: [PATCH 2/2] Distinguish between mouse and keyboard/encoder events in sidebar tree --- src/library/library.cpp | 2 + src/library/libraryfeature.cpp | 33 +--------------- src/library/libraryfeature.h | 15 +------ src/library/sidebarmodel.cpp | 90 +++++++++++++++++++++++++----------------- src/library/sidebarmodel.h | 13 +++--- 5 files changed, 65 insertions(+), 88 deletions(-) diff --git a/src/library/library.cpp b/src/library/library.cpp index 5a22fe9f84..20d447656c 100644 --- a/src/library/library.cpp +++ b/src/library/library.cpp @@ -210,6 +210,8 @@ void Library::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) { connect(m_pSidebarModel, SIGNAL(selectIndex(const QModelIndex&)), pSidebarWidget, SLOT(selectIndex(const QModelIndex&))); connect(pSidebarWidget, SIGNAL(pressed(const QModelIndex&)), + m_pSidebarModel, SLOT(pressed(const QModelIndex&))); + connect(pSidebarWidget, SIGNAL(clicked(const QModelIndex&)), m_pSidebarModel, SLOT(clicked(const QModelIndex&))); // Lazy model: Let triangle symbol increment the model connect(pSidebarWidget, SIGNAL(expanded(const QModelIndex&)), diff --git a/src/library/libraryfeature.cpp b/src/library/libraryfeature.cpp index 701e17a513..eb54b28ce7 100644 --- a/src/library/libraryfeature.cpp +++ b/src/library/libraryfeature.cpp @@ -7,45 +7,16 @@ // The reason for this is that LibraryFeature uses slots/signals and for this // to work the code has to be precompiles by moc -namespace { - -// The time between selecting and activating a feature item in the left -// pane. This is required to allow smooth and responsive scrolling through -// a list of items with an encoder! -const int kDefaultClickedChildActivationTimeoutMillis = 250; - -} // anonymous namespace - -LibraryFeature::LibraryFeature( - QObject *parent) - : QObject(parent), - m_clickedChildActivationTimeoutMillis(kDefaultClickedChildActivationTimeoutMillis) { -} - LibraryFeature::LibraryFeature( - int clickedChildActivationTimeoutMillis, QObject *parent) - : QObject(parent), - m_clickedChildActivationTimeoutMillis(clickedChildActivationTimeoutMillis) { - DEBUG_ASSERT(m_clickedChildActivationTimeoutMillis >= 0); -} - -LibraryFeature::LibraryFeature( - UserSettingsPointer pConfig, - QObject* parent) - : QObject(parent), - m_pConfig(pConfig), - m_clickedChildActivationTimeoutMillis(kDefaultClickedChildActivationTimeoutMillis) { + : QObject(parent) { } LibraryFeature::LibraryFeature( UserSettingsPointer pConfig, - int clickedChildActivationTimeoutMillis, QObject* parent) : QObject(parent), - m_pConfig(pConfig), - m_clickedChildActivationTimeoutMillis(clickedChildActivationTimeoutMillis) { - DEBUG_ASSERT(m_clickedChildActivationTimeoutMillis >= 0); + m_pConfig(pConfig) { } QStringList LibraryFeature::getPlaylistFiles(QFileDialog::FileMode mode) const { diff --git a/src/library/libraryfeature.h b/src/library/libraryfeature.h index 7639e03557..0fe348c89c 100644 --- a/src/library/libraryfeature.h +++ b/src/library/libraryfeature.h @@ -30,23 +30,12 @@ class LibraryFeature : public QObject { Q_OBJECT public: explicit LibraryFeature( - QObject* parent); - explicit LibraryFeature( - int clickedChildActivationTimeoutMillis, - QObject* parent = nullptr); - explicit LibraryFeature( - UserSettingsPointer pConfig, - QObject* parent = nullptr); + QObject* parent = nullptr); explicit LibraryFeature( UserSettingsPointer pConfig, - int clickedChildActivationTimeoutMillis, QObject* parent = nullptr); ~LibraryFeature() override = default; - int clickedChildActivationTimeoutMillis() const { - return m_clickedChildActivationTimeoutMillis; - } - virtual QVariant title() = 0; virtual QIcon getIcon() = 0; @@ -135,8 +124,6 @@ class LibraryFeature : public QObject { private: QStringList getPlaylistFiles(QFileDialog::FileMode mode) const; - - const int m_clickedChildActivationTimeoutMillis; }; #endif /* LIBRARYFEATURE_H */ diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index c2bb5d198b..22a93e2165 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -8,14 +8,23 @@ #include "library/browse/browsefeature.h" #include "util/assert.h" +namespace { + +// The time between selecting and activating (= clicking) a feature item +// in the sidebar tree. This is essential to allow smooth scrolling through +// a list of items with an encoder or the keyboard! A value of 300 ms has +// been chosen as a compromise between usability and responsiveness. +const int kPressedUntilClickedTimeoutMillis = 300; + +} // anonymous namespace + SidebarModel::SidebarModel( QObject* parent) : QAbstractItemModel(parent), m_iDefaultSelectedIndex(0), - m_clickedChildActivationTimer(new QTimer(this)), - m_clickedFeature(nullptr) { - m_clickedChildActivationTimer->setSingleShot(true); - connect(m_clickedChildActivationTimer, SIGNAL(timeout()), this, SLOT(slotActivateChildAtClickedFeatureIndex())); + m_pressedUntilClickedTimer(new QTimer(this)) { + m_pressedUntilClickedTimer->setSingleShot(true); + connect(m_pressedUntilClickedTimer, SIGNAL(timeout()), this, SLOT(slotPressedUntilClickedTimeout())); } void SidebarModel::addLibraryFeature(LibraryFeature* feature) { @@ -222,62 +231,72 @@ QVariant SidebarModel::data(const QModelIndex& index, int role) const { return QVariant(); } -void SidebarModel::onFeatureIndexClicked( - LibraryFeature* feature, - QModelIndex index) { - m_clickedChildActivationTimer->stop(); - m_clickedFeature = feature; - m_clickedIndex = index; +void SidebarModel::startPressedUntilClickedTimer(QModelIndex pressedIndex) { + m_pressedIndex = pressedIndex; + m_pressedUntilClickedTimer->start(kPressedUntilClickedTimeoutMillis); } -void SidebarModel::slotActivateChildAtClickedFeatureIndex() { - if (m_clickedFeature) { - m_clickedFeature->activateChild(m_clickedIndex); - } +void SidebarModel::stopPressedUntilClickedTimer() { + m_pressedUntilClickedTimer->stop(); + m_pressedIndex = QModelIndex(); } -void SidebarModel::clicked(const QModelIndex& index) { - //qDebug() << "SidebarModel::clicked() index=" << index; +void SidebarModel::slotPressedUntilClickedTimeout() { + if (m_pressedIndex.isValid()) { + QModelIndex clickedIndex = m_pressedIndex; + stopPressedUntilClickedTimer(); + clicked(clickedIndex); + } +} - // We use clicked() for keyboard and mouse control, and the - // following code breaks that for us: - /*if (QApplication::mouseButtons() != Qt::LeftButton) { - return; - }*/ +void SidebarModel::pressed(const QModelIndex& index) { + stopPressedUntilClickedTimer(); + if (index.isValid()) { + if (index.internalPointer() == this) { + m_sFeatures[index.row()]->activate(); + } else { + startPressedUntilClickedTimer(index); + } + } +} +void SidebarModel::clicked(const QModelIndex& index) { + // When triggered by a mouse event pressed() has been + // invoked immediately before. That doesn't matter, + // because we stop any running timer before handling + // this event. + stopPressedUntilClickedTimer(); if (index.isValid()) { if (index.internalPointer() == this) { m_sFeatures[index.row()]->activate(); } else { - TreeItem* tree_item = (TreeItem*)index.internalPointer(); + TreeItem* tree_item = static_cast(index.internalPointer()); if (tree_item) { - onFeatureIndexClicked(tree_item->feature(), index); - DEBUG_ASSERT(m_clickedFeature); - // Deferred activation is required for smooth scrolling when using - // encoder knobs - m_clickedChildActivationTimer->start( - m_clickedFeature->clickedChildActivationTimeoutMillis()); + LibraryFeature* feature = tree_item->feature(); + DEBUG_ASSERT(feature); + feature->activateChild(index); } } } } + void SidebarModel::doubleClicked(const QModelIndex& index) { + stopPressedUntilClickedTimer(); if (index.isValid()) { if (index.internalPointer() == this) { return; } else { TreeItem* tree_item = (TreeItem*)index.internalPointer(); if (tree_item) { - onFeatureIndexClicked(tree_item->feature(), index); - DEBUG_ASSERT(m_clickedFeature); - m_clickedFeature->onLazyChildExpandation(m_clickedIndex); + LibraryFeature* feature = tree_item->feature(); + feature->onLazyChildExpandation(index); } } } } void SidebarModel::rightClicked(const QPoint& globalPos, const QModelIndex& index) { - //qDebug() << "SidebarModel::rightClicked() index=" << index; + stopPressedUntilClickedTimer(); if (index.isValid()) { if (index.internalPointer() == this) { m_sFeatures[index.row()]->activate(); @@ -287,10 +306,9 @@ void SidebarModel::rightClicked(const QPoint& globalPos, const QModelIndex& inde { TreeItem* tree_item = (TreeItem*)index.internalPointer(); if (tree_item) { - onFeatureIndexClicked(tree_item->feature(), index); - DEBUG_ASSERT(m_clickedFeature); - m_clickedFeature->activateChild(m_clickedIndex); - m_clickedFeature->onRightClickChild(globalPos, m_clickedIndex); + LibraryFeature* feature = tree_item->feature(); + feature->activateChild(index); + feature->onRightClickChild(globalPos, index); } } } diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index 17c3bb8cae..3dc4891ed3 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -38,6 +38,7 @@ class SidebarModel : public QAbstractItemModel { bool hasTrackTable(const QModelIndex& index) const; public slots: + void pressed(const QModelIndex& index); void clicked(const QModelIndex& index); void doubleClicked(const QModelIndex& index); void rightClicked(const QPoint& globalPos, const QModelIndex& index); @@ -67,7 +68,7 @@ class SidebarModel : public QAbstractItemModel { void selectIndex(const QModelIndex& index); private slots: - void slotActivateChildAtClickedFeatureIndex(); + void slotPressedUntilClickedTimeout(); private: QModelIndex translateSourceIndex(const QModelIndex& parent); @@ -75,13 +76,11 @@ class SidebarModel : public QAbstractItemModel { QList m_sFeatures; unsigned int m_iDefaultSelectedIndex; /** Index of the item in the sidebar model to select at startup. */ - QTimer* const m_clickedChildActivationTimer; - LibraryFeature* m_clickedFeature; - QModelIndex m_clickedIndex; + QTimer* const m_pressedUntilClickedTimer; + QModelIndex m_pressedIndex; - void onFeatureIndexClicked( - LibraryFeature* feature, - QModelIndex index); + void startPressedUntilClickedTimer(QModelIndex pressedIndex); + void stopPressedUntilClickedTimer(); }; #endif /* SIDEBARMODEL_H */