This fixes:
- Indented notifications line (it's back)
- Heading being non-clickable
- Buttons not being right-aligned in history
- Keyboard navigation
- Long app names
broulik |
Plasma |
This fixes:
No Linters Available |
No Unit Test Coverage |
Buildable 26224 | |
Build 26242: arc lint + arc unit |
applets/notifications/package/contents/ui/FullRepresentation.qml | ||
---|---|---|
454–457 | Move the Svg somewhere outside the delegate, I don't want multiple instances of it | |
applets/notifications/package/contents/ui/NotificationItem.qml | ||
128 | Please use one of the State below for inGroup state | |
148 | This MouseArea should not be necessary and it makes the close button unusable |
applets/notifications/package/contents/ui/NotificationItem.qml | ||
---|---|---|
128 | I originally did this but I had a problem. It displays and works just as good, but it will throw a "you are using anchors in layouts" error in the console. Even though I disable the anchors in the propertychanges, it seems like the property is updated after throwing the error. | |
148 | Why shouldn't it be necessary? PlasmoidHeading inherits from the Control element, which does not let click events through according to the documentation, so I thought I'd need to catch them on top of it. Regarding the close button unusable, it's due to the order of the elements, I'll fix it immediately |
I used the Svg instead of the plasmoidHeading to avoid using a Control. User-wise, it works correctly, but can I get your opinion code-wise?
applets/notifications/package/contents/ui/FullRepresentation.qml | ||
---|---|---|
454–457 | I'm a bit confused here. The PlasmaCore.svg{id: lineSvg...} element was inside a different PlasmaCore.SvgItem that I removed. So what I did was essentially to move it from that SvgItem to this SvgItem. That one is the only instance of PlasmaCore.Svg. | |
applets/notifications/package/contents/ui/NotificationItem.qml | ||
128 | I moved it in a RowLayout to remove the necessity of checking altogether (otherwise, I should've put anchors.margins AND layout.margins for each case and it would've have been quite ugly imo) |
Keyboard navigation is now working again; however, pressing tab to focus buttons seem to no longer be possible after this patch; looking at the code, I'm given the impression that it was true before porting notifications to page as well. Is that correct, or should I investigate tabbing?
applets/notifications/package/contents/ui/NotificationItem.qml | ||
---|---|---|
116 ↗ | (On Diff #81727) | When inGroup the height will be zero so you effectively leak the item contents outside? I don't get what this is supposed to do. |
130–134 ↗ | (On Diff #81727) | This item is pointless since there's only the NotificationHeader inside |
I added one last thing: I feel like it was not a good idea to show the heading in history for notifications that are not in group. I added a inHistory bool, false by default, that's set true in FullRepresentation for notifications that are in history. Heading is not shown for any notification in the history.
I added one last thing
Can we please not mix behavior changes into a patch that fixes bugs and regressions.