climateaction-tech / branch-theme

The theme used in the branch magazine by climateAction.tech
13 stars 2 forks source link

Fix image appearance during high carbon intensity #32

Closed jacklenox closed 1 year ago

jacklenox commented 1 year ago

There have been several reports of images not displaying correctly when the website is in high carbon intensity mode. Almost every issue I've seen here is caused by new developments with the WordPress block editor interfering with the original code for handling carbon intensity settings. Namely, through the (entirely reasonable) use of the display CSS property.

Switching to using !important priority – as per this PR – appears to fix almost all (if not all) of these issues.

This is merged to staging and can be tested here: https://branch-staging.climateaction.tech/

As far as I've tested it, it causes no interference with existing content.

Update: I've added a further commit which widens the net to ensure images in the new block editor's gallery elements aren't ignored.

Further update – 9 June 2023: After testing with lots of the problematic issue 4 content, I realised a number of problems were persisting. One particular issue of the new gallery block is that it relies heavily on flexbox, which means the dimensions of images it contains cannot be relied upon. This PR therefore now contains an image placeholder generator that can generate replica images of the same dimensions as images waiting to load.

These allow us to effectively simulate how the gallery will be laid out, and give us a pixel-perfect preview before we choose whether to view an image. These placeholder images only cost about a KB of transferred data. This is now live on staging, where this particularly troubling post can now be seen in its fixed state.

Before:

Screenshot 2023-06-09 at 17-53-13 NORCO

After:

Screenshot 2023-06-09 at 17-53-32 NORCO

jacklenox commented 1 year ago

I've worked on this today and updated the main comment above. It's definitely now ready for review, @hanopcan.

jacklenox commented 1 year ago

The only thing I would say is there are very few comments in the code in general, especially the JS. I know we want to refactor that at some point but realistically I don't know when that would be. I'll approve this merge request as I want to get these fixes out there without any further delays, but wanted to raise the issue of lack of code comments.

@hanopcan Thank you, this is a good point. I have made some efforts to add comments and rename certain variables and functions so the code is a little more self-commenting. A lot of what appears to be a change here isn't actually different from before, it has just been moved around, or the level of nesting has changed, or I've added additional line breaks to make things easier to read. I did start adding more commentary to the existing code, but I'm so distressed by how bad it is that I decided to abandon the effort. I'm very keen to start work on refactoring some of this and during that process I'll make it much easier to read and much better commented.