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

Add school icons #689

Merged
merged 8 commits into from
Jan 15, 2023
Merged

Add school icons #689

merged 8 commits into from
Jan 15, 2023

Conversation

wmisener
Copy link
Collaborator

@wmisener wmisener commented Jan 14, 2023

Adds school POI icons. Fixes #78, and makes progress on #435.

In #78, a wide variety of US road maps were shown, a sizeable proportion of which displayed schools as some variation of a flat-roofed building topped by a pennant-shaped flag and flagpole. The icon I propose adheres to this design, although there's room for stylistic choice regarding the location, size, and direction of the flag.
poi_school

I colored the icon "infrastructure blue" to adhere to the Contributing Guide, with the specified 2px-wide knockout. In the future, a separate color could be considered, especially in conjunction with an area fill if one is desired.
Location: San Francisco, CA (localhost link)
Screen Shot 2023-01-13 at 10 55 29 PM

Right now I've selected it to appear at zoom 15, the same as hospitals. Browsing around, it may be better for both of these icons to appear even earlier, but a decision can wait until there are more POIs to compare to.
Location: Beverly Hills, CA (localhost link)
Screen Shot 2023-01-13 at 10 53 48 PM

This PR does not render colleges/universities or preschools. It also does not differentiate between different grade levels, as proposed in #78. It's unclear if such differentiation is possible with current tagging practices, but if so, there's probably room in the "building" icon, or to the right of the flagpole, for a letter. There's also no distinction between public/private/religious, though that could also be added in the future, perhaps through inverting the fill or changing the flag to a different symbol.
Location: Salinas, CA (localhost link)
Screen Shot 2023-01-13 at 10 54 16 PM

Also adds a legend entry, which is just "School".
Location: Kingman, AZ (localhost link)
Screen Shot 2023-01-13 at 10 59 37 PM

Interaction with other features:
Location: New York, NY (localhost link)
Screen Shot 2023-01-14 at 12 16 08 AM

