Bug 385158

Summary:	QDialogButtonBox::Discard is overridden to use "edit-clear" icon; upstream setting ("edit-delete" icon) is correct
Product:	[Frameworks and Libraries] frameworks-kwidgetsaddons	Reporter:	Nate Graham <nate>
Component:	general	Assignee:	Christoph Feck <cfeck>
Status:	RESOLVED FIXED
Severity:	normal	CC:	kde, kdelibs-bugs-null, plasma-bugs-null, simonandric5
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://commits.kde.org/kwidgetsaddons/6f30aad51452583d5bfc6815acae099b3f0e017c	Version Fixed/Implemented In:
Sentry Crash Report:

Description Nate Graham 2017-09-27 23:22:56 UTC

When you create a QDialogButtonBox with a Discard button via QDialogButtonBox::Discard, it uses the edit-clear icon from the theme. An example is Spectacle's main window.

This is incorrect: the Discard action is destructive, so it should use the edit-delete icon. In fact, upstream Qt uses this icon by default: https://github.com/qt/qtbase/blob/6d699d08200b1fe3a616dfbc275d46c98b77fcbd/src/widgets/styles/qcommonstyle.cpp#L5780

Somewhere we're overriding this, and we should probably stop, because edit-delete is more correct.

Comment 1 Kai Uwe Broulik 2017-09-28 08:21:55 UTC

In frameworkintegration/src/kstyle/kstyle.cpp it is set to dialog-cancel
It is kwidgetsaddons KGuiItem::discard() that Spectacle is using that is set to edit-clear

Comment 2 Nate Graham 2017-09-28 13:11:48 UTC

Thanks for the info, Kai! I've submitted a patch for this: https://phabricator.kde.org/D8029

Comment 3 Nate Graham 2017-09-28 16:44:06 UTC

Git commit 6f30aad51452583d5bfc6815acae099b3f0e017c by Nathaniel Graham.
Committed on 28/09/2017 at 16:43.
Pushed by ngraham into branch 'master'.

Use edit-delete icon for destructive discard action

Summary:

Use edit-delete icon for buttons that execute destructive discard actions. This is what Upstream Qt originally had, but we were overriding it to use a less-appropriate icon (edit-clear)

Test Plan:
Tested in KDE Neon. An example of the change can be seen in Spectacle's main window:
{F4233176}

Reviewers: #frameworks, #vdg, dfaure, rkflx, davidedmundson, cfeck

Reviewed By: cfeck

Subscribers: cfeck, #frameworks

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

M  +1    -1    src/kstandardguiitem.cpp

https://commits.kde.org/kwidgetsaddons/6f30aad51452583d5bfc6815acae099b3f0e017c

Comment 4 Christoph Feck 2017-09-28 21:04:56 UTC

Thanks for your bug work, Nate. Hope you don't burn out too soon ;)

Comment 5 Nate Graham 2017-09-28 21:12:41 UTC

Thanks, me too! And thanks for cleaning up some of my messes when triaging bugs. I'll get better over time.

Judging by past experience with previous goal-directed hobbies, you should have my full-time attention for about 2 years. After that I'll burn out if I either feel like I'm not making progress, or lose interest if I've achieved the goal I set out to achieve (Improving KDE software's use cases for regular people such that I can switch my mother from her Mac and not have it be a support nightmare). Assuming I'm still making progress and the goal hasn't been achieved yet, I may just keep working on this stuff for a long time. :)

FWIW, you guys have been amazingly more patient and welcoming than the GNOME folks were when I first tried to jump on board their train. It's a pleasure to work together on this project.