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

Incorrect tactile paving option for crossings #3844

Merged
merged 4 commits into from
Mar 6, 2022

Conversation

arrival-spring
Copy link
Contributor

Fixes #3813

As the same form is used for all tactile paving quests, showing the incorrect option only for nodes with highway=crossing or highway=traffic_signals (in the same way as the steps answer option in path quests) seemed like the best way of doing it.

Copy link
Member

@matkoniecz matkoniecz left a comment

Choose a reason for hiding this comment

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

tested, works fine (is not appearing for kerb quests, as expected)

And big thanks for implementing it!

@westnordost
Copy link
Member

westnordost commented Mar 5, 2022

I was about to comment about that, because answering "on one side only" does not make sense when the app asks about just one kerb or just a bus stop. It is confusing (to read) to have such an answer option when it is not used.

Please instead just create a TactilePavingCrosswalkForm and leave the old TactilePavingForm as is. It may actually be less code this way.

@arrival-spring
Copy link
Contributor Author

Fair enough

Tested and working as expected

Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

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

It is actually less code than before :-)

Thank you for doing this change!

@arrival-spring
Copy link
Contributor Author

Thanks for doing the shortening, could have sworn I'd already done that!

@westnordost westnordost merged commit 9e4d8c1 into streetcomplete:master Mar 6, 2022
@arrival-spring arrival-spring deleted the incorrect-tactile branch March 6, 2022 22:49
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.

Allow tagging tactile_paving=incorrect for when only one side of a crossing has it
3 participants