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

fix(gatsby-plugin-sharp): ignore incorrect duotone options #28999

Merged
merged 5 commits into from
Jan 25, 2021

Conversation

mxstbr
Copy link
Contributor

@mxstbr mxstbr commented Jan 12, 2021

Right now, if you set duotone with true as you would for grayscale there is no default highlight/shadow. So the image doesn't process and gives a warning: warn [gatsby-plugin-image] Could not read image data file ".json". This may mean that the images in "/src/pages/random.js" were not processed.

This fixes people wrongly specifying duotone without a highlight/shadow by ignoring the option and warning the user that it was ignored.

[ch22572]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jan 12, 2021
@mxstbr mxstbr requested a review from a team January 12, 2021 13:24
Co-authored-by: Matt Kane <matt@gatsbyjs.com>
laurieontech
laurieontech previously approved these changes Jan 12, 2021
@laurieontech laurieontech added topic: media Related to gatsby-plugin-image, or general image/media processing topics and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jan 12, 2021
Copy link
Contributor

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

This only changes the base64 image. The main change needs to be in process-file

@ascorbic
Copy link
Contributor

You can't rely on logging in process-file though, because when running in Cloud you can't see those errors. I think the best bet is to also check in image-data.ts, early on.

@laurieontech
Copy link
Contributor

Oh wow, I thought that is where this was. Wake up me.

ascorbic
ascorbic previously approved these changes Jan 25, 2021
Copy link
Contributor

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

👍

@laurieontech laurieontech added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jan 25, 2021
@mxstbr mxstbr merged commit 298eb31 into master Jan 25, 2021
@mxstbr mxstbr deleted the gatsby-plugin-image/ignore-duotone branch January 25, 2021 15:36
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
…28999)

* fix(gatsby-plugin-sharp): ignore incorrect duotone options

* Update packages/gatsby-plugin-sharp/src/index.js

Co-authored-by: Matt Kane <matt@gatsbyjs.com>

* Update index.js

* add warning for invalid duotone options

Co-authored-by: Matt Kane <matt@gatsbyjs.com>
Co-authored-by: Laurie <laurie@gatsbyjs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes topic: media Related to gatsby-plugin-image, or general image/media processing topics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants