feat(gatsby-remark-images): Fix blur-up preview image not working on initial page load #14816

YoshiWalsh · 2019-06-16T05:59:21Z

Description

This fixes a bug where the blur-up preview image would not work correctly on initial page load, because the image would load before the JavaScript necessary for the effect.

This change enables a simple blur-up effect (without the fade animation) to work even with JavaScript disabled. Once the JavaScript loads, the original fade effect is restored for any images that are still loading.

It's worth noting that this change does hide the alt text by setting the colour to transparent, and uses JavaScript to show the alt text once the image loading is completed (either due to failure or success) so for users with JavaScript disable the alt text will not appear. For this reason I have also made the alt text appear as the title of the image if the image didn't already have a title.

This change should not impact accessibility, as screen readers don't seem to care about the colour of image alt text.

Before:

After:

Related Issues

These changes fix #14325.

Previously this effect only worked for AJAX-based page loads. This change allows the effect to (mostly) work on initial load when the JS isn't yet loaded. If JS is already loaded then the original behaviour is preserved.

This increases the head size by 160 bytes but decreases the size of each image element by 103 bytes. The head is only downloaded on initial page load (not AJAX loads), so if the visitor encounters 2 or more images during their visit then this change was worth it.

YoshiWalsh · 2019-06-17T10:45:29Z

Hi @KyleAMathews , could you provide me some guidance here? I'm having trouble with the tests.

The gatsbygram e2e tests seem to be failing, but the gatsbygram example doesn't use gatsby-remark-images at all, so I can't see how I would've broken it.

I'm also having trouble running the e2e tests locally.

EDIT: After pulling the latest changes from master the tests are now passing. I think they just needed to be re-run, whatever issue was there must've been transient.

YoshiWalsh · 2019-06-30T05:41:34Z

Hey, I've updated my changes to match master, but now some of the tests are failing. I don't think the failures have anything to do with me, they seem transient. Could someone please re-run the CircleCI workflow?

wardpeet

Thanks for fixing this! I've added a small nit. I couldn't update your branch myself so can you also merge master in? This way our tests should work again.

Thank you for your patience and getting this out the door!

wardpeet · 2019-07-04T08:43:18Z

packages/gatsby-remark-images/src/constants.js

@@ -1,3 +1,13 @@
+exports.defaults = {


exports.defaults might be confused with export default so maybe rename it to something verbose like DEFAULT_OPTIONS.

Suggested change

exports.defaults = {

exports.DEFAULT_OPTIONS= {

@wardpeet

As @wardpeet pointed out, `exports.defaults` could be mistaken for `export default`

…sue-14325

wardpeet

looks good, thank you!

YoshiWalsh · 2019-07-04T13:03:00Z

Thanks @wardpeet , checks are passing now. Looking forward to getting this merged! :)

gatsbot · 2019-07-04T13:20:59Z

Holy buckets, @JoshuaWalsh — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

fionawhim · 2019-09-12T18:08:00Z

Is moving the styles to <head> required for functionality, or is it just a potential size optimization?

YoshiWalsh · 2019-09-13T02:32:20Z

@fionawhim It's just an optimisation/cleanliness-improvement. It's not required for the functionality.

Fixes gatsbyjs#17593 This PR reverts a part of gatsbyjs#14816 that moved the image styles into a <style> tag in the head. Per this comment from @JoshuaWalsh, putting the styles in the <head> was just a cleanliness improvement, and did not affect the functionality that gatsbyjs#14816 was addressing: gatsbyjs#14816 (comment) Reverting this PR allows the responsive images to display correctly when the Remark HTML is displayed outside of the Gatsby layout, such as in an RSS feed.

* Moves responsive image CSS back into attribute Fixes #17593 This PR reverts a part of #14816 that moved the image styles into a <style> tag in the head. Per this comment from @JoshuaWalsh, putting the styles in the <head> was just a cleanliness improvement, and did not affect the functionality that #14816 was addressing: #14816 (comment) Reverting this PR allows the responsive images to display correctly when the Remark HTML is displayed outside of the Gatsby layout, such as in an RSS feed. * Updates Jest snapshots for image style changes

* Moves responsive image CSS back into attribute Fixes #17593 This PR reverts a part of #14816 that moved the image styles into a <style> tag in the head. Per this comment from @JoshuaWalsh, putting the styles in the <head> was just a cleanliness improvement, and did not affect the functionality that #14816 was addressing: gatsbyjs/gatsby#14816 (comment) Reverting this PR allows the responsive images to display correctly when the Remark HTML is displayed outside of the Gatsby layout, such as in an RSS feed. * Updates Jest snapshots for image style changes

YoshiWalsh added 2 commits June 10, 2019 15:41

Allow gatsby-remark-image blur-up to work on initial page load

e6c506c

Previously this effect only worked for AJAX-based page loads. This change allows the effect to (mostly) work on initial load when the JS isn't yet loaded. If JS is already loaded then the original behaviour is preserved.

YoshiWalsh requested a review from a team as a code owner June 16, 2019 05:59

YoshiWalsh added 5 commits June 16, 2019 16:13

Fix errors discovered by linter

12c71e6

Update jest tests

22b555e

Merge branch 'master' into topic/issue-14325

56e2b95

Update new jest tests

3127baa

Merge branch 'master' into topic/issue-14325

8ac989d

YoshiWalsh changed the title ~~Topic/issue 14325~~ Fix blur-up preview image not working on initial page load Jun 17, 2019

Merge branch 'master' into topic/issue-14325

ebddc31

lannonbr added the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label Jun 29, 2019

YoshiWalsh added 2 commits June 30, 2019 14:54

Merge branch 'master' into topic/issue-14325

cc02105

Fix whitespace-related test failures

f67a847

wardpeet self-assigned this Jul 4, 2019

wardpeet previously approved these changes Jul 4, 2019

View reviewed changes

wardpeet added status: awaiting author response Additional information has been requested from the author and removed status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response labels Jul 4, 2019

YoshiWalsh added 3 commits July 4, 2019 21:28

Rename defaults to DEFAULT_OPTIONS

88f5265

As @wardpeet pointed out, `exports.defaults` could be mistaken for `export default`

Merge remote-tracking branch 'origin/topic/issue-14325' into topic/is…

46ce822

…sue-14325

Merge remote-tracking branch 'upstream/master' into topic/issue-14325

88161c0

YoshiWalsh dismissed wardpeet’s stale review via 88161c0 July 4, 2019 11:29

Merge remote-tracking branch 'upstream/master' into topic/issue-14325

61f1702

wardpeet approved these changes Jul 4, 2019

View reviewed changes

wardpeet merged commit c4a7c40 into gatsbyjs:master Jul 4, 2019

wardpeet changed the title ~~Fix blur-up preview image not working on initial page load~~ feat(gatsby-remark-images): Fix blur-up preview image not working on initial page load Jul 4, 2019

fionawhim mentioned this pull request Sep 12, 2019

[gatsby-remark-images] Large blank spaces / blurs in RSS feeds #17593

Closed

fionawhim mentioned this pull request Nov 30, 2019

Moves responsive image CSS back into attribute #19888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gatsby-remark-images): Fix blur-up preview image not working on initial page load #14816

feat(gatsby-remark-images): Fix blur-up preview image not working on initial page load #14816

YoshiWalsh commented Jun 16, 2019 •

edited

YoshiWalsh commented Jun 17, 2019 •

edited

YoshiWalsh commented Jun 30, 2019

wardpeet left a comment

wardpeet Jul 4, 2019

wardpeet left a comment

YoshiWalsh commented Jul 4, 2019

gatsbot bot commented Jul 4, 2019

fionawhim commented Sep 12, 2019

YoshiWalsh commented Sep 13, 2019

feat(gatsby-remark-images): Fix blur-up preview image not working on initial page load #14816

feat(gatsby-remark-images): Fix blur-up preview image not working on initial page load #14816

Conversation

YoshiWalsh commented Jun 16, 2019 • edited

Description

Related Issues

YoshiWalsh commented Jun 17, 2019 • edited

YoshiWalsh commented Jun 30, 2019

wardpeet left a comment

Choose a reason for hiding this comment

wardpeet Jul 4, 2019

Choose a reason for hiding this comment

wardpeet left a comment

Choose a reason for hiding this comment

YoshiWalsh commented Jul 4, 2019

gatsbot bot commented Jul 4, 2019

fionawhim commented Sep 12, 2019

YoshiWalsh commented Sep 13, 2019

YoshiWalsh commented Jun 16, 2019 •

edited

YoshiWalsh commented Jun 17, 2019 •

edited