Use collapsible box to configure visible roles

Summary:
A modal dialog that opens another modal dialog is bad UX and should be
avoided whenever possible. These days we have `KCollapsibleGroupBox`
that can be used to show advanced settings without cluttering the
default layout.

This change removes the `AdditionalInfoDialog` and uses a collapsible
groupbox instead. The `Apply` buttons gets enabled whenever a role gets
checked or unchecked, consistently with the existing checkboxes.

Test Plan: Change visible roles in the view properties dialog.

Reviewers: #dolphin, #vdg

Reviewed By: #vdg

Subscribers: ngraham, kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D13569

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index beaa0ec50059cc95ace982b974ea7ec958e5e79b..ea867de14fa27c6ccf781b41d81aca1a03738285 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -88,7 +88,6 @@ set(dolphinprivate_LIB_SRCS
     kitemviews/private/kitemlistviewanimation.cpp
     kitemviews/private/kitemlistviewlayouter.cpp
     kitemviews/private/kpixmapmodifier.cpp
     kitemviews/private/kitemlistviewanimation.cpp
     kitemviews/private/kitemlistviewlayouter.cpp
     kitemviews/private/kpixmapmodifier.cpp

-    settings/additionalinfodialog.cpp

     settings/applyviewpropsjob.cpp
     settings/viewmodes/viewmodesettings.cpp
     settings/viewpropertiesdialog.cpp
     settings/applyviewpropsjob.cpp
     settings/viewmodes/viewmodesettings.cpp
     settings/viewpropertiesdialog.cpp

diff --git a/src/settings/additionalinfodialog.cpp b/src/settings/additionalinfodialog.cpp
deleted file mode 100644 (file)
index 535aa94..0000000
--- a/src/settings/additionalinfodialog.cpp
+++ /dev/null
@@ -1,121 +0,0 @@
-/***************************************************************************
- *   Copyright (C) 2007-2012 by Peter Penz <peter.penz19@gmail.com>        *
- *                                                                         *
- *   This program is free software; you can redistribute it and/or modify  *
- *   it under the terms of the GNU General Public License as published by  *
- *   the Free Software Foundation; either version 2 of the License, or     *
- *   (at your option) any later version.                                   *
- *                                                                         *
- *   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 General Public License for more details.                          *
- *                                                                         *
- *   You should have received a copy of the GNU General Public License     *
- *   along with this program; if not, write to the                         *
- *   Free Software Foundation, Inc.,                                       *
- *   51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA          *
- ***************************************************************************/
-
-#include "additionalinfodialog.h"
-
-#include "kitemviews/kfileitemmodel.h"
-
-#include <KConfigGroup>
-#include <KLocalizedString>
-#include <KSharedConfig>
-#include <KWindowConfig>
-#include <config-baloo.h>
-
-#include <QCheckBox>
-#include <QDialogButtonBox>
-#include <QLabel>
-#include <QPushButton>
-#include <QVBoxLayout>
-
-#ifdef HAVE_BALOO
-    #include <Baloo/IndexerConfig>
-#endif
-
-AdditionalInfoDialog::AdditionalInfoDialog(QWidget* parent,
-                                           const QList<QByteArray>& visibleRoles) :
-    QDialog(parent),
-    m_visibleRoles(visibleRoles),
-    m_listWidget(nullptr)
-{
-    setWindowTitle(i18nc("@title:window", "Additional Information"));
-    setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Minimum);
-
-    auto layout = new QVBoxLayout(this);
-    setLayout(layout);
-
-    // Add header
-    auto header = new QLabel(this);
-    header->setText(i18nc("@label", "Select which additional information should be shown:"));
-    header->setWordWrap(true);
-    layout->addWidget(header);
-
-    // Add checkboxes
-    bool indexingEnabled = false;
-#ifdef HAVE_BALOO
-    Baloo::IndexerConfig config;
-    indexingEnabled = config.fileIndexingEnabled();
-#endif
-
-    m_listWidget = new QListWidget(this);
-    m_listWidget->setSelectionMode(QAbstractItemView::NoSelection);
-    const QList<KFileItemModel::RoleInfo> rolesInfo = KFileItemModel::rolesInformation();
-    foreach (const KFileItemModel::RoleInfo& info, rolesInfo) {
-        QListWidgetItem* item = new QListWidgetItem(info.translation, m_listWidget);
-        item->setCheckState(visibleRoles.contains(info.role) ? Qt::Checked : Qt::Unchecked);
-
-        const bool enable = ((!info.requiresBaloo && !info.requiresIndexer) ||
-                            (info.requiresBaloo) ||
-                            (info.requiresIndexer && indexingEnabled)) && info.role != "text";
-
-        if (!enable) {
-            item->setFlags(item->flags() & ~Qt::ItemIsEnabled);
-        }
-    }
-    layout->addWidget(m_listWidget);
-
-    auto buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this);
-    connect(buttonBox, &QDialogButtonBox::accepted, this, &AdditionalInfoDialog::accept);
-    connect(buttonBox, &QDialogButtonBox::rejected, this, &AdditionalInfoDialog::reject);
-    layout->addWidget(buttonBox);
-
-    auto okButton = buttonBox->button(QDialogButtonBox::Ok);
-    okButton->setShortcut(Qt::CTRL + Qt::Key_Return);
-    okButton->setDefault(true);
-
-    const KConfigGroup dialogConfig(KSharedConfig::openConfig(QStringLiteral("dolphinrc")), "AdditionalInfoDialog");
-    KWindowConfig::restoreWindowSize(windowHandle(), dialogConfig);
-}
-
-AdditionalInfoDialog::~AdditionalInfoDialog()
-{
-    KConfigGroup dialogConfig(KSharedConfig::openConfig(QStringLiteral("dolphinrc")), "AdditionalInfoDialog");
-    KWindowConfig::saveWindowSize(windowHandle(), dialogConfig);
-}
-
-QList<QByteArray> AdditionalInfoDialog::visibleRoles() const
-{
-    return m_visibleRoles;
-}
-
-void AdditionalInfoDialog::accept()
-{
-    m_visibleRoles.clear();
-
-    int index = 0;
-    const QList<KFileItemModel::RoleInfo> rolesInfo = KFileItemModel::rolesInformation();
-    foreach (const KFileItemModel::RoleInfo& info, rolesInfo) {
-        const QListWidgetItem* item = m_listWidget->item(index);
-        if (item->checkState() == Qt::Checked) {
-            m_visibleRoles.append(info.role);
-        }
-        ++index;
-    }
-
-    QDialog::accept();
-}

