Skip to content

Add segregated combo box field for segregated surface sub-keys#12332

Draft
RudyTheDev wants to merge 16 commits into
openstreetmap:developfrom
RudyTheDev:surface-segregated
Draft

Add segregated combo box field for segregated surface sub-keys#12332
RudyTheDev wants to merge 16 commits into
openstreetmap:developfrom
RudyTheDev:surface-segregated

Conversation

@RudyTheDev

@RudyTheDev RudyTheDev commented May 12, 2026

Copy link
Copy Markdown
Contributor

This PR implements a new field input widget "segregated combo" for combined use by surface, cycleway:surface and footway:surface.

subprime example prime example

The field is shown and replaces the regular surface field when segregated=yes is set or if either cycleway:surface or footway:surface are already set.

From user's perspective it's still the "same" field, just a different variant that shows extra related values.

Here are the various scenarios:

Scenario Preview
Disabled by default - segregated not specified disabled by default
Disabled on segregated=no disabled on nonsegregated
Enabled on segregated=yes enabled on segregated
Enabled when either sub-key is already set, regardless of segregated value enabled when subkeys
Enabled for multiple selection where every preset supports it if any individual selection would have shown it enabled if any selection supports
Disabled for multiple selection unless every preset supports it disabled for mixed selection

Since this is based on combo box and directional combo box, it should work for RTL by default - I didn't do anything special for it:

RTL

Also, when setting the main value, it shows it as empty sub-key placeholders (similar to how access field does it):

placeholders

I added a bunch of unit tests to check most of these cases and assumptions. In particular, that tag values aren't messed up by the new widget and that widget displays when expected.

It's currently added for "Cycle & Foot Path" and "Cycle & Foot Crossing" presets (though technically "Cycle & Foot Crossing" doesn't use the segregated field for some reason).

This PR implements openstreetmap/id-tagging-schema#205 and supersedes openstreetmap/id-tagging-schema#1998 with a more targeted solution.

This PR relies on schema repo update (ideditor/schema-builder#302) and afterwards requires tagging repo update (openstreetmap/id-tagging-schema#2249).

This PR focuses on cycleway surfaces, but this will potentially similarly work for other fields like smoothness. In fact, this can potentially support stuff like maxspeed:backward and lanes:forward (for example, when lanes=3) and such.

I also encountered a couple minor existing issues that I fixed. See comments below.

AI Disclaimer: I used LLM assistance with various code suggestions. I read and checked any such additions or changes and further refactored and updated them manually. All text like this PR was written by me.

Comment thread css/80_app.css
------------------------------------------------------- */
.form-field-input-access,
.form-field-input-directionalcombo {
.form-field-input-directionalCombo,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While basing this on directionalCombo, I noticed the resulting output has element class form-field-input-directionalCombo but form-field-input-directionalcombo CSS selector name. I wasn't sure if I was missing something obvious here, but a little test with adding color: red; to the selector block showed that it was indeed never applying:

unselected before unselected after

So I renamed it to actually apply. (I guess the alternative is to not append field.type to selector/class name and hard-code it uncapitalized. Or have capitalized variant/type name for all the strings in directional_combo.js.)

As far as I can tell, all this does is makes the caption 1 pixel taller. I am not 100% sure what the block was actually intended to do here.

@tordans

tordans commented May 12, 2026

Copy link
Copy Markdown
Collaborator

@RudyTheDev please rebase on top of current develop, there are messy, accidental commits in here that we removed by force pushing on develop.


Did you see #12005 as well? I was looking into something similar to find a way to expose all kind of sub-tags in a more general way.

Comment thread css/80_app.css
width: 100%;
border-radius: 0;
line-height: 0.95rem;
align-items: center;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While looking at fields with combo box rows, I noticed that the labels were not vertically-aligned. This isn't too bad for access (but which I cannot unsee now) and barely noticeable for directional combos. However while I was adding the custom combo widget, it made a noticeable difference. In any case, I adjusted the selector to make the contents center:

Image Image

This also re-centered the dropdown triangle, which I think is fine (but we can also make the selector super-specific to just the labels). It now (almost) matches regular combo box triangle location.

Comment thread .gitattributes
* text=auto
*.bat eol=crlf
*.js eol=lf
*.ts eol=lf

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also noticed that Linter is expecting *.ts files to have LF line endings, but .gitattributes doesn't actually specify that, so git will default to local line endings (which in my case meant CRLF and made linter complain). In any case, I added it since it looks there have been a bunch of new *.ts files recently.

@RudyTheDev

Copy link
Copy Markdown
Contributor Author

@RudyTheDev please rebase on top of current develop, there are messy, accidental commits in here that we removed by force pushing on develop.

@tordans Oh, urgh, I didn't notice develop had a history rewrite. I deliberately didn't want to make a merge commit, so I didn't pull the origin... I'll try to repush, although I've never done something like this onto a cross-repo PR.

Did you see #12005 as well? I was looking into something similar to find a way to expose all kind of sub-tags in a more general way.

Yes, it's one of the issues/PRs I briefly looked at (I haven't pasted all the links yet here). But it's also a much larger generic solution, so I am not certain about the timescale on it as applicable to this. And as you say, it's about discussing the UX options for long-term generic approach, not an implementation (yet). (If you want, I can leave some comments there.) Whereas I'm focusing more on completed code right now since my original issue has been waiting for a long time. I imagine 12005 could theoretically eventually replace this with a bunch of per-field specification. But even this PR took quite a lot of figuring out in terms of various edge cases. So I'd rather have something working sooner (similar to how directionalCombo has been a real life-changer for sidewalks).

RudyTheDev added 16 commits May 12, 2026 15:23
…visibility to show regular surface field when the segregated version isn't shown
…onfiguration doesn't override linter expectations
…y show when there is a label to fix segregated combo visual glitch
@RudyTheDev

This comment was marked as resolved.

@tordans

This comment was marked as resolved.

@tordans

tordans commented May 12, 2026

Copy link
Copy Markdown
Collaborator

But it's also a much larger generic solution, so I am not certain about the timescale on it as applicable to this. And as you say, it's about discussing the UX options for long-term generic approach, not an implementation (yet). (If you want, I can leave some comments there.)

Yes, please add comments over there.


One advantage of that approach could be, that it does not require changes to the scheme, because it reuses the existing fields and schema for but tags (at least that is the next iteration idea…). Where your change here requires changes to schema-builder and tagging-schema, which both are not trivial to review and implement. Esp. since we just made a schema-builder release today(?).

@tordans

tordans commented May 24, 2026

Copy link
Copy Markdown
Collaborator

I just wondered: Can we use the same UI for this situation:

traffic_sign=DE:*|nil
traffic_sign:forward=DE:*|none
traffic_sign:backward=DE:*|none

Basically for any place, where we have a :forward, :backward and possibly a main tag without postfix. Like maxspeed for example.
(The main tag is often used to encode a value that is more likely used by routers that do not handle the forward|backward)

The UI could look the same. But we would need a different trigger, because there is no segregated=yes that we can use as an indicator. I could be something like an extra Button on the label-row or a +-icon somewhere or small linke below "split forward|backward".

And of course we would need to come up with a more generic name than segregatedCombo and expand ideditor/schema-builder#302 to have logic to describe all three keys.

@RudyTheDev

Copy link
Copy Markdown
Contributor Author

Can we use the same UI for this situation

I guess theoretically. I think 12005 is closer to what you describe than this. The "problem" is that this implementation is not for a generic type of field - it's only for two extra comboboxes with implied parent value fallback, which is a rather specific case that applies only to some tags and requires minimal fiddling with presets. It's the same sort of assumptions as directional combo. In other words, you could eventually generalize it, sure, but it would create a lot of additional logic to make it more generic against all the assumptions - field type, css, fallback, value, labeling, translation, prerequisite, multi-value, multi-selection, tests, etc. Like, I thought briefly how a generic version would work. But at some point, you would just end up implementing 12005. It was way, way out of scope of what I wanted to add. Even making a small schema change is a whole update process across multiple repos. Just trying to figure out all the repos and setting up local testing was a major pain. It's why I made this targeted PR even knowing that generic solutions have already been proposed, discussed and worked on. My concern is that trying to do it all at once will not get either feature done. So personally, I would support eventually implementing something like 12005 that works for every subkey and then enabling and adjusting it to have preset-specific logic. Like a preset saying subkeys = [ { key: forward, type: combo, label: "Forward", etc.... This top-to-bottom design to me sounds more logical than trying to do bottom-to-top generalizing of specific fields. Anyway, I'm kind of repeating what I said on that PR. So I don't think it's impossible, but to put it bluntly: who is implementing it and when is it going to be ready? Because you can have a custom field for (traffic sign) subkey directions done in a few days. But getting a generic version done...

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.

2 participants