Skip to content

add colour field type#38

Merged
tyrasd merged 1 commit intoideditor:mainfrom
k-yle:colour
Sep 29, 2022
Merged

add colour field type#38
tyrasd merged 1 commit intoideditor:mainfrom
k-yle:colour

Conversation

@k-yle
Copy link
Copy Markdown
Collaborator

@k-yle k-yle commented Jan 24, 2022

Closes #26

Follow-up from openstreetmap/iD#8782 (comment)

This means we can more reliably detect colour fields (e.g. building:colour=*, roof:colour=*), rather than the current logic in iD which would erroneoulsy match tags like seamark:buoy_cardinal:colour_pattern=*

@tyrasd tyrasd added this to the v5 milestone Jan 28, 2022
@tyrasd tyrasd added the enhancement New feature or request label Jan 28, 2022
tyrasd
tyrasd previously requested changes Feb 17, 2022
Copy link
Copy Markdown
Collaborator

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

Thanks!

Could you please also add the new field type to the documentation on the readme?

@tyrasd
Copy link
Copy Markdown
Collaborator

tyrasd commented Feb 18, 2022

Ah, one more thing. I overlooked that you chose the British spelling colour while one other field (localized) uses American spelling. I think it would probably be better if we remain consistent in that regard, wouldn't it?

//edit: Ah, wait. localized is also fine in British English. I must have remembered that one the wrong way around. Let's keep the spelling as is then. FWIW, the colour field in the presets repo also uses the BE spelling in the file name: https://github.com/openstreetmap/id-tagging-schema/blob/main/data/fields/colour.json

@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Aug 1, 2022

@tyrasd, this PR is approved, but only you are able to merge. Is there anything holding up a merge, such as needing to migrate id-tagging-schema’s roof:colour field back to the colour field type?

@tyrasd tyrasd merged commit 58da483 into ideditor:main Sep 29, 2022
tyrasd added a commit to openstreetmap/iD that referenced this pull request Oct 18, 2022
@k-yle k-yle deleted the colour branch November 20, 2022 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a colour field type

3 participants