WIP: [Notifications] Remove close button border by porting to IconItem
Changes PlannedPublic

Authored by filipf on Mon, Feb 11, 9:56 PM.

Details

Reviewers
ngraham
rooty
Group Reviewers
VDG
Plasma
Summary

This patch ports the close button from a PlasmaComponents.Button to an IconItem so as to:

  1. achieve better visuals -> the close button frame is ugly and unnecessary with Breeze
  2. achieve better symmetry -> the frame makes the close button appear indented
  3. achieve consistency with ToolTipInstance.qml -> also uses an IconItem for its close button

To do/Dilemmas:

  • address symmetry issues caused by the port -> headings are moved slightly upwards
  • port the Configure button as well?
  • add a fallback hover-aware circle for 3rd party themes whose "window-close" icons have no built-in hover effects?
Test Plan

Before:


After:

Diff Detail

Repository
R120 Plasma Workspace
Branch
port-notification-close-button (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8171
Build 8189: arc lint + arc unit
filipf created this revision.Mon, Feb 11, 9:56 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMon, Feb 11, 9:56 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Mon, Feb 11, 9:56 PM
filipf planned changes to this revision.Mon, Feb 11, 9:57 PM
filipf edited the test plan for this revision. (Show Details)
filipf added reviewers: ngraham, VDG, rooty, Plasma.

Any help, as well as opinions, are appreciated.

We have to find a way to take care of the spacing now that the "moat" is gone
Noto Sans 10 pt looks decent enough


But some other fonts actually get pushed upward, see Ubuntu 10pt
before:

after:

filipf edited the summary of this revision. (Show Details)Mon, Feb 11, 10:27 PM
filipf edited the summary of this revision. (Show Details)

I don't like this approach because the IconItem has no pressed state. This is already a problem for the Task Manager tooltips, and I'd like to not replicate it here too. Also I don't really like the fact that the close button is red here even before it's hovered; dismissing a notification is not a destructive action.

I have an idea though. Try applying this on top of your patch:

diff --git a/applets/notifications/package/contents/ui/NotificationItem.qml b/applets/notifications/package/contents/ui/NotificationItem.qml
index 83bd9dcd..5386e557 100644
--- a/applets/notifications/package/contents/ui/NotificationItem.qml
+++ b/applets/notifications/package/contents/ui/NotificationItem.qml
@@ -237,8 +237,8 @@ MouseArea {
             PlasmaCore.IconItem {
                 id:closeIcon
                 anchors.fill: parent
-                active: parent.containsMouse
-                source: "window-close"
+                active: parent.pressed
+                source: parent.containsMouse ? "window-close" : "window-close-symbolic"
                 animated: false
             }
         }

(You can do this with git apply - then paste, then hit Ctrl+d)

Let me know if that appeals to you.

ngraham added a comment.EditedTue, Feb 12, 7:06 PM

Or alternatively we could make the existing ToolButton flat all the time and then it will have the advantage of behaving like all other ToolButtons that people are already familiar with.

Let me know if that appeals to you.

You bet it does, it's looks much better. Unfortunately from what I'm seeing "window-close" is picked up from the desktop\Plasma theme while "window-close-symbolic" is picked up from the icon theme.

In practice that means if you, for instance, use the Oxygen icon theme and the Adapta desktop\Plasma theme, the default look of things is:

And when you hover over the icon you get this:

... which is the opposite of what we'd want!

Or alternatively we could make the existing ToolButton flat all the time and then it will have the advantage of behaving like all other ToolButtons that people are already familiar with.:

We've thought about this too, but the problem is that the button is still centered and placed within a button, which makes it look badly aligned:

Ideally we actually wanted to use RoundButton, but we cannot for the life of us get icons to show up in it.