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

Use SVGs instead of PNGs for interface icons where possible #4775

Closed
wants to merge 22 commits into from

Conversation

hiddewie
Copy link
Contributor

@hiddewie hiddewie commented May 9, 2024

Part of #850

This pull request replaces PNG icons with their minimized SVG variants, to ensure screens with high pixel densities can display the OpenStreetMap website with beautiful graphics.

Split off pull requests:

For each of the icons, I replaced the PNG with the corresponding SVG. The PNG variant has been removed. Also, I simplified and minimized the SVGs for improved compression and less data transfer. This also removes privacy sensitive information from the SVG files.

In all cases, I can create separate pull requests if that makes the review process easier.

Let me know if I can add more changes to this pull requests for the remaining images, or if I should create separate pull requests.

Browser support

I could not find documentation about which browsers are supposed to be supported by the OpenStreetMap website.

Internet Explorer 11 is the only browser that might have problems displaying SVG graphics related to scaling. This browser is no longer supported.

See https://caniuse.com/svg

In particular I removed the srcset for replacing a PNG icon with an SVG icon, because browsers can display SVG icons as source in <img>.

@hiddewie hiddewie marked this pull request as ready for review May 9, 2024 09:33
@HolgerJeromin
Copy link
Contributor

Nice to have minified svg in the repo, but they are minified in the build pipelines since years.

@hiddewie
Copy link
Contributor Author

hiddewie commented May 9, 2024

Nice to have minified svg in the repo, but they are minified in the build pipelines since years.

Yes, I noticed. But the compression is not working well because the SVG icons contain many other elements that are not visible to the user, including raster images.

Local:
image

Currently on https://www.openstreetmap.org:

image

This saves more than 50% transfer size for just the OSM logo.

@hiddewie
Copy link
Contributor Author

hiddewie commented May 9, 2024

Apart from the size of the SVG, many contain information about the users who created the files, including much inkscape metadata, e.g. https://github.com/openstreetmap/openstreetmap-website/blob/master/app/assets/images/osm_logo.svg?short_path=2919c14#L18.

This information is also published to the internet: https://www.openstreetmap.org/assets/osm_logo-4afddaae0230a5a46687fdc751ed256dfdccde144118cb02a7d7960f207a4b92.svg (sodipodi:docname="osm_logo_soft_freds_version.svg" and inkscape:export-filename="/home/fred/bla.png")

@danieldegroot2
Copy link
Contributor

Related #3481

@tomhughes
Copy link
Member

tomhughes commented May 9, 2024

Apart from the size of the SVG, many contain information about the users who created the files, including much inkscape metadata, e.g. https://github.com/openstreetmap/openstreetmap-website/blob/master/app/assets/images/osm_logo.svg?short_path=2919c14#L18.

This information is also published to the internet: https://www.openstreetmap.org/assets/osm_logo-4afddaae0230a5a46687fdc751ed256dfdccde144118cb02a7d7960f207a4b92.svg (sodipodi:docname="osm_logo_soft_freds_version.svg" and inkscape:export-filename="/home/fred/bla.png")

Right but is the answer to that a better compressor, or a better compressor configuration maybe?

@AntonKhorev
Copy link
Collaborator

Why does it have to be one pull request with everything? Surely it's possible to change the OpenID icon without changing the sign up globe illustration.

Some images are simple enough, others are complicated. If you look at globe images in this PR and compare them with what's currently deployed, you'll be able to see some differences.

@AntonKhorev
Copy link
Collaborator

See #4671 (comment) for avatar images. Have you tested your changes with Gravatar?

@AntonKhorev
Copy link
Collaborator

See https://caniuse.com/svg

Also see https://www.caniemail.com/search/?s=svg if you're changing the mailer output.

@hiddewie
Copy link
Contributor Author

hiddewie commented May 9, 2024

Also see https://www.caniemail.com/search/?s=svg if you're changing the mailer output.

Good point, I will revert the e-mail related changes and keep the PNGs there.

@hiddewie
Copy link
Contributor Author

hiddewie commented May 9, 2024

Why does it have to be one pull request with everything? Surely it's possible to change the OpenID icon without changing the sign up globe illustration.

I mentioned this in the pull request:

In all cases, I can create separate pull requests if that makes the review process easier.

I can split the pull request per image if the reviewers/maintainers prefer that.

@hiddewie
Copy link
Contributor Author

hiddewie commented May 9, 2024

See #4671 (comment) for avatar images. Have you tested your changes with Gravatar?

The default Avatar does not work for me locally using the master branch either. It generates a URL like http://www.gravatar.com/avatar/2fbaab606d286af6087830ec8208bee5.jpg?s=100&d=http%3A%2F%2Flocalhost%3A3000%2Fassets%2Favatar_large-54d681ddaf47c4181b05dbfae378dc0201b393bbad3ff0e68143c3d5f3880ace.png which is not served by Gravatar.

I tried manipulating the URL to use the published SVG from the current website, and indeed it seems that Gravatar does not like SVGs....

image

Good call, I will revert the Gravatar changes as well...

@AntonKhorev
Copy link
Collaborator

Looks like there was no attempt to convert app/assets/images/confirm-illustration.png.

It would be nice if all of those globe images were cropped to 200px height or shorter (this is the min height of their container in css) and ~530px width and their bottom and right sides aligned with the image edges.

@hiddewie
Copy link
Contributor Author

I will split off as many of the separate icons into separate pull requests.

It would be nice if all of those globe images were cropped to 200px height or shorter (this is the min height of their container in css) and ~530px width and their bottom and right sides aligned with the image edges.

Yes I can do that

@hiddewie
Copy link
Contributor Author

hiddewie commented May 14, 2024

Split off all sections into separate pull requests:

Closing this one

@hiddewie hiddewie closed this May 14, 2024
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.

None yet

5 participants