Where are you?

[jpnurmi]

The map in the original design is not accessible. Therefore, the initial version of the page comes without a map.

location.mp4

Close: #38

[Feichtmeier]

How about a second corresponding text widget that displays the timezone that your location belongs to?

[jpnurmi]

@elioqoshi do you have any thoughts on @Feichtmeier's proposal above?

[elioqoshi]

@Feichtmeier 's addition is a good proposal! I do feel the whole page for just a dropdown feels a little empty. In my initial explorations I had a flow where one can choose the timezone via a dropdown AND the map (if you'd choose it in one, the other one would be updated accordingly). Do you think this is feasible?

[jpnurmi]

I have added a timezone entry and updated the screencast in the PR description. If we decide to add a map, I'd like to leave that as a separate (big) task. 😄

[oSoMoN]

Quick smoke tests: my location is auto-detected to Madrid (Spain), which isn't very accurate, but IIRC ubiquity doesn't do a better job, so that's not a regression. The timezone (Europe/Madrid) is correct.

Searching in the location text field isn't working as I'd expect: when I input "barcelon", I would expect "Barcelona (Catalonia, Spain)" to be in the list of results, but it isn't:

flutter: DEBUG geo_service: Search "barcelon" matches 1 locations
flutter: DEBUG geo_service: Search "barcelona" matches 17 locations

And a dummy question, because I haven't read the code yet: can the names of cities and timezones be localized in the selected language, or is the data only available in English?

[jpnurmi]

my location is auto-detected to Madrid (Spain), which isn't very accurate

The result is from https://geoip.ubuntu.com/lookup. I get sometimes even slightly different results when refreshing the page.

Searching in the location text field isn't working as I'd expect: when I input "barcelon", I would expect "Barcelona (Catalonia, Spain)" to be in the list of results, but it isn't:

I got also surprised that geoname-lookup.ubuntu.com doesn't do a "starts with" search.

• https://geoname-lookup.ubuntu.com/?query=barcelon
  vs.
• https://geoname-lookup.ubuntu.com/?query=barcelona

[jpnurmi]

can the names of cities and timezones be localized in the selected language, or is the data only available in English?

Non-English alternate names are included in both online and offline lookups:

• https://geoname-lookup.ubuntu.com/?query=berliini (Berlin in Finnish)
• local offline database with 15000 capital/largest cities (cities15000.txt)

[oSoMoN]

I got also surprised that geoname-lookup.ubuntu.com doesn't do a "starts with" search.

* https://geoname-lookup.ubuntu.com/?query=barcelon
  vs.

* https://geoname-lookup.ubuntu.com/?query=barcelona

And that's a known bug which will turn 10 years old later this year, unless we do something about it ;)

[oSoMoN]

That's a minor detail, so feel free to ignore, but I find the logging a bit misleading. When I input "barcelona", I see:

flutter: DEBUG geo_service: Search "barcelona" matches 17 locations

So far so good. Then I click on the first entry in the results, my location is correctly selected, but I see:

flutter: DEBUG geo_service: Search "Barcelona (Catalonia, Spain)" matches 0 locations

I understand why this happens, but for someone reading the logs it's a tad misleading, maybe we could avoid issuing a request when the user selects an entry in the search results?

[jpnurmi]

Yeah, the extra request is a bit annoying.

The problem is that Autocomplete.optionsBuilder gets called before the Autocomplete.onSelected handler.

1. submit text
2. build (search) options
3. select location

It gets really ugly but something like this would prevent the spurious search.

diff --git a/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart b/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart
index c8f8e641..709df3e3 100644
--- a/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart
+++ b/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart
@@ -80,7 +80,14 @@ class WhereAreYouPageState extends State<WhereAreYouPage> {
                         );
                       },
                       displayStringForOption: formatLocation,
-                      optionsBuilder: (value) {
+                      optionsBuilder: (value) async {
+                        // wait for onSelected to get called
+                        await Future.delayed(const Duration(microseconds: 1));
+                        // avoid searching a selected and formatted location
+                        if (value.text ==
+                            formatLocation(model.selectedLocation)) {
+                          return model.locations;
+                        }
                         return model.searchLocation(value.text);
                       },
                       onSelected: model.selectLocation,

[jpnurmi]

I guess we can assume that city names don't contain parentheses, so here's another possibility:

diff --git a/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart b/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart
index c8f8e641..a2d64f82 100644
--- a/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart
+++ b/packages/ubuntu_desktop_installer/lib/pages/where_are_you/where_are_you_page.dart
@@ -81,6 +81,10 @@ class WhereAreYouPageState extends State<WhereAreYouPage> {
                       },
                       displayStringForOption: formatLocation,
                       optionsBuilder: (value) {
+                        if (RegExp(r'^.*\(.*\)$').hasMatch(value.text)) {
+                          // avoid searching a selected and formatted location
+                          return model.locations;
+                        }
                         return model.searchLocation(value.text);
                       },
                       onSelected: model.selectLocation,

[oSoMoN]

It gets really ugly but something like this would prevent the spurious search.

I have to agree this is rather ugly, I'd prefer if we could devise a cleaner approach.

I guess we can assume that city names don't contain parentheses, so here's another possibility:

Unfortunately, it appears that the names of some cities listed in cities15000.txt do contain parentheses (e.g. "Newport (Kentucky)", "سقّز (٢)", "Τιμπού (θερινή) Πουνάκα (χειμερινή)", …).

And given the implementation of GeoLocation.toDisplayString(), there's no guarantee the display string will always contain parentheses (although I would expect the case without parentheses to be infrequent).

The documentation for Autocomplete.onSelected() says « Any TextEditingController listeners will not be called when the user selects an option », so perhaps we could have a listener that saves the value of the last actual search in the model, and if the formatted value for that last search is equal to value.text, then avoid issuing a new search query?

[oSoMoN]

LGTM overall, just a couple of very minor comments.

[jpnurmi]

The documentation for Autocomplete.onSelected() says « Any TextEditingController listeners will not be called when the user selects an option »

Hmm, this is not true. The first thing it does is to update the text editing controller's value, and then calls onSelected.

https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/autocomplete.dart#L331-L335

[jpnurmi]

I'm afraid adding a listener or checking the editing controller's current value doesn't really help unless we postpone the search in one way or another. The problem is essentially the same as previously that when building options, we don't know yet if an option has been selected, which could be either by clicking an option or by submitting the field using the keyboard.

[jpnurmi]

Thanks! I've opened #652 for the extra search request.