D20576: Add new notifications KCM

2019-05-09 Thread Kai Uwe Broulik 
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:be5819d0cab7: Merge branch 
kcm-redesign/notifications (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20576?vs=57758=57797

REVISION DETAIL
  https://phabricator.kde.org/D20576

AFFECTED FILES
  CMakeLists.txt
  kcms/CMakeLists.txt
  kcms/notifications/CMakeLists.txt
  kcms/notifications/Messages.sh
  kcms/notifications/filterproxymodel.cpp
  kcms/notifications/filterproxymodel.h
  kcms/notifications/kcm.cpp
  kcms/notifications/kcm.h
  kcms/notifications/kcm_notifications.desktop
  kcms/notifications/package/contents/ui/ApplicationConfiguration.qml
  kcms/notifications/package/contents/ui/PopupPositionPage.qml
  kcms/notifications/package/contents/ui/ScreenPositionSelector.qml
  kcms/notifications/package/contents/ui/SourcesPage.qml
  kcms/notifications/package/contents/ui/main.qml
  kcms/notifications/package/metadata.desktop
  kcms/notifications/sourcesmodel.cpp
  kcms/notifications/sourcesmodel.h

To: broulik, #plasma, #vdg, davidedmundson
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-05-08 Thread Kai Uwe Broulik 
broulik updated this revision to Diff 57758.
broulik added a comment.


  - Don't leak `KNotification`
  - Ensure proper transient

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20576?vs=57652=57758

REVISION DETAIL
  https://phabricator.kde.org/D20576

AFFECTED FILES
  CMakeLists.txt
  kcms/CMakeLists.txt
  kcms/notifications/CMakeLists.txt
  kcms/notifications/Messages.sh
  kcms/notifications/filterproxymodel.cpp
  kcms/notifications/filterproxymodel.h
  kcms/notifications/kcm.cpp
  kcms/notifications/kcm.h
  kcms/notifications/kcm_notifications.desktop
  kcms/notifications/package/contents/ui/ApplicationConfiguration.qml
  kcms/notifications/package/contents/ui/PopupPositionPage.qml
  kcms/notifications/package/contents/ui/ScreenPositionSelector.qml
  kcms/notifications/package/contents/ui/SourcesPage.qml
  kcms/notifications/package/contents/ui/main.qml
  kcms/notifications/package/metadata.desktop
  kcms/notifications/sourcesmodel.cpp
  kcms/notifications/sourcesmodel.h

To: broulik, #plasma, #vdg, davidedmundson
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-05-08 Thread David Edmundson 
davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  I don't like this thing with source proxy. But I can kinda see what it's 
doing and it looks like it'll work.
  
  Lets go for it

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg, davidedmundson
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-05-06 Thread Kai Uwe Broulik 
broulik updated this revision to Diff 57652.
broulik added a comment.


  - Load source model only on sources page but do so immediately there when 
needed
  - Don't set width inside layout

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20576?vs=57047=57652

REVISION DETAIL
  https://phabricator.kde.org/D20576

AFFECTED FILES
  CMakeLists.txt
  kcms/notifications/CMakeLists.txt
  kcms/notifications/Messages.sh
  kcms/notifications/filterproxymodel.cpp
  kcms/notifications/filterproxymodel.h
  kcms/notifications/kcm.cpp
  kcms/notifications/kcm.h
  kcms/notifications/kcm_notifications.desktop
  kcms/notifications/package/contents/ui/ApplicationConfiguration.qml
  kcms/notifications/package/contents/ui/PopupPositionPage.qml
  kcms/notifications/package/contents/ui/ScreenPositionSelector.qml
  kcms/notifications/package/contents/ui/SourcesPage.qml
  kcms/notifications/package/contents/ui/main.qml
  kcms/notifications/package/metadata.desktop
  kcms/notifications/sourcesmodel.cpp
  kcms/notifications/sourcesmodel.h

To: broulik, #plasma, #vdg
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-05-06 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> davidedmundson wrote in SourcesPage.qml:60
> Can you talk me through all this:
> 
> We have a listview with a filtered model with a selected item
> We change the filter
> 
> As long as the proxies are working correctly and track a movement as a 
> movement, the currentIndex of the listview should adjust.
> 
> What does happen?

When I enter a query for which there is no result, the view gets empty and 
`ListView` gets all confused and loses the index or sets it to some random item

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-05-06 Thread Marco Martin 
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in ApplicationConfiguration.qml:103
> you shouldn't specify a width inside a layout.

implicitWidth or Layout.preferredWidth

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-05-02 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> ApplicationConfiguration.qml:103
> +Item {
> +width: Kirigami.Units.gridUnit
> +}

you shouldn't specify a width inside a layout.

> SourcesPage.qml:35
> +Component.onCompleted: {
> +// TODO the page is push'd in Qt.callLater, why is the model still 
> not loaded when we get here?
> +Qt.callLater(function() {

The kcm.filteredModel?

> SourcesPage.qml:60
> +// and then use a proxy model to filter it. We don't get any QML change 
> signals anywhere
> +// and ListView needs a currentIndex number
> +Connections {

Can you talk me through all this:

We have a listview with a filtered model with a selected item
We change the filter

As long as the proxies are working correctly and track a movement as a 
movement, the currentIndex of the listview should adjust.

What does happen?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: davidedmundson, mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D20576: Add new notifications KCM

2019-04-26 Thread Kai Uwe Broulik 
broulik updated this revision to Diff 57047.
broulik added a comment.


  - Drop remove feature for seen apps. They shouldn't really pile up, as 
discussed with VDG
  - Some cleanup

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20576?vs=56304=57047

REVISION DETAIL
  https://phabricator.kde.org/D20576

AFFECTED FILES
  CMakeLists.txt
  kcms/CMakeLists.txt
  kcms/notifications/CMakeLists.txt
  kcms/notifications/Messages.sh
  kcms/notifications/filterproxymodel.cpp
  kcms/notifications/filterproxymodel.h
  kcms/notifications/kcm.cpp
  kcms/notifications/kcm.h
  kcms/notifications/kcm_notifications.desktop
  kcms/notifications/package/contents/ui/ApplicationConfiguration.qml
  kcms/notifications/package/contents/ui/PopupPositionPage.qml
  kcms/notifications/package/contents/ui/ScreenPositionSelector.qml
  kcms/notifications/package/contents/ui/SourcesPage.qml
  kcms/notifications/package/contents/ui/main.qml
  kcms/notifications/package/metadata.desktop
  kcms/notifications/sourcesmodel.cpp
  kcms/notifications/sourcesmodel.h

To: broulik, #plasma, #vdg
Cc: mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20576: Add new notifications KCM

2019-04-24 Thread Marco Martin 
mart added inline comments.

INLINE COMMENTS

> broulik wrote in sourcesmodel.cpp:243
> Not a stupid question, most likely a leftover

delete?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: mart, ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20576: Add new notifications KCM

2019-04-23 Thread Björn Feber 
GB_2 added a comment.


  You can now use `edit-remove`, which got added in D20700 
.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D20576: Add new notifications KCM

2019-04-20 Thread Björn Feber 
GB_2 added a comment.


  Here you go: https://phabricator.kde.org/D20700

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D20576: Add new notifications KCM

2019-04-17 Thread Björn Feber 
GB_2 added a comment.


  In D20576#451805 , @ngraham wrote:
  
  > Yeah, I was thinking of the Breeze icon theme one, which IMO looks better:
  >
  > F6777816: Screenshot_20190417_074431.png 

  >
  > I agree that they should match each other. I think @ndavis is working on 
that in D20623 , as a matter of fact!
  
  
  This is the one from the Plasma theme which will be gone with that patch.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D20576: Add new notifications KCM

2019-04-17 Thread Nathaniel Graham 
ngraham added a subscriber: ndavis.
ngraham added a comment.


  In D20576#451796 , @GB_2 wrote:
  
  > Hmm, it seems like the Plasma theme `list-remove` is different from the 
normal Breeze icon theme `list-remove`. I think we should have the icon from 
the Plasma theme in the Breeze icon theme too, but with a different name, so we 
don't overwrite the existing one.
  
  
  Yeah, I was thinking of the Breeze icon theme one, which IMO looks better:
  
  F6777816: Screenshot_20190417_074431.png 

  
  I agree that they should match each other. I think @ndavis is working on that 
in D20623 , as a matter of fact!

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ndavis, GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart

D20576: Add new notifications KCM

2019-04-17 Thread Björn Feber 
GB_2 added a comment.


  Hmm, it seems like the Plasma theme `list-remove` is different from the 
normal Breeze icon theme `list-remove`. I think we should have the icon from 
the Plasma theme in the Breeze icon theme too, but with a different name, so we 
don't overwrite the existing one.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: GB_2, mmustac, ngraham, zzag, plasma-devel, jraleigh, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-17 Thread Marijo Mustac 
mmustac added a comment.


  What about edit-clear-symbolic or window-close (I think this would be the 
same as for the dismiss action in the widget?)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: mmustac, ngraham, zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-17 Thread Kai Uwe Broulik 
broulik added a comment.


  List-remove looks awful in this context
  F6777476: Screenshot_20190417_105550.png 


REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ngraham, zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-16 Thread Nathaniel Graham 
ngraham added a comment.


  In D20576#451149 , @broulik wrote:
  
  > In D20576#450778 , @ngraham 
wrote:
  >
  > > The trash icons in the app list make me think that it's going to 
uninstall those apps lol
  >
  >
  > Suggestions welcome.
  
  
  `list-remove`, maybe?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ngraham, zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-16 Thread Kai Uwe Broulik 
broulik added a comment.


  In D20576#450778 , @ngraham wrote:
  
  > The trash icons in the app list make me think that it's going to uninstall 
those apps lol
  
  
  Suggestions welcome.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ngraham, zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-15 Thread Nathaniel Graham 
ngraham added a comment.


  The trash icons in the app list make me think that it's going to uninstall 
those apps lol

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: ngraham, zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-15 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> zzag wrote in sourcesmodel.cpp:243
> A bit stupid question: how it gets deleted?

Not a stupid question, most likely a leftover

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-15 Thread Vlad Zagorodniy 
zzag added inline comments.

INLINE COMMENTS

> sourcesmodel.cpp:243
> +
> +KConfig *config = new KConfig(file, KConfig::NoGlobals);
> +
> config->addConfigSources(QStandardPaths::locateAll(QStandardPaths::GenericDataLocation,

A bit stupid question: how it gets deleted?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

To: broulik, #plasma, #vdg
Cc: zzag, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20576: Add new notifications KCM

2019-04-15 Thread Kai Uwe Broulik 
broulik created this revision.
broulik added reviewers: Plasma, VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  This completely rewrites the notifications KCM using `libnotificationmanager`.
  All notification-related settings are moved from the widget to a KCM. This 
includes popup placement settings, history, job management, as well as Task 
Manager progress and badges.
  
  The main page contains global notification settings. The timeout heuristic 
based on word count is dropped and instead a fixed configurable timeout value 
(1-120s, default 5s) is used. On the applications
  
  BUG: 401616
  BUG: 398543
  BUG: 393123
  FIXED-IN: 5.16.0

TEST PLAN
  Depends on D20265  D20266 

  
  Task manager changes to respect these settings pending.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D20576

AFFECTED FILES
  CMakeLists.txt
  kcms/CMakeLists.txt
  kcms/notifications/CMakeLists.txt
  kcms/notifications/Messages.sh
  kcms/notifications/filterproxymodel.cpp
  kcms/notifications/filterproxymodel.h
  kcms/notifications/kcm.cpp
  kcms/notifications/kcm.h
  kcms/notifications/kcm_notifications.desktop
  kcms/notifications/package/contents/ui/ApplicationConfiguration.qml
  kcms/notifications/package/contents/ui/PopupPositionPage.qml
  kcms/notifications/package/contents/ui/ScreenPositionSelector.qml
  kcms/notifications/package/contents/ui/SourcesPage.qml
  kcms/notifications/package/contents/ui/main.qml
  kcms/notifications/package/metadata.desktop
  kcms/notifications/sourcesmodel.cpp
  kcms/notifications/sourcesmodel.h

To: broulik, #plasma, #vdg
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart