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

Authored by ngraham on Jan 13 2020, 9:12 PM.

Details

Summary

This fixes the bug I introduced when I fixed 414442 in the wrong way, makes the UI more
recular, removes a QQC1 dep, and makes the view fully keyboard-navigeable.

BUG: 414442
FIXED-IN: 5.18.0

Test Plan

Diff Detail

Repository
R114 Plasma Addons
Branch
port-to-listview-in-scrollview (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21540
Build 21558: arc lint + arc unit
ngraham created this revision.Jan 13 2020, 9:12 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 13 2020, 9:12 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Jan 13 2020, 9:12 PM
fvogt added a subscriber: fvogt.Jan 13 2020, 9:20 PM

When I tested this, there was a very noticable delay (~10s) between starting the search and items appearing, so the busy indicator is IMO necessary.

For me it was instant after this patch. But I'll re-add it.

ngraham updated this revision to Diff 73464.Jan 13 2020, 9:24 PM

Re-add busy indicator

broulik added inline comments.Jan 17 2020, 3:36 PM
applets/weather/package/contents/ui/config/WeatherStationPicker.qml
43–44

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

135

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

144

This is unlike any other list we have in settings?

167

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

ngraham updated this revision to Diff 73780.Jan 17 2020, 3:56 PM
ngraham marked 3 inline comments as done.

Address review comments

ngraham added inline comments.Jan 17 2020, 3:56 PM
applets/weather/package/contents/ui/config/WeatherStationPicker.qml
144

I copied it from the notifications KCM :)

Using Kirigami.SearchField would be nice, so Ctrl+F acts as a shortcut for getting back to the search field when focus jumped to the list.

(And can we later improve that weird workflow where no providers are selected and the Search field is disabled for no immediately obvious reason? :)

applets/weather/package/contents/ui/config/WeatherStationPicker.qml
41

You no longer hide the noSearchResultsReport label when a new query starts, awkwardly overlapping the busy indicator until the new one completes.

94

Maybe explicitly focus this it when the dialog opens again otherwise on subsequent opening of the dialog it will instead have whatever item was focused previously?

144

The notifications KCM doesn't wrap, though?

146

Unused id

166

This should be set programatically when the query finishes like before instead of a binding. Otherwise you'll end up in the weird situation where it says "Nothing found for foo" and then you start typing a new query and it live updates to "Nothing found for bar" even though you didn't actually launch a new query

ngraham updated this revision to Diff 74184.Jan 22 2020, 11:35 PM
ngraham marked 5 inline comments as done.

More review comments

ngraham marked 2 inline comments as done.Jan 22 2020, 11:36 PM
broulik accepted this revision.Jan 30 2020, 3:35 PM
This revision is now accepted and ready to land.Jan 30 2020, 3:35 PM
This revision was automatically updated to reflect the committed changes.