Added the new PlasmoidHeading component to notifications, see:
Details
- Reviewers
broulik ngraham - Commits
- R120:ca7a9bcbbc1f: Add top area to notifications
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- notification_toparea
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 24583 Build 24601: arc lint + arc unit
Please also test with icons and configure button, I use https://tests.peter.sh/notification-generator/ which can generate a multitude of test notifications
applets/notifications/package/contents/ui/NotificationPopup.qml | ||
---|---|---|
168 ↗ | (On Diff #77406) | You know I'm not a fan of randomly dividing sizes |
Yepp, I did test it with various notifications.
I do! But since I was unable to add that class to my local framework, I tested it by putting the PlasmoidHeading in the same folder and using "PlasmoidHeading" instead of "PlasmaExtras.PlasmoidHeading". Then I replaced it to make the diff, but got the name wrong.
Anyway - the main thing missing in this task is how to avoid broke history - do you have any tip on how I could address it?
applets/notifications/package/contents/ui/NotificationItem.qml | ||
---|---|---|
119 | I set up Kate to show trailing spaces now, so I should stop putting them accidentally. | |
applets/notifications/package/contents/ui/NotificationPopup.qml | ||
168 ↗ | (On Diff #77406) | What should I use? 0 is too small, while the entire margin is too big. |
If it doesn't work for you, that's a good bet it won't work for other people either, which means it can't actually be tested and merged. :) Whatever is preventing it from working when you integrate it in the correct way is a bug that needs to be fixed so that the patch is testable and merge-able, not something you can hack around.
This looks amazing! I love it. I think the content area below the header needs an additional units.smallSpacing margin though. The text in that area comes really close to touching the header:
Weird, I do see a margin. Do you have any idea what could case this issue (us seeing different things)? I assumed that the margin was given by a qml Heading default topmargin
The padding looks much better with default font settings:
I'm using 11pt Ubuntu though, not 10pt Noto Sans. So now I'm thinking that there's some issue with making the padding dependent on the font. Still, that's clearly unrelated to this patch (it just exposes a pre-existing issue) so I say go for it! Bonus points for investigating the font padding issue in a follow-up patch. :)
So I think the padding issue isn't related to fonts after all. I noticed while doing a backup today that there's no spacing between the header and the content underneath it even when there's no text.