Bug 425911 - Notification countdown circle is no longer matching close icon
Summary: Notification countdown circle is no longer matching close icon
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Notifications (show other bugs)
Version: 5.19.90
Platform: Other Linux
: NOR normal
Target Milestone: 1.0
Assignee: Nate Graham
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-08-28 17:44 UTC by Kai Uwe Broulik
Modified: 2020-10-25 17:52 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.20.1


Attachments
Circle screenshot (3.24 KB, image/png)
2020-08-28 17:44 UTC, Kai Uwe Broulik
Details
Non-default font size (442.33 KB, image/png)
2020-10-14 18:11 UTC, popov895
Details
The indicator is hidden behind the icon (44.05 KB, image/png)
2020-10-22 09:46 UTC, popov895
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Uwe Broulik 2020-08-28 17:44:16 UTC
Created attachment 131246 [details]
Circle screenshot

SUMMARY
Before meddling with ToolButton icon sizes, the circle would encompass the close icon. Now it is overlapping which looks bad.

STEPS TO REPRODUCE
1. Have a screen with 2x scaling I guess and dont use Qt scaling in plasma
2. get a notification
3. 

OBSERVED RESULT
Countdown circle looks bad

EXPECTED RESULT
Countdown circle looks good


SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 5.19.80
KDE Frameworks Version: 5.74
Qt Version: 5.14.2
Comment 1 Nate Graham 2020-08-28 17:57:18 UTC
Works for me at 250% scale with Qt scaling, FWIW.
Comment 2 Nate Graham 2020-08-29 03:28:56 UTC
Does it work if you turn on Qt scaling with PLASMA_USE_QT_SCALING=1?
Comment 3 Kai Uwe Broulik 2020-08-30 16:13:14 UTC
Yes, but it messes with my desktop in other interesting ways.
Comment 4 Nate Graham 2020-08-30 16:54:27 UTC
Working on a fix.
Comment 5 Bug Janitor Service 2020-08-30 16:55:57 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/235
Comment 6 popov895 2020-10-14 18:11:41 UTC
Created attachment 132362 [details]
Non-default font size

The patch doesn't work if set a font size different than the default
Comment 7 Nate Graham 2020-10-16 12:38:49 UTC
Git commit 07992bc4abd6e9a3c4d1397eb051dfba50715010 by Nate Graham.
Committed on 16/10/2020 at 12:38.
Pushed by ngraham into branch 'master'.

[notifications] Fix margins of close button timeout indicator

The previous calculation multiplied the DPR by `units.smallSpacing`, which was not correct; we wanted DPR added to the existing margin, not multiplied by it.

Also using Math.floor is safer since Math.round sometimes rounds up (e.g. in case DPR is 2.501) which can cause undesirable effects.

M  +2    -2    applets/notifications/package/contents/ui/NotificationHeader.qml

https://invent.kde.org/plasma/plasma-workspace/commit/07992bc4abd6e9a3c4d1397eb051dfba50715010
Comment 8 Nate Graham 2020-10-16 12:39:40 UTC
Git commit 6d2ef8e25612d0c125190983bfed90bec106ec00 by Nate Graham.
Committed on 16/10/2020 at 12:39.
Pushed by ngraham into branch 'Plasma/5.20'.

[notifications] Fix margins of close button timeout indicator

The previous calculation multiplied the DPR by `units.smallSpacing`, which was not correct; we wanted DPR added to the existing margin, not multiplied by it.

Also using Math.floor is safer since Math.round sometimes rounds up (e.g. in case DPR is 2.501) which can cause undesirable effects.


(cherry picked from commit 07992bc4abd6e9a3c4d1397eb051dfba50715010)

M  +2    -2    applets/notifications/package/contents/ui/NotificationHeader.qml

https://invent.kde.org/plasma/plasma-workspace/commit/6d2ef8e25612d0c125190983bfed90bec106ec00
Comment 9 popov895 2020-10-22 09:45:55 UTC
As I said in Comment 6, this bug can still be reproduced by setting the font size to be larger than the default.
Comment 10 popov895 2020-10-22 09:46:46 UTC
Created attachment 132628 [details]
The indicator is hidden behind the icon
Comment 11 Nate Graham 2020-10-25 17:20:01 UTC
That's the intended appearance now. :)
Comment 12 popov895 2020-10-25 17:52:17 UTC
Really? It's better to completely remove this indicator than to leave it as it's now.