Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gatsby-remark-images): add disableBgImage option #19152

Merged
merged 7 commits into from Nov 4, 2019
Merged

Conversation

@dsewnr
Copy link
Contributor

dsewnr commented Oct 30, 2019

Description

There's a very long inline style on bgImage. Lots images are possible to exceed the 50,000 bytes limit on AMP.

disableBgImage can be used to prevent "Stylesheet too long" error on AMP

dsewnr added 3 commits Oct 30, 2019
… "Stylesheet too long" error on AMP
… "Stylesheet too long" error on AMP
@dsewnr dsewnr requested review from gatsbyjs/core as code owners Oct 30, 2019
Copy link
Contributor

pieh left a comment

This looks great, let's just do small change to docs and upload missing snapshot file.

You can generate one by running yarn gatsby-remark-images -u in root of repository.

packages/gatsby-remark-images/README.md Outdated Show resolved Hide resolved
@pieh pieh changed the title Add disableBgImage option for gatsby-remark-images package to prevent "Stylesheet too long" error on AMP feat(gatsby-remark-images): add disableBgImage option Oct 30, 2019
dsewnr and others added 2 commits Oct 31, 2019
Co-Authored-By: Michal Piechowiak <misiek.piechowiak@gmail.com>
@dsewnr dsewnr requested a review from pieh Nov 1, 2019
gatsbybot and others added 2 commits Nov 1, 2019
Copy link
Contributor Author

dsewnr left a comment

I don't know what's wrong with my environment. I got the result when I ran yarn workspace gatsby-remark-images test -u.

- style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url('data:image/svg+xml,%3csvg /'MOCK SVG/'%3c/svg%3e'); background-size: cover; display: block;\\"
+ style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url('data:image/svg+xml,%3csvg \\\\'MOCK SVG\\\\'%3c/svg%3e'); background-size: cover; display: block;\\"
@pieh

This comment has been minimized.

Copy link
Contributor

pieh commented Nov 4, 2019

I don't know what's wrong with my environment. I got the result when I ran yarn workspace gatsby-remark-images test -u.

- style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url('data:image/svg+xml,%3csvg /'MOCK SVG/'%3c/svg%3e'); background-size: cover; display: block;\\"
+ style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url('data:image/svg+xml,%3csvg \\\\'MOCK SVG\\\\'%3c/svg%3e'); background-size: cover; display: block;\\"

This might be related to our jest config. We have top level jest configuration, so if in root you run yarn jest gatsby-remark-images -u it will produce snapshot that will pass in our CI and this is what I did to "fix" the snapshot (I assume this is related to some path normalization things we have in our top level jest config). Sorry for providing bad command to run to update snapshot in previous comment (there was missing jest there).

When I run your command, I also got same result, so there's nothing wrong in your environment.

@pieh
pieh approved these changes Nov 4, 2019
Copy link
Contributor

pieh left a comment

Thanks @dsewnr!

@pieh pieh merged commit d3d9020 into gatsbyjs:master Nov 4, 2019
20 checks passed
20 checks passed
Danger All good
Details
Peril All green. Woo!
Details
ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_pipeline Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_long_term_caching Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_structured_logging Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: starters_validate Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node10 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node12 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node8 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_www Your tests passed on CircleCI!
Details
ci/circleci: windows_unit_tests Your tests passed on CircleCI!
Details
cypress: default-group 67 tests passed in 00:29
Details
@gatsbot

This comment has been minimized.

Copy link

gatsbot bot commented Nov 4, 2019

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

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

  1. 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.
  2. 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!

rickiesmooth added a commit to rickiesmooth/gatsby that referenced this pull request Nov 8, 2019
* Add disableBgImage option for gatsby-remark-images package to prevent "Stylesheet too long" error on AMP

* Add disableBgImage option for gatsby-remark-images package to prevent "Stylesheet too long" error on AMP

* Test disableBgImage

* Update gatsby-remark-images snapshot

* Update packages/gatsby-remark-images/README.md

Co-Authored-By: Michal Piechowiak <misiek.piechowiak@gmail.com>

* chore: format

* fix snapshot
blainekasten pushed a commit to blainekasten/gatsby-1 that referenced this pull request Jan 21, 2020
* Add disableBgImage option for gatsby-remark-images package to prevent "Stylesheet too long" error on AMP

* Add disableBgImage option for gatsby-remark-images package to prevent "Stylesheet too long" error on AMP

* Test disableBgImage

* Update gatsby-remark-images snapshot

* Update packages/gatsby-remark-images/README.md

Co-Authored-By: Michal Piechowiak <misiek.piechowiak@gmail.com>

* chore: format

* fix snapshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.