Add field sidewalk on highway presets#1507
Conversation
|
🍱 Your pull request preview is ready Please use this preview to check your changes. Ideally use the test documentation template and document your test results by commenting on the PR. This will speed up the review process for everyone. FYI, once this PR is merged, you can use the iD Editor Preview to test your changes in interaction with all other changes. |
Test-DocumentationPreview links & Sidebar Screenshots
SearchAll good: Info-
|
|
a3009de to
99de31c
Compare
|
Thanks @Dimitar5555 I just pushed some changes to change the order and labe.
This is now better with the change wording. I don't see a lot we can do here. It would be something to change in iD but I think the UI is fine as is. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Great—thanks for the heads-up! :) Are |
my understanding is that those should never have sidewalk… However I did not analyse existing data (maybe per country). We do not want to end up with spammy „no“s when the no is the clear default. |
Here is a
Wouldn't "hiding" the field in |
Co-authored-by: Máté Gyöngyösi <gyongyosimate1@gmail.com>
34880eb to
41ef739
Compare
Agreed, I just added it. (The |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Máté Gyöngyösi <gyongyosimate1@gmail.com>
Co-authored-by: Máté Gyöngyösi <gyongyosimate1@gmail.com>
|
Thanks a lot, LGTM! |
| "charge_toll", | ||
| "covered_no", | ||
| "cycleway", | ||
| "sidewalk", |
There was a problem hiding this comment.
and say tertiary is inheriting from primary fields
There was a problem hiding this comment.
Right, we only need to change this one place; it follows the same pattern that cycleway has.
This comment was marked as resolved.
This comment was marked as resolved.
|
left/right and way direction is deeply nonobvious but I have no great ideas how to better handle it maybe have a visual GUI like StreetComplete has? but it is not exactly so intuitive maybe show sidewalk sides on map if user edits these fields? but I have no idea how to code it |
It's a bit of a bummer that it does not handle this more gracefully. It's still the most used tagging version according to the raw usage numbers: 🤔 What we would need here is a way to define "equivalent" tagging variants, e.g. @tordans do you think the following would be a good approach to tackle this: 1) implement hardcoded tag-equivalents as outlined in the paragraph above, 2) release iD, 3) merge this PR, 4) release tagging schema with sidewalk field, 5) discuss more generic handling of such cases in schema-builder repo. What do you think? |
This is what openstreetmap/iD#10935 would add, right? (I might be missunderstanding it…) |
By the way, in terms of length, the subkeys are about 40% as common as the values, so not as stark as the raw way counts:
|
|
@1ec5, worth noting the trend, too - a week and a half later, those queries now return 360,066 km "unsided" (~89%) and 162,924 "sided" (102%) (and I did not mass convert them or anything, though maybe someone is?) |
|
openstreetmap/iD@9266707 just appeared in iD would closing/reopening PR to trigger rebuild would deploy preview with it included? Or is it taking latest released iD? |
|
Yes, it pulled latest iD commit from master branch, so it can be tested properly here. I will try to find some bugs. |
I opened openstreetmap/iD#11950 |
|
Is it just me or is there noticeably 0.5s lag after selecting option from dropdown menu? EDIT: apparently just me, and even for me it stopped happening |
|
one cute part: where single way has say |








Following the long discussion in #454 this PR add the field sidewal.
Supported values:
yes|no|separateAutocomplete is disabled to prevent miss tagging – improving the situation described in #1278
Supported key:
sidewalk|sidewalk:both|sidewalk:left|sidewalk:rightIt will use
sidewalk:bothwhen user change date or add new data based on the evaluation in #454 (comment).The third tagging practice of using the values
left, right, bothwill be supported once openstreetmap/iD#10839 is implemented. Update: See also openstreetmap/iD#10935All those parts have already been discussed in the linked PR.
The main part that is new here is, which presets show the field.
I added the field to all presents, that have the
"cycleway"field today. Just like the cycleway field the sidewalk field is placed in themoreFields. We could do more data crunching and analysis to find out if we would want to move the field to thefieldssection in some cases but I suggest to do this later to keep this PR simple.