Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: AddAddressStreet #1782

Merged
merged 110 commits into from
May 18, 2020
Merged

WIP: AddAddressStreet #1782

merged 110 commits into from
May 18, 2020

Conversation

dbdean
Copy link
Contributor

@dbdean dbdean commented Apr 17, 2020

Thanks to @matkoniecz for his great work on starting this. I've gotten it up to date with StreetComplete master and had a go at moving the implementation to AAddLocalizedNameForm.

Once this is completed it should resolve #213.

I've also

  • Added the street-number to the prompt to make it easier to tell which house is being referred to (and often as an additional clue to the street it belongs to)
  • Upped the distance for close streets to 250 metres as houses in the middle of long, straight streets were getting no suggestions
  • Made it remember the last street you entered, for easier surveying along streets.

There's still plenty to do though, so this is just a WIP at the moment. I'm very open to suggestions, and it's very possible that some of this is a bit hacked together, as I'm not super familiar with this codebase - I'm happy to change anything!

@dbdean
Copy link
Contributor Author

dbdean commented May 17, 2020

Additionally, I originally said in #213 that the quest should only be asked for housenumbers at the beginning of the road (house numbers 1, 2, 3, 4, ...) to avoid spamming the user with quests where he answers with always the same answer and because it is more efficient to enter all the addr:street for a street in a full editor like JOSM via multiselect. Only for places where it is not clear to which road a building belongs, it is necessary to survey it. These would be the beginning of roads and maybe intersections. What do you think of this?

I would prefer that the quest shows for all street numbers without streets/places/etc. There are a lot of places where street numbers are ambiguous even not in the low numbers (at intersections generally). I think given that (mappers using) StreetComplete created all of these numbers-without-streets (at least in my area), we should be responsible for cleaning them up. It's no more spammy than the addr:housenumber quest.

@dbdean
Copy link
Contributor Author

dbdean commented May 17, 2020

We could do something like (missing street-names within 50m of an intersection of two differently named roads) to catch intersections. I'm not sure how an equivalent design for addr:place would work.

Even so, could we make an option to show all for those of us who want to see them? Or maybe two quests: intersection street-names and non-intersection street names?

@westnordost
Copy link
Member

I would prefer that the quest shows for all street numbers without streets/places/etc. There are a lot of places where street numbers are ambiguous even not in the low numbers (at intersections generally). I think given that (mappers using) StreetComplete created all of these numbers-without-streets (at least in my area), we should be responsible for cleaning them up. It's no more spammy than the addr:housenumber quest.

One argument for not showing the quest not near intersections was that the user might not always know in which he is currently is because he does not have a good overview.
But okay, let's go with showing it for all and see how it is accepted.

This made it necessary to refactor/generalize the AAddLocalizedNameForm
a bit, but also solved the bug where the place name input in the form
would not be persisted correctly on recreation of activity
1. no quest should start out prefilled in a way that one can immediately (or inadvertedly) tap on OK
2. proper implementation would need more work so that it works correctly for names with multiple locales
@westnordost
Copy link
Member

I am currently doing some smaller fixes and adaptions on it.

One good news: I think your assumption that around only works on nodes is wrong, at least since 0.7.55. See https://overpass-turbo.eu/s/U6a

1. no quest should start out prefilled in a way that one can immediately (or inadvertedly) tap on OK
2. proper implementation would need more work so that it works correctly for names with multiple locales
@westnordost
Copy link
Member

The problem was the RoadNameSuggestionsDao, which did the checks only based on points. I am right now changing this.

@westnordost
Copy link
Member

Hmm, after I rummaged about in the code, I wonder if an interface like this wouldn't be nicer? (It's also easier to implement):

What street does the house number 1 belong to?
Tap on the map to select which street it does belong to

@westnordost
Copy link
Member

Well maybe this could be added later as an additional method how to input the road name. One simply has to override onClickMapAt(position: LatLon, clickAreaSizeInMeters: Double) and return true.

road segments with same name(s) are now merged in the road name suggestions dao name and then sorted by distance to the element
Copy link
Contributor Author

@dbdean dbdean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't want it to go this way (in comment to automatically having the last street name ready to go), then I think we are going to need some way to automatically fill in the last value, as the quest will get dull fast otherwise.

@dbdean
Copy link
Contributor Author

dbdean commented May 18, 2020

Can we use the Recycler view to show the last N chosen street names, and a blank space, then the user has to explicity select the last street name before they can click OK?

@dbdean
Copy link
Contributor Author

dbdean commented May 18, 2020

Might even be useful for the AddRoadName quest too.

@dbdean
Copy link
Contributor Author

dbdean commented May 18, 2020

Making that change is a little above my level of ability here at the moment though.

@dbdean
Copy link
Contributor Author

dbdean commented May 18, 2020

If the limiting it to the top-4 closest roads works, maybe we can just show them all at the same time in the form, and a button to put in a different road if it isn't one of them. Then the user could easily click on the last road, then click ok in only two touches.

@westnordost
Copy link
Member

Making that change is a little above my level of ability here at the moment though.

Actually, it is easier than you think. The AALocalizedNameForm is a beast of complexity, that's why you think it is difficult. I will just do it quickly.

@westnordost westnordost merged commit 247fb80 into streetcomplete:master May 18, 2020
@RubenKelevra
Copy link
Contributor

What street does the (house) number %s belong to

I think the title could be a bit more descriptive of what the user is expected to do. I was used to the 'select one from the list' with an arrow on the right of the street sign and was a bit surprised that I'm supposed to tap on a street on the map.

Maybe something like this?

Tap on the street which this house number %s belongs to.

@westnordost
Copy link
Member

Pretty much that text is visible there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quest for missing addr:street
4 participants