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

Undo change from SVG to WEBP #645

Closed
3 tasks
fulldecent opened this issue Nov 30, 2020 · 6 comments · Fixed by #649
Closed
3 tasks

Undo change from SVG to WEBP #645

fulldecent opened this issue Nov 30, 2020 · 6 comments · Fixed by #649

Comments

@fulldecent
Copy link
Contributor

These commits are bad:

7c539d8
f089853
da59354

From @LianaHus @ioedeveloper


They removed vector images in a format that works with all browsers and replaced it with a raster format that only works with some browsers.

Here is how remix currently loads on macOS Big Sur / Safari for me:

Screen Shot 2020-11-29 at 20 08 12


Work plan

  • Create SVG images by decoding the strings at data:image/svg
  • s/.webp/.svg/c
  • Delete those WEBP files
@GrandSchtroumpf
Copy link
Contributor

@fulldecent thanks for keeping a positive discussion on github. We all try to do our best to improve devX.
These commits are not "bad", they try to provide a better experience for most developers. Some background:

webp is a great format recommended to be use on the web. As it happens, Safari and IE don't support webp, and we didn't realized that as nobody in the team are using neither of them.

The right working plan would be to provide a fallback to svg or png for browser that doesn't support webp. Like that users using other browser than Safari or IE can enjoy the benefice of webp images.

@LianaHus
Copy link
Collaborator

@fulldecent
Copy link
Contributor Author

In general, SVG vector graphics are superior to bitmap graphics. They look great on every device/screen/resolution, they minimize file size and they are losslessly editable.

The only times to avoid SVG are:

  • Screenshots (too much detail)
  • Photography (too much detail)
  • Unimportant animations (because most people don't bother to animate SVGs)
  • Ridiculously high level of detail (e.g. a fractal rendered in full precision)

These are the reasons I recommend to revert to using SVG.


Human/voice note: The people mentioned above are good. And the commits were well intentioned. And they even made an improvement (refactoring image from inline to separate file). I am concerned only with the negative impact of the commit mentioned above.

@fulldecent
Copy link
Contributor Author

@GrandSchtroumpf the article you cite https://web.dev/serve-images-webp/ does not match your summary "webp is a great format recommended to be use on the web" in this context.

That article (correctly) only recommends:

WebP is an excellent replacement for JPEG, PNG, and GIF images

It does not recommend replacing SVGs with WebPs.


@LianaHus likewise, the article you cite https://www.macrumors.com/2020/06/22/webp-safari-14/ only compares:

It provides lossy and lossless compression with smaller file sizes as compared to JPEG and PNG files.

Likewise, no recommendation is made for replacing SVGs to be WebPs.

@LianaHus
Copy link
Collaborator

LianaHus commented Dec 9, 2020

@fulldecent your problem should be solved now in remix-alpha can you confirm?
This PR was about moving from base64 to wepb. We don't have svgs of all icons we use and don't want to use external svgs for security reasons.
The article was about adding support to safari.

@fulldecent
Copy link
Contributor Author

Looks good in Safari.


We do have SVGs of all of the icons except unitTesting.webp.

Proof:

Here is the original artwork for localPlugin in SVG format 040876c

Unknown.svg.zip

Attached above ^^


There is no security issue of using SVG files as images that are hosted in this repository.

Or if there is a security issue, then it is surely less that that of hosting HTML in this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants