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

replace cycleway with flexible directionalCombo field type #79

Closed
tyrasd opened this issue Dec 12, 2022 · 3 comments · Fixed by #83
Closed

replace cycleway with flexible directionalCombo field type #79

tyrasd opened this issue Dec 12, 2022 · 3 comments · Fixed by #83
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@tyrasd
Copy link
Collaborator

tyrasd commented Dec 12, 2022

See openstreetmap/iD#9423 for more details and openstreetmap/id-tagging-schema#674 for a possible first application of this new field.

@tyrasd tyrasd added the documentation Improvements or additions to documentation label Dec 12, 2022
@tyrasd tyrasd added this to the v6 milestone Dec 12, 2022
@tyrasd
Copy link
Collaborator Author

tyrasd commented Dec 12, 2022

One quite popular "directional" tag are maxspeed:<direction> with ~200k uses, but in order to support these also, the would need to be extended further, as the maxspeed field currently uses the special roadspeed field type.

Perhaps directional should rather be an option which could be used with different field types?

@tordans
Copy link
Collaborator

tordans commented Dec 28, 2022

…as the maxspeed field currently uses the special roadspeed field type

Just for those of us, wo wondered what that looked like:

image

@tordans
Copy link
Collaborator

tordans commented Dec 28, 2022

One quite popular "directional" tag are maxspeed:<direction> with ~200k uses, but in order to support these also, the would need to be extended further, as the maxspeed field currently uses the special roadspeed field type.

I have a slightly different take: Exposing those tags is more important, than the fact that they have their own special field type. Why? If they are hidden, the chance of miss-tagging is higher than having them visible but a less convenient editing experience.

Perhaps directional should rather be an option which could be used with different field types?

Looking at the fields features (https://github.com/ideditor/schema-builder#fields) that would require an additional layer / hierarchy, right?

Adding to my comment above, I would vote for first refactor the cycleway feature to be used with parking. And only then look into solving the maxspeed case. No need to do it all at once… — Update: I see you merged openstreetmap/iD#9423, so that comment is out of date :-).

Back on topic:

We could also turn this round:

  • The directionCombo is a special field
  • And a field can have an additional, optional property of unit or format, which would be format: "speed" | "height" | "colour".
    One could even move more of the type values to the format like "text" | "number" | "tel" | "url"

What I don't like is, that the HTML "type" attribute has values like "tel", "color"; the current schema fits more into this pattern.

Another take:

Staying with our idea, to make add add a wrapperType: "directional" (or something like this). The "localized" field could be moved into this pattern as well. That would allow us to add "description:de" as a type="textarea" + wrapperType="localized".

tordans added a commit to tordans/schema-builder that referenced this issue Dec 28, 2022
tordans added a commit to tordans/schema-builder that referenced this issue Dec 28, 2022
tordans added a commit to tordans/schema-builder that referenced this issue Dec 28, 2022
Rename `cycleway` to `directionalCombo`

Following the update in iD (https://github.com/openstreetmap/iD/pull/9423/files#diff-b3604385ed7e095c6c6fd1973a4c3d376f21e56f26452299a358d033411a38f6R65-R67) this changes the name of the field to make it reusable.

Fixes ideditor#79
tyrasd added a commit that referenced this issue Jan 20, 2023
* `key` is for the _common_ version of the tag (e.g. `*:both`)
* `keys` is for the _directional_ subtags (e.g. `*:left` / `*:right`)

refs #79, #83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants