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

Add option to keep metadata in files processed by gatsby-plugin-sharp #10210

Merged
merged 5 commits into from
Dec 7, 2018
Merged

Add option to keep metadata in files processed by gatsby-plugin-sharp #10210

merged 5 commits into from
Dec 7, 2018

Conversation

FineWolf
Copy link
Contributor

Work Done

  • Added an option to keep metadata (ICC profile, EXIF information) from files processed by gatsby-plugin-sharp. The old behavior remains as default.
  • gatsby-plugin-sharp was one of the only plugins who didn't support passing configuration thru gatsby-config.js. The only way to enable MozJpeg was via an environment variable. This also has been normalized while maintaining old behavior as default.

Reasoning

I'm currently rebuilding my website with gatsby. While I was working on my portfolio section, I noticed that many of my pictures were rendering differently in Firefox vs Chrome due to the lack of ICC profile attached to the image (untagged images in most browsers render using the Monitor's RGB profile; Chrome seems to assume the pictures are using sRGB IEC61966-2.1). Also, I tend to like to keep my Copyright EXIF information on my images.

This PR (with no breaking changes to old behavior) allows us to specify if we want to strip metadata in the images or not.

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good; left a few comments!

packages/gatsby-plugin-sharp/src/index.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sharp/src/index.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sharp/README.md Outdated Show resolved Hide resolved
packages/gatsby-plugin-sharp/README.md Outdated Show resolved Hide resolved
@FineWolf
Copy link
Contributor Author

Changes have been made as requested. (cc. @DSchau )

@FineWolf
Copy link
Contributor Author

FineWolf commented Dec 3, 2018

Made a quick change to ensure that our setting is passed on to Pngquant when running imagemin. imagemin-mozjpeg and imagemin-webp do not strip metadata.

@FineWolf
Copy link
Contributor Author

FineWolf commented Dec 6, 2018

@DSchau any news on this PR? Right now I'm relying on a fork for my builds.

@DSchau
Copy link
Contributor

DSchau commented Dec 6, 2018

@FineWolf thanks for the ping. I'll give this another once over, and probably get it merged in. Thanks for your patience!

@DSchau DSchau merged commit 5bc0b36 into gatsbyjs:master Dec 7, 2018
@gatsbot
Copy link

gatsbot bot commented Dec 7, 2018

Holy buckets, @FineWolf — 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. (Currently we’ve got a couple t-shirts available, plus some socks that are really razzing our berries right now.)
  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!

@DSchau
Copy link
Contributor

DSchau commented Dec 7, 2018

Successfully published:

m-allanson added a commit that referenced this pull request Dec 8, 2018
* master: (1421 commits)
  feat(gatsby-image): add onStartLoad prop  (#6702)
  fix(docs): add Ecosystem to docs sidebar, consistency with tutorial sidebar (#10350)
  fix(www): Starters.yaml housekeeping (#10354)
  docs: add ttag starter (#10352)
  docs: document branching (#9983)
  plugin checker initial commit (#7062)
  docs: new starter features is required (#10353)
  docs: migrated line highlighting to highlight-line, highlight-start, highlight-end (#10202)
  Add Birra Napoli to site showcase (#10344)
  Add BetterDocs to site showcase (#10349)
  chore(release): Publish
  Add option to keep metadata in files processed by `gatsby-plugin-sharp` (#10210)
  fix(gatsby): [loki] sync db autosaves (#10212)
  Add Ad Hoc Homework to sites.yml (#10346)
  fix(graphql-skip-limit): declare `graphql` peer dependency (#10305)
  fix(gatsby-plugin-offline): gracefully degrade if appshell isn't precached (#10329)
  Service workers note (#10276)
  fix(docs): link fixes, podcast addition (#10332)
  feat(docs): Create clearer pathways in docs (#9898)
  feat(www): Rename community section to creators (#10312)
  ...
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
…p` (gatsbyjs#10210)

* Add option to keep metadata in files processed by `gatsby-plugin-sharp`

* Since pluginOptions are strictly type-checked, no need for the strict check for false.

Co-Authored-By: FineWolf <[email protected]>

* Changed README.md and index.js based on recommendations from @DSchau

* Spelling corrections in README.md

* Fixed Pngquant not respecting the stripMetadata option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants