From 9478f1473006147c643c228793822b2eeb4b0606 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Wed, 19 Oct 2022 18:03:12 +0200 Subject: [PATCH] Make details view mode's full row activation optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In d3839617193e92463806580699caa595c892b8a6 the details view mode was changed in a way that made the full row of an item the click target instead of only having the item's icon and text be the representative clickable area of an item. This commit makes this new behaviour optional through a setting which can be changed in Dolphin's settings dialog. The explanation for introducing yet another setting in this case is as follows: While the introduced change is an improvement for many typical workflows, there are some workflows for which this new behaviour is problematic. Quite prominently a usage of Dolphin that tries to maximise information density is made worse by the change because now side padding is necessary to click the view's background. While the side padding is and was optional, disabling it made switching the active view in split view mode more difficult among other things. For a more complete discussion about the issues, please check out the bug report(s) and the discussion in Dolphin's gitlab issue with number 34. Co-authored-by: Ivan Čukić BUG: 453700 FIXED-IN: 22.12 --- src/kitemviews/kstandarditemlistview.cpp | 7 +++++- src/kitemviews/kstandarditemlistview.h | 2 ++ src/kitemviews/kstandarditemlistwidget.cpp | 1 + src/settings/dolphin_detailsmodesettings.kcfg | 4 ++++ src/settings/viewmodes/viewsettingstab.cpp | 22 +++++++++++++++++++ src/settings/viewmodes/viewsettingstab.h | 1 + src/views/dolphinitemlistview.cpp | 7 +++++- src/views/dolphinitemlistview.h | 3 +++ 8 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/kitemviews/kstandarditemlistview.cpp b/src/kitemviews/kstandarditemlistview.cpp index 4b7c2d9a42..0ba23e654d 100644 --- a/src/kitemviews/kstandarditemlistview.cpp +++ b/src/kitemviews/kstandarditemlistview.cpp @@ -36,7 +36,7 @@ void KStandardItemListView::setItemLayout(ItemLayout layout) m_itemLayout = layout; // keep the leading padding option unchanged here - setHighlightEntireRow(layout == DetailsLayout); + setHighlightEntireRow(itemLayoutHighlightEntireRow(layout)); setSupportsItemExpanding(itemLayoutSupportsItemExpanding(layout)); setScrollOrientation(layout == CompactLayout ? Qt::Horizontal : Qt::Vertical); @@ -93,6 +93,11 @@ bool KStandardItemListView::itemSizeHintUpdateRequired(const QSet& c return false; } +bool KStandardItemListView::itemLayoutHighlightEntireRow(ItemLayout layout) const +{ + return layout == DetailsLayout; +} + bool KStandardItemListView::itemLayoutSupportsItemExpanding(ItemLayout layout) const { return layout == DetailsLayout; diff --git a/src/kitemviews/kstandarditemlistview.h b/src/kitemviews/kstandarditemlistview.h index 7f0550ec36..527d14b9d1 100644 --- a/src/kitemviews/kstandarditemlistview.h +++ b/src/kitemviews/kstandarditemlistview.h @@ -50,6 +50,8 @@ protected: void initializeItemListWidget(KItemListWidget* item) override; bool itemSizeHintUpdateRequired(const QSet& changedRoles) const override; virtual bool itemLayoutSupportsItemExpanding(ItemLayout layout) const; + /** To be overriden by sub-classes to specify when full row highlighting should be enabled. */ + virtual bool itemLayoutHighlightEntireRow(ItemLayout layout) const; virtual void onItemLayoutChanged(ItemLayout current, ItemLayout previous); void onScrollOrientationChanged(Qt::Orientation current, Qt::Orientation previous) override; void onSupportsItemExpandingChanged(bool supportsExpanding) override; diff --git a/src/kitemviews/kstandarditemlistwidget.cpp b/src/kitemviews/kstandarditemlistwidget.cpp index 9a2939b23d..67afbb248f 100644 --- a/src/kitemviews/kstandarditemlistwidget.cpp +++ b/src/kitemviews/kstandarditemlistwidget.cpp @@ -241,6 +241,7 @@ KStandardItemListWidget::KStandardItemListWidget(KItemListWidgetInformant* infor m_customizedFont(), m_customizedFontMetrics(m_customizedFont), m_isExpandable(false), + m_highlightEntireRow(false), m_supportsItemExpanding(false), m_dirtyLayout(true), m_dirtyContent(true), diff --git a/src/settings/dolphin_detailsmodesettings.kcfg b/src/settings/dolphin_detailsmodesettings.kcfg index a58a8d00b0..aad59743c7 100644 --- a/src/settings/dolphin_detailsmodesettings.kcfg +++ b/src/settings/dolphin_detailsmodesettings.kcfg @@ -31,6 +31,10 @@ 20 + + + true + true diff --git a/src/settings/viewmodes/viewsettingstab.cpp b/src/settings/viewmodes/viewsettingstab.cpp index cc6818a301..850004c6f3 100644 --- a/src/settings/viewmodes/viewsettingstab.cpp +++ b/src/settings/viewmodes/viewsettingstab.cpp @@ -101,6 +101,10 @@ ViewSettingsTab::ViewSettingsTab(Mode mode, QWidget* parent) : m_expandableFolders = new QCheckBox(i18nc("@option:check", "Expandable")); topLayout->addRow(i18nc("@label:checkbox", "Folders:"), m_expandableFolders); + m_highlightEntireRow = new QCheckBox(i18nc("@option:check", "Highlight entire row")); + topLayout->addRow(i18nc("@label:checkbox", "Selection effect:"), m_highlightEntireRow); + + #ifndef Q_OS_WIN // Sorting properties m_numberOfItems = new QRadioButton(i18nc("option:radio", "Number of items")); @@ -161,6 +165,7 @@ ViewSettingsTab::ViewSettingsTab(Mode mode, QWidget* parent) : connect(m_widthBox, &QComboBox::currentIndexChanged, this, &ViewSettingsTab::changed); break; case DetailsMode: + connect(m_highlightEntireRow, &QCheckBox::toggled, this, &ViewSettingsTab::changed); connect(m_expandableFolders, &QCheckBox::toggled, this, &ViewSettingsTab::changed); #ifndef Q_OS_WIN connect(m_recursiveDirectorySizeLimit, &QSpinBox::valueChanged, this, &ViewSettingsTab::changed); @@ -195,6 +200,22 @@ void ViewSettingsTab::applySettings() CompactModeSettings::setMaximumTextWidthIndex(m_widthBox->currentIndex()); break; case DetailsMode: + // We need side-padding when the full row is a click target to still be able to not click items. + // So here the default padding is enabled when the full row highlight is enabled. + if (m_highlightEntireRow->isChecked() && !DetailsModeSettings::highlightEntireRow()) { + auto detailsModeSettings = DetailsModeSettings::self(); + const bool usedDefaults = detailsModeSettings->useDefaults(true); + const int defaultSidePadding = detailsModeSettings->sidePadding(); + detailsModeSettings->useDefaults(usedDefaults); + if (DetailsModeSettings::sidePadding() < defaultSidePadding) { + DetailsModeSettings::setSidePadding(defaultSidePadding); + } + } else if (!m_highlightEntireRow->isChecked() && DetailsModeSettings::highlightEntireRow()) { + // The full row click target is disabled so now most of the view area can be used to interact + // with the view background. Having an extra side padding has no usability benefit in this case. + DetailsModeSettings::setSidePadding(0); + } + DetailsModeSettings::setHighlightEntireRow(m_highlightEntireRow->isChecked()); DetailsModeSettings::setExpandableFolders(m_expandableFolders->isChecked()); #ifndef Q_OS_WIN DetailsModeSettings::setDirectorySizeCount(m_numberOfItems->isChecked()); @@ -238,6 +259,7 @@ void ViewSettingsTab::loadSettings() m_widthBox->setCurrentIndex(CompactModeSettings::maximumTextWidthIndex()); break; case DetailsMode: + m_highlightEntireRow->setChecked(DetailsModeSettings::highlightEntireRow()); m_expandableFolders->setChecked(DetailsModeSettings::expandableFolders()); #ifndef Q_OS_WIN if (DetailsModeSettings::directorySizeCount()) { diff --git a/src/settings/viewmodes/viewsettingstab.h b/src/settings/viewmodes/viewsettingstab.h index 36dde0583f..2cc133b524 100644 --- a/src/settings/viewmodes/viewsettingstab.h +++ b/src/settings/viewmodes/viewsettingstab.h @@ -56,6 +56,7 @@ private: DolphinFontRequester* m_fontRequester; QComboBox* m_widthBox; QComboBox* m_maxLinesBox; + QCheckBox* m_highlightEntireRow; QCheckBox* m_expandableFolders; QRadioButton* m_numberOfItems; QRadioButton* m_sizeOfContents; diff --git a/src/views/dolphinitemlistview.cpp b/src/views/dolphinitemlistview.cpp index cd6dbd0a73..60d5577b83 100644 --- a/src/views/dolphinitemlistview.cpp +++ b/src/views/dolphinitemlistview.cpp @@ -83,7 +83,7 @@ void DolphinItemListView::readSettings() beginTransaction(); setEnabledSelectionToggles(m_selectionTogglesEnabled); - setHighlightEntireRow(DetailsModeSettings::sidePadding()); + setHighlightEntireRow(itemLayoutHighlightEntireRow(itemLayout())); setSupportsItemExpanding(itemLayoutSupportsItemExpanding(itemLayout())); updateFont(); @@ -107,6 +107,11 @@ KItemListWidgetCreatorBase* DolphinItemListView::defaultWidgetCreator() const return new KItemListWidgetCreator(); } +bool DolphinItemListView::itemLayoutHighlightEntireRow(ItemLayout layout) const +{ + return layout == DetailsLayout && DetailsModeSettings::highlightEntireRow(); +} + bool DolphinItemListView::itemLayoutSupportsItemExpanding(ItemLayout layout) const { return layout == DetailsLayout && DetailsModeSettings::expandableFolders(); diff --git a/src/views/dolphinitemlistview.h b/src/views/dolphinitemlistview.h index 464aec1b4c..25476290f6 100644 --- a/src/views/dolphinitemlistview.h +++ b/src/views/dolphinitemlistview.h @@ -47,6 +47,9 @@ public: protected: KItemListWidgetCreatorBase* defaultWidgetCreator() const override; + /** Overwriting in the Dolphin-specific class because we want this to be user-configurable. + * @see KStandardItemListView::itemLayoutHighlightEntireRow */ + bool itemLayoutHighlightEntireRow(ItemLayout layout) const override; bool itemLayoutSupportsItemExpanding(ItemLayout layout) const override; void onItemLayoutChanged(ItemLayout current, ItemLayout previous) override; void onPreviewsShownChanged(bool shown) override;