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

Create preset for footwear_decontamination #1235

Merged
merged 4 commits into from
Jul 15, 2024
Merged

Conversation

k-yle
Copy link
Collaborator

@k-yle k-yle commented May 25, 2024

A preset was recently added for amenity=bicycle_wash - see #1032.

This PR creates a preset for the similar approved tag man_made=footwear_decontamination.

It is currently limited to Australia and New Zealand, since I'm not aware of anywhere else in the world where these features exist. However, I see that man_made=carpet_hanger (#452) is a global preset, despite only existing in a very small number of countries. So maybe it's okay to make this a global preset?

Copy link

🍱 You can preview the tagging presets of this pull request here.

@tordans
Copy link
Collaborator

tordans commented May 25, 2024

I think starting with presets locally is better. It makes it easier to have a good UX when less relevant options are not present for other countries (and easy to fix later). I would rather think we should limit the other preset to countries that are relevant.

@tordans
Copy link
Collaborator

tordans commented Jun 5, 2024

@k-yle looking at our new draft docs, I wonder if this is merge ready or fits our criteria already. A few thoughts…

@matkoniecz
Copy link
Contributor

However, I see that man_made=carpet_hanger (#452) is a global preset, despite only existing in a very small number of countries.

Maybe man_made=carpet_hanger should become limited?

screen12
screen13

@k-yle
Copy link
Collaborator Author

k-yle commented Jun 6, 2024

Icon: [...]

I've opened rapideditor/temaki#93 to create a dedicated icon, so I suppose this PR needs waitfor-icon a matching icon is missing for this preset

Usage: It's only 160 elements ATM which is really low given #1229 (files). However, its also a regional preset, so that does not have to be a blocker. [...]

It is low, but it's higher than amenity=bicycle_wash when it was added, and that one is a global preset. That's the main reason why I thought this would be acceptable.

Currently, the fallback preset is Man-Made Feature which doesn't really help beginners to find the tag, even I don't map them sometimes, because it's so hard to search the wiki for a specific tag. So a preset would help everyone who maps trails/forests/bush areas

k-yle and others added 3 commits June 11, 2024 11:14
@tordans
Copy link
Collaborator

tordans commented Jun 14, 2024

Is this ready to merge from a technical point of view? @k-yle @tyrasd ?

I resolved conflicts that happened after merging #1237 and updated the icons.json that was generated during one of the npm run commands. The is released on https://rapideditor.github.io/temaki/docs/ so that should work as well.

I am wondering where the updated icon will be fetched. Temaki is not in the package.json of this repo and not in the schema builder repo. So it looks like the consumers of this repo need to handle adding the icon libraries themselves like in id.

So from my point of view this can be merged. Any objections?

@k-yle
Copy link
Collaborator Author

k-yle commented Jun 16, 2024

Is this ready to merge from a technical point of view?

I believe so, but iD will need to update temaki before the next release. I was hoping @dependabot would do it, but the bot seems to limit itself to 5 open PRs. So I've created openstreetmap/iD#10280

I am wondering where the updated icon will be fetched. [...]

Yes, in iD's case, running npm run dist:svg:temaki will bundle every icon from temaki into a single file:
dist/img/temaki-sprite.svg, which is loaded when you open iD

@k-yle
Copy link
Collaborator Author

k-yle commented Jul 14, 2024

Is this ready to merge from a technical point of view?

Now that openstreetmap/iD#10280 is merged, I believe this is ready to be merged

@tordans tordans merged commit 87c39e1 into openstreetmap:main Jul 15, 2024
5 checks passed
@k-yle k-yle deleted the footwear branch July 15, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants