-
Notifications
You must be signed in to change notification settings - Fork 54
Add WebP support to psammead-image component #4606
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👏
Even though you did 99% of the work @amoore108 GitHub won't let me approve because I'm the author of the PR. Has my approval anyway 🙂
Haha no worries! Thanks for the pairing at the start, most of the changes I did were just test fixes. I'll approve it on my end. |
@@ -19,13 +29,15 @@ AmpImg.propTypes = { | |||
sizes: string, | |||
src: string.isRequired, | |||
srcset: string, | |||
fallbackSrcset: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new prop may warrant a doc update to readme.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Part of bbc/simorgh#9628
Overall change:
psammead-image
component to use a<picture>
element with 2 sources. 1 for WebP srcsets and 1 fallback srcset for JPG/PNG.psammead-image
andpsammead-media-player
major versionsCode changes:
<picture>
tag to thepsammead-image
component<amp-img fallback.../>
nested component to handle image format fallback in AMPfallbackSrcset
prop