D26640: [applets/weather] Port weather station picker to QQC2+ListView

Kai Uwe Broulik noreply at phabricator.kde.org
Fri Jan 17 15:36:55 GMT 2020


broulik added inline comments.

INLINE COMMENTS

> WeatherStationPicker.qml:44
>          if (!success) {
>              noSearchResultReport.text = i18nc("@info", "No weather stations found for '%1'", searchString);
>              noSearchResultReport.visible = true;

Given the item is hidden anyway, you can probably assign this as a binding right away

> WeatherStationPicker.qml:134
> +            background.visible = true;
> +            searchStringEdit.forceActiveFocus();
>          }

Does `focus: true` on the `TextField` instead of the `ListView` make this redundant?

> WeatherStationPicker.qml:144
> +            keyNavigationEnabled: true
> +            keyNavigationWraps: true
> +

This is unlike any other list we have in settings?

> WeatherStationPicker.qml:169
> +                wrapMode: Text.WordWrap
> +                visible: locationListView.count === 0 && searchStringEdit.length > 0
> +                enabled: false

You're constantly breaking this binding by assigning to it elsewhere programmatically.

REPOSITORY
  R114 Plasma Addons

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

To: ngraham, #vdg, #plasma, broulik
Cc: fvogt, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200117/4fb8b5ef/attachment.html>


More information about the Plasma-devel mailing list