D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs added a comment.


  In D11261#224058 , @abetts wrote:
  
  > In D11261#224056 , @Fuchs wrote:
  >
  > > In D11261#224054 , @abetts 
wrote:
  > >
  > > > We have discussed something like this before. My only worry with this 
implementation is that we have so much red on the screen that it becomes 
distracting. Not the fault of this patch, but I am wondering if there is a way 
that we can use a monochrome X button?
  > >
  > >
  > > Both broulik and I would have prefered the trash icon, however, due to 
consistency ...
  > >
  > > red, on the other hand, is good. It is a destructive action which can't 
be undone, so a bit of red doesn't hurt.
  >
  >
  > Then please use the same X icon at the top of the list, to clear all.
  
  
  I am definitely not going to use the same icon for different actions, sorry. 
That goes against everything I've been taught in UX and, for destructive 
actions, is in my opinion quite catastrophic. If two buttons do different 
things, they should not have the same icon.
  
  What should be fixed is that action-delete is something else in plasma (see 
screenshot) than in non-plasma (where it is a trash can, which would work).  If 
that is fixed, the icon also looks better, and fixed across every piece of qml 
that uses it. But that's a different bug.
  
  A different icon that has been proposed is "clear-all", however, due to the 
nature of that icon  (a left pointing arrow) it also looks wrong. Once that is 
fixed, I'd be willing to change to clear-all, but then as well it should be 
fixed across plasma, and not only in this plasmoid here.
  
  tl;dr:   until some icons are fixed and more global discussions have been 
held, I am not going to alter this change further.

REPOSITORY
  R120 Plasma Workspace

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

To: Fuchs, broulik
Cc: abetts, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Andres Betts 
abetts added a comment.


  In D11261#224056 , @Fuchs wrote:
  
  > In D11261#224054 , @abetts wrote:
  >
  > > We have discussed something like this before. My only worry with this 
implementation is that we have so much red on the screen that it becomes 
distracting. Not the fault of this patch, but I am wondering if there is a way 
that we can use a monochrome X button?
  >
  >
  > Both broulik and I would have prefered the trash icon, however, due to 
consistency ...
  >
  > red, on the other hand, is good. It is a destructive action which can't be 
undone, so a bit of red doesn't hurt.
  
  
  Then please use the same X icon at the top of the list, to clear all.

REPOSITORY
  R120 Plasma Workspace

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

To: Fuchs, broulik
Cc: abetts, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs added a comment.


  In D11261#224054 , @abetts wrote:
  
  > We have discussed something like this before. My only worry with this 
implementation is that we have so much red on the screen that it becomes 
distracting. Not the fault of this patch, but I am wondering if there is a way 
that we can use a monochrome X button?
  
  
  Both broulik and I would have prefered the trash icon, however, due to 
consistency ...
  
  red, on the other hand, is good. It is a destructive action which can't be 
undone, so a bit of red doesn't hurt.

REPOSITORY
  R120 Plasma Workspace

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

To: Fuchs, broulik
Cc: abetts, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Andres Betts 
abetts added a comment.


  We have discussed something like this before. My only worry with this 
implementation is that we have so much red on the screen that it becomes 
distracting. Not the fault of this patch, but I am wondering if there is a way 
that we can use a monochrome X button?

REPOSITORY
  R120 Plasma Workspace

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

To: Fuchs, broulik
Cc: abetts, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs closed this revision.

REPOSITORY
  R120 Plasma Workspace

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

To: Fuchs, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs added a comment.


  New screenshot with fixes F5750621: clearhistory.png 


REPOSITORY
  R120 Plasma Workspace

BRANCH
  fuchs-notification-clearhistory (branched from master)

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

To: Fuchs, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs updated this revision to Diff 29325.
Fuchs added a comment.


  - Better hack for placement

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11261?vs=29321=29325

BRANCH
  fuchs-notification-clearhistory (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/Notifications.qml

To: Fuchs, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs updated this revision to Diff 29321.
Fuchs added a comment.


  - Change the layout import according to kbroulik

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11261?vs=29320=29321

BRANCH
  fuchs-notification-clearhistory (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/Notifications.qml

To: Fuchs, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs updated this revision to Diff 29320.
Fuchs added a comment.


  - Use Math.round() as per discussion

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11261?vs=29319=29320

BRANCH
  fuchs-notification-clearhistory (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/Notifications.qml

To: Fuchs, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11261: Add a button to clear the notification history

2018-03-12 Thread Christian 
Fuchs updated this revision to Diff 29319.
Fuchs added a comment.


  - Capitalized History in the tooltip label as per discussion

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11261?vs=29318=29319

BRANCH
  fuchs-notification-clearhistory (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/Notifications.qml

To: Fuchs, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart