taiwangoldcard / website

Taiwan Gold Card Community website
https://taiwangoldcard.com
Other
43 stars 18 forks source link

fix: broken banner image #221

Closed ruridge closed 2 years ago

ruridge commented 2 years ago

Hi! 👋 I noticed the banner image on the home page was not loading. I had a look through the history and I think this solves for the requirements but if there's context I'm missing please let me know.

Problem

Shown in Safari but the same happens in Chrome & Firefox too:

problem in safari

The image displays fine in the E2E tests and locally, which makes me think it's possibly a cache issue. (although I haven't confirmed that).

What I did notice was a sub-optimal solution to reducing bandwidth using JS to inject the image on desktop devices. That solution fails on a number of devices (including iPads and laptop/tablet hybrid devices) where the screen is large enough to use the desktop layout.

Proposed Solution

Make use of the native Picture element. This technique is more commonly used to conditionally download an optimal image size based on screen size and pixel density. This solution replaces the default banner image with the smallest image I could find (defined as a Data URL).

This should solve both problems.

Additional image optimisation

As this solution now uses the Picture element I have taken the opportunity to optionally use the more efficient WebP format with a JPG fallback. WebP is supported by Chrome, Firefox, Edge and Safari from version 14.

Results

Large Displays

solution-large-displays

Small Displays

solution-small-displays
fifieldt commented 2 years ago

Thank you so much for this, and the detailed investigation. Taking a look ...

fifieldt commented 2 years ago

Makes sense, and certainly seems like you know what you're doing :) for interest, the compose theme is one we got off the internet, so we're not too familiar with the code in there.

@erickhun can you confirm this is good?