D9751: [weather] Add configuration option which weather services providers to use

[Friedrich W. H. Kossebau]

kossebau marked 4 inline comments as done.
kossebau added inline comments.

INLINE COMMENTS

> broulik wrote in configWeatherStation.qml:101
> Can you verify that toggling the MenuItem does not break this binding? It shouldn't cause much trouble, though, as you only change selected services in response to this being toggled, right?

Good catch.
Yes, it breaks it, also creates a binding loop which QML warns about. Somehow completely missed that. Shows why I can not put QML expert on my business card :)

Tried to solve by removing the bindings and instead adding a readonly proxy(?) property for the model item to the menu item and then updating the values of the menu item and the model item in the respective changed/toggled handlers. Seems to work, is that a proper solution? Had not found anything to copycat on a quick search, so assembled this from implementation hints across the places. How expensive is such a proxy property?

> broulik wrote in configWeatherStation.qml:147
> Is that the actual icon we use for such popups? A blue flag isn't very descriptive imho, though I can see that Dolphin's "Service" menu also uses that. (I can't think of a better solution, though, other than the funnel filter icon)

Agreed that the flag is quite strange.
While the icon name might be good enough, the current Breeze icon theme implementation with the flag is rather strange and get a new approach. No idea which global(?) metapher is referred to with a flag for services...

> broulik wrote in servicelistmodel.h:54
> There's a `Qt::CheckStateRole`, using that might save you some boilerplate code in various places

Thanks, missed that. Sadly no rolename set, so I still have to do this myself. But yes, saved some lines still :P

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180110/3c8f28be/attachment-0001.html>