D24766: Revert change to make the notification persistent
ngraham added a comment. I had a feeling this would be popular. :) REPOSITORY R134 Discover Software Store BRANCH revert-sticky-notifications (branched from Plasma/5.17) REVISION DETAIL https://phabricator.kde.org/D24766 To: ngraham, apol, broulik, #discover_software_store Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
This revision was automatically updated to reflect the committed changes. Closed by commit R134:d9db05ddef96: Revert change to make the notification persistent (authored by ngraham). REPOSITORY R134 Discover Software Store CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24766?vs=68265=68266 REVISION DETAIL https://phabricator.kde.org/D24766 AFFECTED FILES notifier/DiscoverNotifier.cpp To: ngraham, apol, broulik, #discover_software_store Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
apol accepted this revision. apol added a comment. This revision is now accepted and ready to land. REPOSITORY R134 Discover Software Store BRANCH revert-sticky-notifications (branched from Plasma/5.17) REVISION DETAIL https://phabricator.kde.org/D24766 To: ngraham, apol, broulik, #discover_software_store Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
ngraham updated this revision to Diff 68265. ngraham marked an inline comment as done. ngraham added a comment. Don't stop keeping track of the notification, so we can still close it when Discover is launched REPOSITORY R134 Discover Software Store CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24766?vs=68264=68265 BRANCH revert-sticky-notifications (branched from Plasma/5.17) REVISION DETAIL https://phabricator.kde.org/D24766 AFFECTED FILES notifier/DiscoverNotifier.cpp To: ngraham, apol, broulik, #discover_software_store Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
ngraham added a comment. That pares down the diff quite a bit. :) REPOSITORY R134 Discover Software Store REVISION DETAIL https://phabricator.kde.org/D24766 To: ngraham, apol, broulik, #discover_software_store Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
apol added a comment.
Other than that, LGTM
INLINE COMMENTS
> DiscoverNotifier.cpp:67
> KRun::runCommand(QStringLiteral("plasma-discover"), nullptr);
> -if (m_updatesAvailableNotification) {
> m_updatesAvailableNotification->close(); }
> }
I wouldn't remove these. It's good that it reacts to opening Discover. It shows
that it's connected.
REPOSITORY
R134 Discover Software Store
REVISION DETAIL
https://phabricator.kde.org/D24766
To: ngraham, apol, broulik, #discover_software_store
Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
ognarb added a comment. +1 REPOSITORY R134 Discover Software Store REVISION DETAIL https://phabricator.kde.org/D24766 To: ngraham, apol, broulik, #discover_software_store Cc: ognarb, nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
nicolasfella added a comment. Yes please! REPOSITORY R134 Discover Software Store REVISION DETAIL https://phabricator.kde.org/D24766 To: ngraham, apol, broulik, #discover_software_store Cc: nicolasfella, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D24766: Revert change to make the notification persistent
ngraham created this revision. ngraham added reviewers: apol, broulik, Discover Software Store. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ngraham requested review of this revision. REVISION SUMMARY Nobody liked it and it was annoying. From a conceptual standpoint, it was redundant anyway because the SNI in the system tray is already persistent when there's an update available. Targeting 5.17.1 for this. TEST PLAN Restart plasmashell, see that the "updates available" notification disappears after a few seconds REPOSITORY R134 Discover Software Store BRANCH revert-sticky-notifications (branched from Plasma/5.17) REVISION DETAIL https://phabricator.kde.org/D24766 AFFECTED FILES notifier/DiscoverNotifier.cpp notifier/DiscoverNotifier.h To: ngraham, apol, broulik, #discover_software_store Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
