greenpeace / planet4

Built on top of Wordpress tech, Greenpeace Planet 4 powers digital platforms to engage with millions and win campaigns around the world.
https://planet4.greenpeace.org
Creative Commons Attribution Share Alike 4.0 International
66 stars 27 forks source link

PLANET-5972 Prevent using PNG images for photographs #135

Closed planet-4 closed 2 years ago

planet-4 commented 3 years ago

We currently allow to use PNG images in a lot of places, but these cannot be (lossy) compressed by Cloudflare so end up being more than 10 times as big for the same image. We can restrict the images that can be chosen by file type so that they can only pick a JPG for blocks that are supposed to show photographs (e.g. carousel).

For a sense of scale of the impact. Just a single png photograph on the page can drop the pagespeed score by more than 10 points out of 100.

Tasks

Reporter: pvincent
Sections: Admin Panel, Block: Carousel Header, Block: Gallery, Performance

Potentially affected repositories: planet4-plugin-gutenberg-blocks

cplank commented 2 years ago

I can work on this!

GP-Dan-Tovbein commented 2 years ago

@cplank Go for it!

comzeradd commented 2 years ago

Hi @cplank :wave: Are you working on this? Do you require any assistance from the team?

oekeur commented 2 years ago

What's the status on this? :)

comzeradd commented 2 years ago

This is one is still open for development

oekeur commented 2 years ago

I'll get this :) WIP: https://github.com/oekeur/planet4-master-theme/tree/PLANET-5972

suzi-greenpeace commented 2 years ago

thanks Oscar!

On Mon, Feb 7, 2022 at 4:45 PM OscarKeur @.***> wrote:

I'll get this :) WIP: https://github.com/oekeur/planet4-master-theme/tree/PLANET-5972

— Reply to this email directly, view it on GitHub https://github.com/greenpeace/planet4/issues/135#issuecomment-1032055048, or unsubscribe https://github.com/notifications/unsubscribe-auth/APR5PARZWZRQ37IFC2UMEM3U2BKR3ANCNFSM4252FAOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- suzi grishpul Planet 4 Product Manager, Greenpeace International pronouns: they/them/theirs location: Denver, CO, USA (GMT/UTC-6)

Book some time with me on Fridays: Office Hours https://calendly.com/suzi-grishpul/office-hours

oekeur commented 2 years ago

Hi!
Sorry for the silence.

Update:

It doesn't seem possible to to restrict the selection of PNG for the gallery block because of the implementation of <MediaPlaceholder>. There seems to be a bug regarding the allowedTypes property? <MediaPlaceholder> doesn't respond as expected to the documented properties. accept should restrict the allowed types to be uploaded (which it does). allowedTypes should restrict the selectable mimetypes.
When using the <MediaUpload> component as done in CarouselHeader MediaUpload does respond to allowedTypes.

We could merge this as-is? That would mean the CarouselHeader does have proper restriction of mime types and the gallery does restrict uploading to jpeg, but is does still allow it to select pngs.. A follow-up could be to refactor the gallery to use the MediaUpload instead of MediaPlaceholder to properly restrict the selection of png's?

Not 100% happy with the Add an informative message for editors. But working with what's available, I think this does solve the problem?
Carousel header media library modal
image Gallery mediaplaceholder
image

comzeradd commented 2 years ago

Hey @oekeur, thanks for looking into that.

Yes, I think it's fine to push that as a first fix. Adding the limitation to Carousel Header it's a big win already, since that block is certainly above the fold and a png image could impact performance much more that Gallery.

comzeradd commented 2 years ago

Merged at https://github.com/greenpeace/planet4-plugin-gutenberg-blocks/commit/d29403bc9c9fcb8a1b01305e85bae11e7c49057f