diff --git a/src/settings/additionalinfodialog.h b/src/settings/additionalinfodialog.h
deleted file mode 100644 (file)
index d6bf1e0..0000000
--- a/src/settings/additionalinfodialog.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/***************************************************************************
- *   Copyright (C) 2007-2012 by Peter Penz <peter.penz19@gmail.com>        *
- *                                                                         *
- *   This program is free software; you can redistribute it and/or modify  *
- *   it under the terms of the GNU General Public License as published by  *
- *   the Free Software Foundation; either version 2 of the License, or     *
- *   (at your option) any later version.                                   *
- *                                                                         *
- *   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 General Public License for more details.                          *
- *                                                                         *
- *   You should have received a copy of the GNU General Public License     *
- *   along with this program; if not, write to the                         *
- *   Free Software Foundation, Inc.,                                       *
- *   51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA          *
- ***************************************************************************/
-
-#ifndef ADDITIONALINFODIALOG_H
-#define ADDITIONALINFODIALOG_H
-
-#include <QDialog>
-#include <QList>
-#include <QListWidget>
-
-/**
- * @brief Dialog for changing the additional information shown in the view.
- */
-class AdditionalInfoDialog : public QDialog
-{
-    Q_OBJECT
-
-public:
-    AdditionalInfoDialog(QWidget* parent, const QList<QByteArray>& visibleRoles);
-    ~AdditionalInfoDialog() override;
-    QList<QByteArray> visibleRoles() const;
-
-public slots:
-    void accept() override;
-
-private:
-    QList<QByteArray> m_visibleRoles;
-    QListWidget* m_listWidget;
-};
-
-#endif

diff --git a/src/settings/viewpropertiesdialog.cpp b/src/settings/viewpropertiesdialog.cpp
index 60c9f21f56160659a8fd17a25c32d22713812ccf..88dea041a8c0004816c5f06e9b559a4b91803869 100644 (file)
--- a/src/settings/viewpropertiesdialog.cpp
+++ b/src/settings/viewpropertiesdialog.cpp
@@ -1,6 +1,6 @@
 /***************************************************************************
 /***************************************************************************

- *   Copyright (C) 2006 by Peter Penz                                      *
- *   peter.penz@gmx.at                                                     *
+ *   Copyright (C) 2006 by Peter Penz <peter.penz@gmx.at>                  *
+ *   Copyright (C) 2018 by Elvis Angelaccio <elvis.angelaccio@kde.org>     *

  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *


@@ -20,23 +20,28 @@
 
 #include "viewpropertiesdialog.h"
 
 
 #include "viewpropertiesdialog.h"
 

-#include "additionalinfodialog.h"

 #include "dolphin_generalsettings.h"
 #include "dolphin_iconsmodesettings.h"
 #include "kitemviews/kfileitemmodel.h"
 #include "viewpropsprogressinfo.h"
 #include "views/dolphinview.h"
 
 #include "dolphin_generalsettings.h"
 #include "dolphin_iconsmodesettings.h"
 #include "kitemviews/kfileitemmodel.h"
 #include "viewpropsprogressinfo.h"
 #include "views/dolphinview.h"
 

+#include <KCollapsibleGroupBox>

 #include <KComboBox>
 #include <KLocalizedString>
 #include <KMessageBox>
 #include <KWindowConfig>
 
 #include <KComboBox>
 #include <KLocalizedString>
 #include <KMessageBox>
 #include <KWindowConfig>
 

+#ifdef HAVE_BALOO
+    #include <Baloo/IndexerConfig>
+#endif
+

 #include <QButtonGroup>
 #include <QCheckBox>
 #include <QGridLayout>
 #include <QGroupBox>
 #include <QLabel>
 #include <QButtonGroup>
 #include <QCheckBox>
 #include <QGridLayout>
 #include <QGroupBox>
 #include <QLabel>

+#include <QListWidget>

 #include <QPushButton>
 #include <QRadioButton>
 
 #include <QPushButton>
 #include <QRadioButton>
 


@@ -54,7 +59,6 @@ ViewPropertiesDialog::ViewPropertiesDialog(DolphinView* dolphinView) :
     m_previewsShown(nullptr),
     m_showInGroups(nullptr),
     m_showHiddenFiles(nullptr),
     m_previewsShown(nullptr),
     m_showInGroups(nullptr),
     m_showHiddenFiles(nullptr),

-    m_additionalInfo(nullptr),

     m_applyToCurrentFolder(nullptr),
     m_applyToSubFolders(nullptr),
     m_applyToAllFolders(nullptr),
     m_applyToCurrentFolder(nullptr),
     m_applyToSubFolders(nullptr),
     m_applyToAllFolders(nullptr),


@@ -64,13 +68,14 @@ ViewPropertiesDialog::ViewPropertiesDialog(DolphinView* dolphinView) :
     const bool useGlobalViewProps = GeneralSettings::globalViewProps();
 
     setWindowTitle(i18nc("@title:window", "View Properties"));
     const bool useGlobalViewProps = GeneralSettings::globalViewProps();
 
     setWindowTitle(i18nc("@title:window", "View Properties"));

-    setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Minimum);

 
     const QUrl& url = dolphinView->url();
     m_viewProps = new ViewProperties(url);
     m_viewProps->setAutoSaveEnabled(false);
 
     auto layout = new QVBoxLayout(this);
 
     const QUrl& url = dolphinView->url();
     m_viewProps = new ViewProperties(url);
     m_viewProps->setAutoSaveEnabled(false);
 
     auto layout = new QVBoxLayout(this);

+    // Otherwise the dialog won't resize when we collapse the KCollapsibleGroupBox.
+    layout->setSizeConstraint(QLayout::SetFixedSize);

     setLayout(layout);
 
     auto propsGrid = new QWidget(this);
     setLayout(layout);
 
     auto propsGrid = new QWidget(this);


@@ -107,7 +112,51 @@ ViewPropertiesDialog::ViewPropertiesDialog(DolphinView* dolphinView) :
     m_showInGroups = new QCheckBox(i18nc("@option:check", "Show in groups"));
     m_showHiddenFiles = new QCheckBox(i18nc("@option:check", "Show hidden files"));
 
     m_showInGroups = new QCheckBox(i18nc("@option:check", "Show in groups"));
     m_showHiddenFiles = new QCheckBox(i18nc("@option:check", "Show hidden files"));
 

-    m_additionalInfo = new QPushButton(i18nc("@action:button", "Additional Information"));
+    auto additionalInfoBox = new KCollapsibleGroupBox();
+    additionalInfoBox->setTitle(i18nc("@title:group", "Additional Information Shown"));
+    auto innerLayout = new QVBoxLayout();
+
+    {
+        QList<QByteArray> visibleRoles = m_viewProps->visibleRoles();
+        const bool useDefaultRoles = (m_viewProps->viewMode() == DolphinView::DetailsView) && visibleRoles.isEmpty();
+        if (useDefaultRoles) {
+            // Using the details view without any additional information (-> additional column)
+            // makes no sense and leads to a usability problem as no viewport area is available
+            // anymore. Hence as fallback provide at least a size and date column.
+            visibleRoles.clear();
+            visibleRoles.append("text");
+            visibleRoles.append("size");
+            visibleRoles.append("modificationtime");
+            m_viewProps->setVisibleRoles(visibleRoles);
+        }
+
+        // Add checkboxes
+        bool indexingEnabled = false;
+#ifdef HAVE_BALOO
+        Baloo::IndexerConfig config;
+        indexingEnabled = config.fileIndexingEnabled();
+#endif
+
+        m_listWidget = new QListWidget();
+        connect(m_listWidget, &QListWidget::itemChanged, this, &ViewPropertiesDialog::slotItemChanged);
+        m_listWidget->setSelectionMode(QAbstractItemView::NoSelection);
+        const QList<KFileItemModel::RoleInfo> rolesInfo = KFileItemModel::rolesInformation();
+        foreach (const KFileItemModel::RoleInfo& info, rolesInfo) {
+            QListWidgetItem* item = new QListWidgetItem(info.translation, m_listWidget);
+            item->setCheckState(visibleRoles.contains(info.role) ? Qt::Checked : Qt::Unchecked);
+
+            const bool enable = ((!info.requiresBaloo && !info.requiresIndexer) ||
+                                (info.requiresBaloo) ||
+                                (info.requiresIndexer && indexingEnabled)) && info.role != "text";
+
+            if (!enable) {
+                item->setFlags(item->flags() & ~Qt::ItemIsEnabled);
+            }
+        }
+        innerLayout->addWidget(m_listWidget);
+    }
+
+    additionalInfoBox->setLayout(innerLayout);

 
     QHBoxLayout* sortingLayout = new QHBoxLayout();
     sortingLayout->setMargin(0);
 
     QHBoxLayout* sortingLayout = new QHBoxLayout();
     sortingLayout->setMargin(0);


@@ -127,7 +176,7 @@ ViewPropertiesDialog::ViewPropertiesDialog(DolphinView* dolphinView) :
     propsBoxLayout->addWidget(m_previewsShown);
     propsBoxLayout->addWidget(m_showInGroups);
     propsBoxLayout->addWidget(m_showHiddenFiles);
     propsBoxLayout->addWidget(m_previewsShown);
     propsBoxLayout->addWidget(m_showInGroups);
     propsBoxLayout->addWidget(m_showHiddenFiles);

-    propsBoxLayout->addWidget(m_additionalInfo);
+    propsBoxLayout->addWidget(additionalInfoBox);

 
     connect(m_viewMode, static_cast<void(KComboBox::*)(int)>(&KComboBox::currentIndexChanged),
             this, &ViewPropertiesDialog::slotViewModeChanged);
 
     connect(m_viewMode, static_cast<void(KComboBox::*)(int)>(&KComboBox::currentIndexChanged),
             this, &ViewPropertiesDialog::slotViewModeChanged);


@@ -135,8 +184,6 @@ ViewPropertiesDialog::ViewPropertiesDialog(DolphinView* dolphinView) :
             this, &ViewPropertiesDialog::slotSortingChanged);
     connect(m_sortOrder, static_cast<void(KComboBox::*)(int)>(&KComboBox::currentIndexChanged),
             this, &ViewPropertiesDialog::slotSortOrderChanged);
             this, &ViewPropertiesDialog::slotSortingChanged);
     connect(m_sortOrder, static_cast<void(KComboBox::*)(int)>(&KComboBox::currentIndexChanged),
             this, &ViewPropertiesDialog::slotSortOrderChanged);

-    connect(m_additionalInfo, &QPushButton::clicked,
-            this, &ViewPropertiesDialog::configureAdditionalInfo);

     connect(m_sortFoldersFirst, &QCheckBox::clicked,
             this, &ViewPropertiesDialog::slotSortFoldersFirstChanged);
     connect(m_previewsShown, &QCheckBox::clicked,
     connect(m_sortFoldersFirst, &QCheckBox::clicked,
             this, &ViewPropertiesDialog::slotSortFoldersFirstChanged);
     connect(m_previewsShown, &QCheckBox::clicked,


@@ -279,6 +326,12 @@ void ViewPropertiesDialog::slotShowHiddenFilesChanged()
     markAsDirty(true);
 }
 
     markAsDirty(true);
 }
 

+void ViewPropertiesDialog::slotItemChanged(QListWidgetItem *item)
+{
+    Q_UNUSED(item)
+    markAsDirty(true);
+}
+

 void ViewPropertiesDialog::markAsDirty(bool isDirty)
 {
     if (m_isDirty != isDirty) {
 void ViewPropertiesDialog::markAsDirty(bool isDirty)
 {
     if (m_isDirty != isDirty) {


@@ -287,29 +340,6 @@ void ViewPropertiesDialog::markAsDirty(bool isDirty)
     }
 }
 
     }
 }
 

-void ViewPropertiesDialog::configureAdditionalInfo()
-{
-    QList<QByteArray> visibleRoles = m_viewProps->visibleRoles();
-    const bool useDefaultRoles = (m_viewProps->viewMode() == DolphinView::DetailsView) && visibleRoles.isEmpty();
-    if (useDefaultRoles) {
-        // Using the details view without any additional information (-> additional column)
-        // makes no sense and leads to a usability problem as no viewport area is available
-        // anymore. Hence as fallback provide at least a size and date column.
-        visibleRoles.clear();
-        visibleRoles.append("text");
-        visibleRoles.append("size");
-        visibleRoles.append("modificationtime");
-        m_viewProps->setVisibleRoles(visibleRoles);
-    }
-
-    QPointer<AdditionalInfoDialog> dialog = new AdditionalInfoDialog(this, visibleRoles);
-    if (dialog->exec() == QDialog::Accepted) {
-        m_viewProps->setVisibleRoles(dialog->visibleRoles());
-        markAsDirty(true);
-    }
-    delete dialog;
-}
-

 void ViewPropertiesDialog::applyViewProperties()
 {
     // if nothing changed in the dialog, we have nothing to apply
 void ViewPropertiesDialog::applyViewProperties()
 {
     // if nothing changed in the dialog, we have nothing to apply


@@ -317,6 +347,22 @@ void ViewPropertiesDialog::applyViewProperties()
         return;
     }
 
         return;
     }
 

+    // Update visible roles.
+    {
+        QList<QByteArray> visibleRoles;
+        int index = 0;
+        const QList<KFileItemModel::RoleInfo> rolesInfo = KFileItemModel::rolesInformation();
+        foreach (const KFileItemModel::RoleInfo& info, rolesInfo) {
+            const QListWidgetItem* item = m_listWidget->item(index);
+             if (item->checkState() == Qt::Checked) {
+                visibleRoles.append(info.role);
+            }
+            ++index;
+        }
+
+        m_viewProps->setVisibleRoles(visibleRoles);
+    }
+

     const bool applyToSubFolders = m_applyToSubFolders && m_applyToSubFolders->isChecked();
     if (applyToSubFolders) {
         const QString text(i18nc("@info", "The view properties of all sub-folders will be changed. Do you want to continue?"));
     const bool applyToSubFolders = m_applyToSubFolders && m_applyToSubFolders->isChecked();
     if (applyToSubFolders) {
         const QString text(i18nc("@info", "The view properties of all sub-folders will be changed. Do you want to continue?"));

diff --git a/src/settings/viewpropertiesdialog.h b/src/settings/viewpropertiesdialog.h
index 35c18718e1d5e48d4d35502dffda8daff9e990bf..80cddbf4eeb148f175a0c1c90db7f1e07436dff6 100644 (file)
--- a/src/settings/viewpropertiesdialog.h
+++ b/src/settings/viewpropertiesdialog.h
@@ -1,6 +1,6 @@
 /***************************************************************************
 /***************************************************************************

- *   Copyright (C) 2006 by Peter Penz                                      *
- *   peter.penz@gmx.at                                                     *
+ *   Copyright (C) 2006 by Peter Penz <peter.penz@gmx.at>                  *
+ *   Copyright (C) 2018 by Elvis Angelaccio <elvis.angelaccio@kde.org>     *

  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *


@@ -26,6 +26,8 @@
 #include <QDialog>
 
 class QCheckBox;
 #include <QDialog>
 
 class QCheckBox;

+class QListWidget;
+class QListWidgetItem;

 class KComboBox;
 class QPushButton;
 class QRadioButton;
 class KComboBox;
 class QPushButton;
 class QRadioButton;


@@ -59,8 +61,8 @@ private slots:
     void slotSortFoldersFirstChanged();
     void slotShowPreviewChanged();
     void slotShowHiddenFilesChanged();
     void slotSortFoldersFirstChanged();
     void slotShowPreviewChanged();
     void slotShowHiddenFilesChanged();

+    void slotItemChanged(QListWidgetItem *item);

     void markAsDirty(bool isDirty);
     void markAsDirty(bool isDirty);

-    void configureAdditionalInfo();

 
 signals:
     void isDirtyChanged(bool isDirty);
 
 signals:
     void isDirtyChanged(bool isDirty);


@@ -81,11 +83,11 @@ private:
     QCheckBox* m_previewsShown;
     QCheckBox* m_showInGroups;
     QCheckBox* m_showHiddenFiles;
     QCheckBox* m_previewsShown;
     QCheckBox* m_showInGroups;
     QCheckBox* m_showHiddenFiles;

-    QPushButton* m_additionalInfo;

     QRadioButton* m_applyToCurrentFolder;
     QRadioButton* m_applyToSubFolders;
     QRadioButton* m_applyToAllFolders;
     QCheckBox* m_useAsDefault;
     QRadioButton* m_applyToCurrentFolder;
     QRadioButton* m_applyToSubFolders;
     QRadioButton* m_applyToAllFolders;
     QCheckBox* m_useAsDefault;

+    QListWidget* m_listWidget;

 };
 
 #endif
 };
 
 #endif