This is my first PR, and I think the first PR for POIs after the original (#387), so I hope I haven't missed anything!

@ZeLonewolf ZeLonewolf requested a review from 1ec5 January 14, 2023 18:54
@ZeLonewolf
Copy link
Member

Thanks, this looks great! I appreciate the retro school icon from old maps. Really fits the "Americana" theme in the traditional sense.

I did notice that the knockout on the icon seems to be thicker than that of the other icons, and I wonder if the contributing guidelines need to be tweaked.

Below are the settings I used in Inkscape on the coffee cup, and I want to specifically point out the "Order" section with the six buttons. I'm not sure how I arrived at 2.555 pixels but I'm pretty sure if I'd been more competent about graphic design, I'd have aligned the icon to the grid in a more sensible way:

image

@claysmalley
Copy link
Member

Shields only have a 1px border, so maybe we should stick with a 1px knockout.

Should we use this POI icon for college and university as well?

@ZeLonewolf
Copy link
Member

1px sounds right, so if that 2.5555 were actually 2, that would result in the 1px knockout.

@claysmalley
Copy link
Member

claysmalley commented Jan 14, 2023

It's the other way around. It should be 1px in the file, and it'll show up as 2px at 200% zoom.

My workflow when resizing an icon is to first disable the stroke, scale the icon to the desired dimensions subtracted by 1px, center it, then re-add the 1px stroke.

Screenshot from 2023-01-14 14-15-58

@ZeLonewolf
Copy link
Member

I meant that if you change the overlap order to the top-center option, half of the stroke width is covered by the fill. So my 2.555px stroke was actually 1.25px ish.

@claysmalley
Copy link
Member

You're right, my bad 😄

Workflow is pretty much the same in that case, just use 2px instead of 1px.

@wmisener
Copy link
Collaborator Author

Here's a version with a 1px-wide knockout (i.e., the stroke width is 2px, but the fill covers half of it), does it seem better?
poi_school_1pxknockout

Since shrinking the knockout width left extra room to get to 20px, I made the building square bigger, but kept the flag the same size. I think I like the proportions better now, but don't feel that strongly if others want a bigger flag.

Some screenshots
New York
Screen Shot 2023-01-14 at 11 46 48 AM

Salinas
Screen Shot 2023-01-14 at 11 47 00 AM

Visually, this smaller knockout seems like it's closer to the text and shield knockouts, and less than the cafe and bar ones.

@wmisener
Copy link
Collaborator Author

Should we use this POI icon for college and university as well?

We could. I left them out for now, since a lot of maps differentiate them. A few posted in #78 add a second flag, but there are other icons available, like a mortarboard cap or something. Higher education institutions tend to be more important in a city-wide orientation sense than schools, so they should probably appear at lower zooms as well. Maybe I'll open a separate issue.

@ZeLonewolf
Copy link
Member

It is less thickness for sure than the coffee/bar icons, but based on the discussion above, it sounds like that's because the coffee/bar knockouts are wrong :)

@ZeLonewolf
Copy link
Member

I'm happy leaving higher education for a separate PR/discussion, it doesn't have to be tackled here.

icons/poi_school.svg Outdated Show resolved Hide resolved
@Pengor
Copy link
Member

Pengor commented Jan 14, 2023

Nice work! I'm not sure that the specific feature name (i.e. the name of the school) needs to be shown in the legend though, I think simply showing the icon and "School" is sufficient and saves space

@ZeLonewolf
Copy link
Member

ZeLonewolf commented Jan 14, 2023

Nice work! I'm not sure that the specific feature name (i.e. the name of the school) needs to be shown in the legend though, I think simply showing the icon and "School" is sufficient and saves space

That is a generic capability of the legend, not a specific feature implemented by this PR.

@1ec5
Copy link
Member

1ec5 commented Jan 14, 2023

I'm not sure that the specific feature name (i.e. the name of the school) needs to be shown in the legend though, I think simply showing the icon and "School" is sufficient and saves space

#691

@Pengor
Copy link
Member

Pengor commented Jan 15, 2023

That is a generic capability of the legend, not a specific feature implemented by this PR.

Ah, gotcha, thanks!

src/layer/poi.js Outdated Show resolved Hide resolved
src/layer/poi.js Outdated Show resolved Hide resolved
@ZeLonewolf
Copy link
Member

I believe we also need to make additions to our taginfo file.

@ZeLonewolf ZeLonewolf mentioned this pull request Jan 15, 2023
src/layer/poi.js Outdated Show resolved Hide resolved
src/layer/poi.js Outdated Show resolved Hide resolved
src/layer/poi.js Outdated Show resolved Hide resolved
@wmisener
Copy link
Collaborator Author

I've added colleges, universities, and kindergartens to the PR. I'll open separate issues to track rendering these differently than primary/secondary schools.

I believe we also need to make additions to our taginfo file.

I think I've done this correctly? Someone should probably double check...

@ZeLonewolf
Copy link
Member

I believe we also need to make additions to our taginfo file.

I think I've done this correctly? Someone should probably double check...

There isn't really a way to test it beyond checking taginfo the next day, so generally if it doesn't blow up the linter, all we can do is go by eyeball and post a change later if something is wrong. In any case, if our taginfo JSON has a problem, it's not a big deal if our project isn't listed in taginfo for a day!

@wmisener
Copy link
Collaborator Author

Test renderings:
Preschool: Kigala Preschool (localhost link)
Screen Shot 2023-01-15 at 9 22 50 AM

College: City College of San Francisco (localhost link)
Screen Shot 2023-01-15 at 9 21 31 AM

University: University of California, Los Angeles (localhost link)
Screen Shot 2023-01-15 at 9 20 48 AM

Copy link
Member

@ZeLonewolf ZeLonewolf left a comment

Choose a reason for hiding this comment

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

So many schools!

Thanks for this work, the result looks great. There is going to be some tail work I think around prioritizing POIs and tweaking zoom levels, but I think we need to get more icons on the map before we can really start to do that work. So I think this is good to go.

image

@wmisener
Copy link
Collaborator Author

Preschools are tracked in #693, colleges and universities are tracked in #694.

ZeLonewolf added a commit that referenced this pull request Jan 15, 2023
Based on the discussion in #689, update the contributor guidelines to specify a 1px knockout.
@ZeLonewolf ZeLonewolf merged commit a28ec79 into osm-americana:main Jan 15, 2023
@wmisener wmisener deleted the willkmis_school branch January 15, 2023 20:12
@1ec5 1ec5 mentioned this pull request Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request points of interest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schools
5 participants