D15589: Add proper labels to Trash Emptied notification

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Dolphin, VDG.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
broulik requested review of this revision.

REVISION SUMMARY
It is not shown by default but when the user enables it, it should show something sensible.

TEST PLAN
F6271141: Screenshot_20180918_150547.png <https://phabricator.kde.org/F6271141>
Before it was just a generic useless KDE logo with "Plasma Workspace" heading

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

AFFECTED FILES
src/trash/dolphintrash.cpp

To: broulik, #dolphin, #vdg
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Elvis Angelaccio]

elvisangelaccio accepted this revision.
elvisangelaccio added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

dolphintrash.cpp:71
+ KNotification::event(QStringLiteral("Trash: emptied"), i18n("Trash Emptied"),
+ i18n("The Trash folder was emptied."), QStringLiteral("user-trash"),
+ nullptr, KNotification::DefaultEvent);

Maybe "folder" is redundant?

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Andres Betts]

abetts added a comment.


Love it! +1

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

elvisangelaccio wrote in dolphintrash.cpp:71
Maybe "folder" is redundant?

Agreed, we don't need the word "folder" in this string. In fact, the whole string is redundant since the title "Trash Emptied" already tells you everything you need to know.

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

ngraham wrote in dolphintrash.cpp:71
Agreed, we don't need the word "folder" in this string. In fact, the whole string is redundant since the title "Trash Emptied" already tells you everything you need to know.

Yes but there must also be a main notification text

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Andres Betts]

abetts added a comment.


If that's the case, do we have just a trash icon, without the folder?

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Nathaniel Graham]

ngraham added a comment.

Post by Andres Betts
If that's the case, do we have just a trash icon, without the folder?

`user-trash` looks like a folder at larger sizes. `trash-empty` might even be more semantically appropriate here, and it doesn't look like a folder at a large size. It does look a bit... spartan, though. Might be worth giving it a prettier version for the large sizes.

Or perhaps we could change `user trash` so that its large sizes don't look like a folder. It is after all rather odd to visually indicate that the trash is a folder. It should probably look like a trash can, not a folder.

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Kai Uwe Broulik]

broulik updated this revision to Diff 43307.
broulik added a comment.


- Drop "Folder"

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15589?vs=41896&id=43307

REVISION DETAIL
https://phabricator.kde.org/D15589

AFFECTED FILES
src/trash/dolphintrash.cpp

To: broulik, #dolphin, #vdg, elvisangelaccio
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


All right, ship it! Let's improve the icon in https://bugs.kde.org/show_bug.cgi?id=399613

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D15589

To: broulik, #dolphin, #vdg, elvisangelaccio, ngraham
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R318:4de45ee5076b: Add proper labels to Trash Emptied notification (authored by broulik).

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15589?vs=43307&id=43311

REVISION DETAIL
https://phabricator.kde.org/D15589

AFFECTED FILES
src/trash/dolphintrash.cpp

To: broulik, #dolphin, #vdg, elvisangelaccio, ngraham
Cc: ngraham, abetts, elvisangelaccio, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp