Skip to content

Fix multi-combo field remove and undo buttons not working#12406

Open
RudyTheDev wants to merge 2 commits into
openstreetmap:developfrom
RudyTheDev:multiCombo-delete-fix
Open

Fix multi-combo field remove and undo buttons not working#12406
RudyTheDev wants to merge 2 commits into
openstreetmap:developfrom
RudyTheDev:multiCombo-delete-fix

Conversation

@RudyTheDev

Copy link
Copy Markdown
Contributor

This PR fixes a bug with multiCombo preset fields for sub-keyed values, such as "Fuel Types" (e.g. fuel:diesel), where the "remove" and "undo" buttons would not work/show. This currently affects 19 fields like fuel_multi, payment_multi, diet_multi etc.

Currently:

before base

Pressing the trash can does nothing:

before delete

Making any changes does not show an undo arrow:

before add before manual remove

Fix:

after base

Pressing trash can deletes all the sub-tags and shows undo arrow:

after remove

Modifying key value shows undo arrow:

after modify

Undo arrow is shown when making individual additions (i.e. new key not in original):

after add one

Undo arrow is shown when making individual deletions (i.e. key exists only in original):

after remove one

For all the above, undo now properly restores not just current keys, but also the original keys.

Seems to work for multi-selection fine as well including mixing new/existing elements.

In technical details: this fixes a primary bug in uiField where allKeys() wrapper was not passing tags to presetField.allKeys, thus not accounting for "dynamic" preset keys. Specifically, in uiFieldCombo in .tags where it did tag collection in field.keys = ... for multiCombo variant, this value was never used. The consequence is that tags like fuel:diesel would not be considered by uiField functions, namely remove, revert, isModified and tagsContainFieldKey. Furthermore, this fixes a secondary bug that isModified and revert did not consider that the original entity may have had keys matching the field that no longer exist. In other words, allKeys(_tags) (even with tags given) would not consider the allKeys(original.tags). Since any selected entity triggers either modified flag or adds the keys to revert list, then multi-select also works.

No LLM code, only some auto-complete.

@matkoniecz

matkoniecz commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

npm error 404 Not Found - GET https://registry.npmjs.org/-/npm/v1/attestations/marked@18.0.4 - Not found

maybe close/reopen of PR to try retriggering CI build?

@RudyTheDev RudyTheDev closed this Jun 3, 2026
@RudyTheDev RudyTheDev reopened this Jun 3, 2026
@RudyTheDev

Copy link
Copy Markdown
Contributor Author

maybe close/reopen of PR to try retriggering CI build?

Ah, I didn't know I could do that, thanks for the tip. I assumed I had to push another commit or have a repo member rerun the workflow.

@matkoniecz

Copy link
Copy Markdown
Contributor

have a repo member rerun the workflow.

actually they also would need to close/reopen or push commit (maybe push trash commit and force push previous state)

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