Skip to content

Update roof shape field to add missing wiki suggestions#1654

Merged
tyrasd merged 1 commit intoopenstreetmap:mainfrom
Shrinks99:roof-updates
Sep 23, 2025
Merged

Update roof shape field to add missing wiki suggestions#1654
tyrasd merged 1 commit intoopenstreetmap:mainfrom
Shrinks99:roof-updates

Conversation

@Shrinks99
Copy link
Copy Markdown
Contributor

Description, Motivation & Context

The OSM wiki has more suggested roof_shape options than are listed, all with excellent icons by @mxdanger. This PR adds all the roof shapes with icons to the schema in preperation for #1589

Links and data

Relevant OSM Wiki links:

Relevant tag usage stats:

Checklist and Test-Documentation Template

Read on to get your PR merged faster…

Follow these steps to test your PR yourself and make it a lot easier and faster for maintainers to check and approve it.

This is how it works:

  1. After you submit your PR, the system will create a preview and comment on your PR:

    🍱 Your pull request preview is ready.
    If this is your first contribution to this project, the preview will not happen right away but requires a click from one of the project members. We will do this ASAP.

  2. Once the preview is ready, use it to test your changes.

  3. Now copy the snippet below into a new comment and fill out the blanks.

  4. Now your PR is ready to be reviewed.

## Test-Documentation

### Preview links & Sidebar Screenshots

<!-- Use the preview to find examples, select the feature in question and **copy this link here**.
     Find examples of nodes/areas. Find examples with a lot of tags or very few tags. – Whatever helps to test this thoroughly.
     Add relevant **screenshots** of the sidebar of those examples. -->

<!-- FYI: What we will check:
     - Is the [icon](https://github.com/ideditor/schema-builder/blob/main/ICONS.md) well chosen.
     - Are the fields well-structured and have good labels.
     - Do the dropdowns (etc.) work well and show helpful data. -->

### Search

<!-- **Test the search** of your preset and share relevant **screenshots** here.
     - Test the preset name as search terms.
     - Also test the preset terms and aliases as search terms (if present). -->

### Info-`i`

<!-- **Test the info-i** for your fields and preset and share relevant **screenshots** here.
     The info needs to help mappers understand the preset and when to use it.
     [Learn more…](https://github.com/openstreetmap/id-tagging-schema/blob/main/CONTRIBUTING.md#info-i)
 -->

### Wording

- [ ] American English
- [ ] `name`, `aliases` (if present) use Title Case
- [ ] `terms` (if present) use lower case, sorted A-Z
<!-- Learn more in https://github.com/openstreetmap/id-tagging-schema/blob/main/GUIDELINES.md#2-design-the-preset -->

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 6, 2025

🍱 Your pull request preview is ready

Please use this preview to check your changes. Ideally use the test documentation template and document your test results by commenting on the PR. This will speed up the review process for everyone.

FYI, once this PR is merged, you can use the iD Editor Preview to test your changes in interaction with all other changes.

@Shrinks99
Copy link
Copy Markdown
Contributor Author

Checked in the editor, everything shows up as expected!

@matkoniecz

This comment was marked as resolved.

@matkoniecz matkoniecz closed this Sep 15, 2025
@matkoniecz matkoniecz reopened this Sep 15, 2025
@matkoniecz

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator

@matkoniecz matkoniecz left a comment

Choose a reason for hiding this comment

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

Pinging @tordans as he merged some mergeable PRs I found.

I got confused a bit that you cannot type in roof shape field to filter possible entries but it is not really within responsibility of this PR.

@Shrinks99
Copy link
Copy Markdown
Contributor Author

Shrinks99 commented Sep 17, 2025

@matkoniecz Pretty sure all that is needed to change that behaviour is change "customValues": true

I also don't think customValues should be true in this case to help standardize roof type data... Rather, the combobox input should still be keyboard searchable when it's false

Copy link
Copy Markdown
Member

@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.

confirmed that the additional options are documented and recently often used

@tyrasd tyrasd added the new-value adds value(s) to existing field label Sep 23, 2025
@tyrasd tyrasd merged commit 67c12c1 into openstreetmap:main Sep 23, 2025
10 checks passed
@Shrinks99 Shrinks99 deleted the roof-updates branch September 23, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-value adds value(s) to existing field

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants