StreetComplete icon indicating copy to clipboard operation
StreetComplete copied to clipboard

"This shop has been vacant. What's here now" - "house" is not a valid answer.

Open SomeoneElseOSM opened this issue 3 years ago • 24 comments

How to Reproduce

At this location: https://www.openstreetmap.org/node/850026531 The "This shop has been vacant. What's here now" question appears. Type in "house" as the answer (it's in the suggestions list) Tap OK A message "not recognised" is then shown.

Versions affected

Android 10 and 12, SC version v39.1.

SomeoneElseOSM avatar Feb 17 '22 23:02 SomeoneElseOSM

I think I'm seeing something similar for this: https://www.openstreetmap.org/way/426232580

Using 40.0-beta1, although I get no suggestions whatsoever regardless of what I type.

My assumption in my case, and possibly both, is that the disused aspect of the node/way is impacting on the suggestions/tagging.

Apologies if they're entirely unrelated issues, but it's hard to tell with the different versions too.

peternewman avatar Feb 18 '22 00:02 peternewman

It is what it is.

You need to leave a note instead in this case.

westnordost avatar Feb 18 '22 01:02 westnordost

Is my problem the same as @SomeoneElseOSM then @westnordost or entirely unrelated in which case I'll open a new issue for it?

In @SomeoneElseOSM case, can't you at least hide the inapplicable ones from the suggestion list via the same method that says it's not recognised?

peternewman avatar Feb 18 '22 10:02 peternewman

No its not related. First check if the issue persists with v40.1 before you open a ticket

westnordost avatar Feb 18 '22 11:02 westnordost

It is what it is. You need to leave a note instead in this case.

I don't want to leave a note, since I know exactly what tag changes are needed - I'll hop over into another editor to make those changes. I was just hoping to be able to do that directly in StreetComplete.

SomeoneElseOSM avatar Feb 18 '22 11:02 SomeoneElseOSM

Out of curiosity: Why does the app suggest House if the answer is not recognised?

Helium314 avatar Feb 18 '22 18:02 Helium314

Hmm... good question

westnordost avatar Feb 18 '22 19:02 westnordost

"House" is a clothing store brand: https://nsi.guide/index.html?t=brands&k=shop&v=clothes&tt=house#house-3937bd

westnordost avatar Feb 18 '22 20:02 westnordost

And there is a bug that if the selected name is not the first one in the list of suggestions (when typing that exact name), the form would claim that it doesn't know that preset.

westnordost avatar Feb 18 '22 20:02 westnordost

I'm not sure how complicated is this, but perhaps we should skip such NSI brands which conflict with more regular usage (i.e. tag descriptions)

Or at least show NSI brands under quotes to indicate they relate to names of the brands, e.g. "House" vs. house?

mnalis avatar Feb 18 '22 20:02 mnalis

"House" is a clothing store brand:

... in Poland and some other countries:

https://www.housebrand.com/special/store/

but that appears to be an upstream issue:

https://github.com/osmlab/name-suggestion-index/blob/main/data/brands/shop/clothes.json#L3309

(I'd have expected the "include" list there to be a list of valid countries(

SomeoneElseOSM avatar Feb 18 '22 20:02 SomeoneElseOSM

Sure, to make clear that something is a brand somehow would be an improvement. Though, it is not trivial to do.

westnordost avatar Feb 19 '22 12:02 westnordost

I opened https://github.com/osmlab/name-suggestion-index/pull/6205 upstream to partially limit confusion

matkoniecz avatar Feb 19 '22 13:02 matkoniecz

Sure, to make clear that something is a brand somehow would be an improvement. Though, it is not trivial to do.

What about doing it similar to Vespucci and list

House (clothing shop)

not

House

in the list?

As bonus it may allow to disambiguate between various objects of the same brand. For example Carrefour is running both supermarkets and fuel stations what may be also source of confusion.

Or show object type after it is selected below text field?

matkoniecz avatar May 24 '22 11:05 matkoniecz

Sounds reasonable, you could try that

westnordost avatar May 24 '22 12:05 westnordost

@westnordost @FloEdelmann (pinging as I want to bother you about feedback on code design here)

I looked into it and there is one challenge:

displaying something like "House (Clothing Shop)" is easy: make getFeatureName accessible and use context.resources.getFeatureName(feature.tags, featureDictionary)

The tricky part is how given this text one may get correct Feature back?

Searching House (Clothing Shop) with https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/ShopGoneDialog.kt#L106 will now fail, as this extra (Clothing Shop) is not part of actual preset name.

Ideas:

(1) hacky solution, just remove everything in brackets before searching for original feature. Would work for now and break as soon as brand with brackets in name would appear. And be a blatant hack.

(2) somehow associate Feature with its label in AutoCompleteTextView an then just get correct object, no need for searching it again. Store all shown elements that appeared in convertResultToString and their text representation and allow conversion back? That feels wrong to me, but may be a good solution.

(3) modify FeatureDictionary to allow both presentation and search by "House (Clothing Shop)"? And use that new format in dialog, without substantial code modification there. Seems best solution even if annoying to implement (as I would need to test with a new dependency)

(4) some other smart solution that I am missing

matkoniecz avatar Jun 15 '22 06:06 matkoniecz

(2) somehow associate Feature with its label in AutoCompleteTextView an then just get correct object, no need for searching it again. Store all shown elements that appeared in convertResultToString and their text representation and allow conversion back? That feels wrong to me, but may be a good solution.

(4) some other smart solution that I am missing

I don't know if this is two or four, but if this was a HTML select, you'd have the value and the name independently. I appreciate it's a bit more complicated than that, as it's an AutoComplete, but it seems odd there's no equivalent magic function already.

It looks like you can get the position in the list and find it that way (entirely untested by me, but the theory seems sound): https://stackoverflow.com/questions/56319958/autocompletetextview-get-id-when-select-name https://stackoverflow.com/questions/33094482/how-to-get-id-of-selected-item-in-autocompletetextview-in-android

peternewman avatar Jun 15 '22 11:06 peternewman

I see, so in my view, getSelectedFeature is the culprit.

The SearchAdapter is simply filled with a number of strings and clicking on them fills the associated edit text. Then, the actual feature selected is determined by again finding which features match for the text in the input text. This is indeed not very clean.

The cleanest solution would be the following:

  1. Let the search adapter not display a number of plain strings but a number of items that consist of a display string plus a value (i.e. the Feature) (the same way any RecyclerView.Adapter does)
  2. When one from the list is selected, it is memorized which Feature the user selected from the list (and only cleared if he changes back into the edit text and changes the text
  3. thus, when clicking on "OK", if the user selected a feature from the list, use that one. If he didn't (but only typed one), search for a fitting feature like now. Optionally, one could disallow NOT selecting an item from the list alltogether.

This solution enables us to also customize how the items in the result list look, e.g. we are not limited to a string ("House (Clothing Store)") but the items could be displayed e.g. like this:

👚House
Clothing Store

The unknown here is how flexible the SearchAdapter/built-in search functionality is. Worst case, one cannot use the Android components but has to listen to text changed events on the edit text oneself and display a floating popup menu with a recyclerview.adapter.


The next best solution is a subset of the above: Maybe the SearchAdapter allows a callback to be called when something is selected. At that point, it could be memorized that that feature has been selected and that feature is used when tapping on "OK" unless the user further edits the text field.

westnordost avatar Jun 15 '22 11:06 westnordost

but the items could be displayed e.g. like this:

My first response was that is likely not the best idea due to limited space. But it is partially caused by this form starting in middle of the screen and leaving most of space unused...

matkoniecz avatar Jun 15 '22 11:06 matkoniecz

Point is that if it is not just a string, it could be arranged in any form and also include icons or text formatting, e.g.

👚House Clothing Store
should not take up more vertical space.

westnordost avatar Jun 15 '22 12:06 westnordost

I'll do this, with the icons and all

westnordost avatar Sep 19 '22 22:09 westnordost

Thanks.

I wonder (and this may well be a separate task) whether it would be possible to capture "not a shop any more; not even a disused one" as a possible answer?

SomeoneElseOSM avatar Sep 19 '22 22:09 SomeoneElseOSM

And remove the node completely then? I thought about this, but I think in any case it is better to leave a note. Because it is not always clear that indeed the node should be deleted.

It may now be a (company) office, or some other amenity that is simply not selectable in the dialog (because it is a type of place that does not usually rent a shop-space)

westnordost avatar Sep 19 '22 23:09 westnordost

https://user-images.githubusercontent.com/4661658/191753024-70f11b93-9862-4e94-adae-5563d4e568cd.mp4

westnordost avatar Sep 22 '22 12:09 westnordost

... but I think in any case it is better to leave a note.

Leaving a note wouldn't make sense in my example - I'd only be leaving a note for myself!

Right now I have to switch over to Vespucci to fix it.

SomeoneElseOSM avatar Sep 27 '22 13:09 SomeoneElseOSM

Leaving a note wouldn't make sense in my example - I'd only be leaving a note for myself!

It may make sense to do this, for example in case where I am unable to edit right now. Or for people disliking Vespucci.

matkoniecz avatar Sep 27 '22 14:09 matkoniecz