Add primaryMimeType and fallbackMimeType props to psammead-image
#4608
Conversation
| {fallbackSrcset && <source srcSet={fallbackSrcset} />} | ||
| <source | ||
| srcSet={srcset} | ||
| {...(primaryMimeType && { type: primaryMimeType })} |
There was a problem hiding this comment.
not sure you need to conditionally spread like this. I think if the value is null then the attribute doesn't get rendered
| const getMimeType = srcset => { | ||
| if (!srcset) return null; | ||
|
|
||
| const firstSrcsetUrl = srcset.split(',')[0].split(' ')[0]; |
There was a problem hiding this comment.
just wondering if this is easier to work out in simorgh
There was a problem hiding this comment.
I think its much of a muchness really. It would probably be a bit more work in Simorgh to pass through a mimeType prop or something similar to the Psammead component, with similar checking done in Simorgh. Conscious not having too much rendering logic in Psammead though.
Moved this over to Simorgh and passed through as props
| case 'gif': | ||
| return 'image/gif'; | ||
| default: | ||
| return null; |
There was a problem hiding this comment.
don't we support the raw extension as well? Are there any other extensions missing here?
There was a problem hiding this comment.
raw is used as an iChef recipe, so something like https://ichef.bbci.co.uk/news/raw/myfile.jpg. It doesn't support the '.raw' file extension as it were.
| const getMimeType = srcset => { | ||
| if (!srcset) return null; | ||
|
|
||
| const firstSrcsetUrl = srcset.split(',')[0].split(' ')[0]; |
There was a problem hiding this comment.
| const firstSrcsetUrl = srcset.split(',')[0].split(' ')[0]; | |
| // ?. captures an edge case where the srcset passed in is null or invalid | |
| const [ firstSrcSet ] = srcset?.split(','); | |
| const [ firstSrcSetUrl ] = firstSrcSet?.split(' '); |
This reads a bit more clearly imo. The main bit is we need to be a bit defensive if the src set is invalid, so null or contains an unexpected format. I believe if the srcset was invalid in this way the page would not render at all with the code as it is; I'd write a test to cover this eventuality.
I'd agree with @jroebu14 that this logic may be better to be in Simorgh, is that where the srcset is constructed; you could pass the mime type in as a prop?
There was a problem hiding this comment.
I'll move this logic over to Simorgh and pass through the mime-types as props for each of the srcsets
amoore108
changed the title
primaryMimeType and fallbackMimeType props to psammead-image
Its probably safe enough to have defaults of |
I've added a conditional check for the primary |
Part of bbc/simorgh#9628
Overall change:
Derives the
<source />mime-type by checking the srcsets of the image. This appears to be required by some older browsers that cannot derive the mime-type themselves from the server response.Code changes:
psammead-imagecanonical component to check thesrcsetandfallbackSrcsetURLs to determine what the mime-type of the respective<source />tag should be set to.