Address Nate's UX feedback

-More labels
-More icons
-"Abort" -> "Cancel"

src/selectionmode/bottombarcontentscontainer.cpp

@@ -157,7 +157,7 @@ void BottomBarContentsContainer::addCopyContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to copy files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort Copying"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Copying"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -190,7 +190,7 @@ void BottomBarContentsContainer::addCopyLocationContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to copy the location of files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort Copying"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Copying"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -209,7 +209,7 @@ void BottomBarContentsContainer::addCopyToOtherViewContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to copy the location of files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort Copying"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Copying"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -227,7 +227,7 @@ void BottomBarContentsContainer::addCutContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to cut files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort Cutting"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Cutting"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -260,7 +260,7 @@ void BottomBarContentsContainer::addDeleteContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to delete files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -278,7 +278,7 @@ void BottomBarContentsContainer::addDuplicateContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to duplicate files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort Duplicating"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Duplicating"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -291,15 +291,10 @@ void BottomBarContentsContainer::addDuplicateContents()
 void BottomBarContentsContainer::addGeneralContents()
 {
     if (!m_overflowButton) {
-        m_overflowButton = new QToolButton{this};
         // i18n: This button appears in a bar if there isn't enough horizontal space to fit all the other buttons.
-        // The small icon-only button opens a menu that contains the actions that didn't fit on the bar.
-        // Since this is an icon-only button this text will only appear as a tooltip and as accessibility text.
-        m_overflowButton->setToolTip(i18nc("@action", "More"));
-        m_overflowButton->setAccessibleName(m_overflowButton->toolTip());
-        m_overflowButton->setIcon(QIcon::fromTheme(QStringLiteral("view-more-horizontal-symbolic")));
+        // The small button opens a menu that contains the actions that didn't fit on the bar.
+        m_overflowButton = new QPushButton{QIcon::fromTheme(QStringLiteral("view-more-symbolic")), i18nc("@action", "More"), this};
         m_overflowButton->setMenu(new QMenu{m_overflowButton});
-        m_overflowButton->setPopupMode(QToolButton::ToolButtonPopupMode::InstantPopup);
         m_overflowButton->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::MinimumExpanding); // Makes sure it has the same height as the labeled buttons.
         m_layout->addWidget(m_overflowButton);
     } else {
@@ -348,7 +343,7 @@ void BottomBarContentsContainer::addMoveToOtherViewContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to copy the location of files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort Moving"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Moving"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -366,7 +361,7 @@ void BottomBarContentsContainer::addMoveToTrashContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process of moving files to the trash by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Abort"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 
@@ -427,7 +422,7 @@ void BottomBarContentsContainer::addRenameContents()
     m_layout->addWidget(m_explanatoryLabel);
 
     // i18n: Aborts the current step-by-step process to delete files by leaving the selection mode.
-    auto *cancelButton = new QPushButton(i18nc("@action:button", "Stop Renaming"), this);
+    auto *cancelButton = new QPushButton(QIcon::fromTheme(QStringLiteral("dialog-cancel")), i18nc("@action:button", "Cancel Renaming"), this);
     connect(cancelButton, &QAbstractButton::clicked, this, &BottomBarContentsContainer::selectionModeLeavingRequested);
     m_layout->addWidget(cancelButton);
 

src/selectionmode/bottombarcontentscontainer.h

@@ -12,7 +12,7 @@
 #include "bottombar.h"
 
 #include <QPointer>
-#include <QToolButton>
+#include <QPushButton>
 #include <QWidget>
 
 class DolphinContextMenu;
@@ -144,7 +144,7 @@ private:
     /// @see updateMainActionButtonText
     ActionWithWidget m_mainAction = ActionWithWidget(nullptr);
     /// The button containing all the actions that don't currently fit into the bar.
-    QPointer<QToolButton> m_overflowButton;
+    QPointer<QPushButton> m_overflowButton;
     /// The actionCollection from which the actions for this bar are retrieved.
     KActionCollection *m_actionCollection;
     /// Describes the current contents of the bar.

src/selectionmode/topbar.cpp

@@ -66,8 +66,7 @@ TopBar::TopBar(QWidget *parent) :
     BackgroundColorHelper::instance()->controlBackgroundColor(m_label);
 
     m_closeButton = new QPushButton(QIcon::fromTheme(QStringLiteral("window-close-symbolic")), "", contentsContainer);
-    m_closeButton->setToolTip(i18nc("@action:button", "Exit Selection Mode"));
-    m_closeButton->setAccessibleName(m_closeButton->toolTip());
+    m_closeButton->setText(i18nc("@action:button", "Exit Selection Mode"));
     m_closeButton->setFlat(true);
     connect(m_closeButton, &QAbstractButton::pressed,
             this, &TopBar::selectionModeLeavingRequested);

[Mark Gaiser]

I could just be "suffering" form muscle memory and therefore not really liking this addition. What i used to do when permanently deleting files was:

• Select files
• Shift + delete
• Enter

And gone 🙂

After this change that flow changed to:

• Select files
• Shift + delete
• Arrow left (to change the selected button from cancel to delete permanently)
• Enter

I get that permanent deletion requires some "manual action" to be sure what you're doing isn't accidental. But now there is yet another manual action (after pressing Shift + delete the Enter key was already the extra step). This is imho one manual step too much.

It can probably be easily fixed (haven't tried) by giving the default focus to the delete button instead of the cancel button.

[Mark Gaiser]

cc @ngraham @felixernst

[Nate Graham]

The change in question was unintentional and has no relationship to this commit. See https://bugs.kde.org/show_bug.cgi?id=462845

[Mark Gaiser]

Ahh sorry for the noise! I followed the code back to this commit and didn't thing to look further.

Glad it's on the radar already!

[Nate Graham]

You're welcome, and sorry for the crabby response. A lot of noise has been made about this recently and I was feeling stressed about it. Apparently people are quite passionate about which button should be the default one in the "permanently delete?" dialog!

[Mark Gaiser]

Yup, we're behavior animals that don't like change ;)

That being said, a lot of those changes made in recent years are in large part thanks to you! And that is in super large majority - say 99.99% - very welcome and awesome!

Sometimes something seemingly simple, like this one i triggered on, slips through unintentional. That's when people start complaining as there is a change they don't like. But as you said, it's unintentional (as seen in the bug report too). I'm sure a next release is going to fix it.

Regardless of a fix. Keep up the amazing work you've been doing! It's much appreciated even though people don't always say that. (It's easier to complain then to compliment I suppose ;) )

[Nate Graham]

Thanks for the kind words, I really appreciate it!