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

"cycleway" field: fix multiselections tag corruption + make field reusable #9423

Merged
merged 7 commits into from
Dec 12, 2022

Conversation

tyrasd
Copy link
Member

@tyrasd tyrasd commented Dec 12, 2022

This addresses an important bug fix and makes the field more flexible to use

the bug

Under some (not very obscure or rare) conditions, when the cycleway field was used to edit a multi selection, the operation resulted in a (partial) corruption of the selected entities. For example:

way1: highway=residential + cycleway=no
way2 :highway=residential + cycleway:left=lane

When now setting the "Right side" value to something like shared_lane the result is the following:

way1: highway=residential + cycleway:right=shared_lane + cycleway:left=lane
way2 :highway=residential + cycleway:right=shared_lane + cycleway:left=lane

Note how way 1's "Left side" was silent changed from no to lane. Instead one would expect this:

way1: highway=residential + cycleway:right=shared_lane + cycleway:left=no
way2 :highway=residential + cycleway:right=shared_lane + cycleway:left=lane

This bus was cased because for a multiselection the previous selection only looked at the first selected entity to find out what to do with the not-user-modified tag. Because the common tag vs. left/right tag can be mapped different for different entities in the multiselection, the approach to merge/split/update the tags needs to be made on a per entity basis. The fix for this introduces a new way to perform tag change action: a callback function which is called for each to be modified entity.

the enhancement

This is mostly inspired by openstreetmap/id-tagging-schema#674, which would benefit from a field which can edit a directional (*:forward/backward) tag.

  • Renames the field to directionalCombo (keeping cycleway for backwards compatibility), and replacing hardcoded occurances of the cycleway tag key with the one supplied by the preset field.
  • Gets rid of special handling of the none value: Previously any cycleway:<direction>=none was silently removed from the tags which doesn't make sense for two reasons: a) the documented tag value is no and b) it should be possible to map the absence of a cycleway
  • When both the cycleway and a cycleway:direction tag are present, previously the field used the "common" tag value (i.e. the value of the cycleway tag). Now this ambiguous tagging is represented as “multiple values” in the editor.
  • The field now reuses the combo ui field under the hood, which reduces duplication in the code and brings the field up to par with regular combo fields: features like autosuggestions from taginfo are now available also for this field
  • As a side effect, combo fields can now display fields with strings which include a description hover text in addition to the tag's label.

reducing duplication of code, and brings missing features to the directional version of the field
because the common tag vs. left/right tag situation can be different for different entities in the multiselection, the approach to merge/split/update the tags needs to be made on a per entity basis

this introduces a new way to specify tag changes: a callback function which is called for each to be modified entity
instead of `"strings": { "options": { "<tag-value>": "<translatable-string>", …` it is now also supported to have:

```
"strings": {
    "options": {
        "<tag-value>": {
            "title": "<translatable-string>",
            "description": "<translatable-string>"
        },
        …
```
@tyrasd tyrasd added bug-release-blocker An important bug - let's get this fixed in the next release! enhancement An enhancement to make an existing feature even better labels Dec 12, 2022
@tyrasd tyrasd added this to the 2.24 milestone Dec 12, 2022
@tyrasd tyrasd merged commit 5ef9928 into develop Dec 12, 2022
@tyrasd tyrasd deleted the field-type-directional branch December 12, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-release-blocker An important bug - let's get this fixed in the next release! enhancement An enhancement to make an existing feature even better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant