Differential D28103

Support multiple stacked Passive Notifications
ClosedPublic
Actions

Authored by mart on Mar 17 2020, 4:03 PM.

Details

Reviewers
broulik
Group Reviewers
Kirigami
Commits
R169:4e596d8bed9c: introduce PassiveNotification type
R169:659324f28ec6: Support multiple stacked Passive Notifications
R169:59bf32f93dc3: export the item
Summary

add the ability to support more (up to an arbitrary 4) passive notifications shown at once
see also D28130 D28131

Test Plan

it looks and behaves the same when there is only one. when there are multiple they are stacked on top of each other with the most recent at the top

Diff Detail

Repository
R169 Kirigami
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Mar 17 2020, 4:03 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptMar 17 2020, 4:03 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart requested review of this revision.Mar 17 2020, 4:03 PM
mart updated this revision to Diff 77843.Mar 17 2020, 4:06 PM
  • export the item
mart added a comment.Mar 17 2020, 4:08 PM

One thing I'm not sure of is whether going this way, with one popup that shows only one notification or having a popup with a listview that can show multiple notifications at once...
tough for me an important thing is that this thing does *-NOT* become a full fledged notification system

ngraham added a subscriber: ngraham.Mar 17 2020, 4:09 PM

Didn't this already exist in Kirigami, or am I imagining that?

mart added a comment.Mar 17 2020, 4:10 PM
In D28103#629313, @ngraham wrote:

Didn't this already exist in Kirigami, or am I imagining that?

there is ApplicationWindow.showPassiveNotification.
it was only accessible by that (and not accessible by kcms) this makes it an instatiable component

look and features stay the same

mart edited the summary of this revision. (Show Details)Mar 17 2020, 4:11 PM
ngraham added inline comments.Mar 17 2020, 4:22 PM
src/controls/templates/PassiveNotification.qml
18 ↗(On Diff #77843)

messages of limited importance -> maybe something a bit more explicit like transient, ignorable status messages of limited importance

21 ↗(On Diff #77843)

the applciation -> The application

This revision was not accepted when it landed; it landed in state Needs Review.Mar 18 2020, 11:49 AM
Closed by commit R169:4e596d8bed9c: introduce PassiveNotification type (authored by mart). · Explain Why
This revision was automatically updated to reflect the committed changes.
mart reopened this revision.Mar 18 2020, 4:13 PM
mart updated this revision to Diff 77934.Mar 18 2020, 4:13 PM
  • export the item
  • support multiple notifications
  • make it a private component again
  • fix passive notifications for ApplciationItem
mart retitled this revision from introduce PassiveNotification type to Support multiple stacked Passive Notifications.Mar 18 2020, 4:15 PM
mart edited the summary of this revision. (Show Details)
mart edited the test plan for this revision. (Show Details)
broulik added a subscriber: broulik.Mar 18 2020, 4:25 PM
broulik added inline comments.
src/controls/templates/private/PassiveNotification.qml
26

Math.round please

35

Just padding?

66–70

When I rapidly create new passive popups, I can exceed the limit. Maybe it needs a while loop where it closes without animation or something

mart edited the test plan for this revision. (Show Details)Mar 18 2020, 4:41 PM
mart edited the summary of this revision. (Show Details)
mart updated this revision to Diff 77942.Mar 18 2020, 4:58 PM
mart marked 2 inline comments as done.
  • fix quick spawning of notifications
mart added inline comments.Mar 18 2020, 4:58 PM
src/controls/templates/private/PassiveNotification.qml
35

actually, it would make the implicitHeight calculation fail (it's dumb like that and that's why for the new property inset they only added leftInset topInset etcand not a simple inset one)

broulik accepted this revision.Mar 19 2020, 10:13 AM
This revision is now accepted and ready to land.Mar 19 2020, 10:13 AM
Closed by commit R169:659324f28ec6: Support multiple stacked Passive Notifications (authored by mart). · Explain WhyMar 19 2020, 10:18 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 77986

View Options

src/controls/AbstractApplicationItem.qml

Loading...
View Options

src/controls/AbstractApplicationWindow.qml

Loading...
View Options

src/controls/templates/private/PassiveNotification.qml

Loading...
View Options

src/controls/templates/qmldir

Loading...