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

[Kai Uwe Broulik]

broulik added a comment.


  Cool.
  
  > Anyone any insight whether that is worth filing?
  
  QtQuick Controls 1 is basically dead..

INLINE COMMENTS

> configWeatherStation.qml:101
> +                checkable: true
> +                checked: model.checked
> +                onToggled: {

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?

> configWeatherStation.qml:147
> +                id: serviceSelectionButton
> +                iconName: "services"
> +                tooltip: i18n("Select weather services providers")

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)

> servicelistmodel.cpp:32
> +    const QVariantList plugins = dataengine->containerForSource(QLatin1String("ions"))->data().values();
> +    foreach (const QVariant& plugin, plugins) {
> +        const QStringList pluginInfo = plugin.toString().split(QLatin1Char('|'));

Use range-for in new code, given `plugins` is already `const`, safe to do:
`for (const QVariant &plugin : plugins)`

> servicelistmodel.cpp:86
> +        item.checked = value.toBool();
> +        emit dataChanged(index, index);
> +

I think it would be better to also check if it actually changed, I recall QML's "model" property not being as smart with that as I thought it would be.

> servicelistmodel.h:54
> +    enum Roles {
> +        CheckedRole = Qt::UserRole
> +    };

There's a `Qt::CheckStateRole`, using that might save you some boilerplate code in various places

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/20180109/4c57cfb2/attachment.html>