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.

Sign up for GitHub

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

Jump to bottom

fix(gatsby-plugin-sharp): Handle diff duotone settings #35075

Merged
merged 5 commits into from
Mar 14, 2022
Merged

fix(gatsby-plugin-sharp): Handle diff duotone settings #35075

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
891f83b
handle duotone
LekoArts Mar 8, 2022
94b48c9
Merge branch 'master' into duotone-hash-bug
LekoArts Mar 9, 2022
332a5de
update tests
LekoArts Mar 9, 2022
2cfff49
Update index.js.snap
LekoArts Mar 9, 2022
e8c3ff3
incorporate review comments
LekoArts Mar 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/gatsby-plugin-sharp/src/__tests__/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ Object {
"base64": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAMAAAHNUN2pAAAAD1BMVEXugW5lZmbMemybcGl6aWdJY41OAAAACXBIWXMAAAPoAAAD6AG1e1JrAAAAVUlEQVQY062QUQ6AMAhD28L9z2w2RObEH+P7WJquK2TAgEh0SoVfF+GQPk9DC5kvxNLv+QYJ1eBVy0VGxJYJltFfkI/q+87T2mzxQo3pj8rmI6QPGx6CUQCaU8vWagAAAABJRU5ErkJggg==",
"height": 100,
"originalName": "test.png",
"src": "/static/1234/df2e1/test.png",
"srcSet": "/static/1234/df2e1/test.png 1x",
"src": "/static/1234/31820/test.png",
"srcSet": "/static/1234/31820/test.png 1x",
"tracedSVG": undefined,
"width": 100,
}
Expand All @@ -38,15 +38,15 @@ Object {
"aspectRatio": 1,
"base64": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAMAAAHNUN2pAAAAD1BMVEXugW5lZmbMemybcGl6aWdJY41OAAAACXBIWXMAAAPoAAAD6AG1e1JrAAAAVUlEQVQY062QUQ6AMAhD28L9z2w2RObEH+P7WJquK2TAgEh0SoVfF+GQPk9DC5kvxNLv+QYJ1eBVy0VGxJYJltFfkI/q+87T2mzxQo3pj8rmI6QPGx6CUQCaU8vWagAAAABJRU5ErkJggg==",
"density": 72,
"originalImg": "/static/1234/df2e1/test.png",
"originalImg": "/static/1234/31820/test.png",
"originalName": "test.png",
"presentationHeight": 100,
"presentationWidth": 100,
"sizes": "(max-width: 100px) 100vw, 100px",
"src": "/static/1234/df2e1/test.png",
"srcSet": "/static/1234/bd373/test.png 25w,
/static/1234/3a8be/test.png 50w,
/static/1234/df2e1/test.png 100w",
"src": "/static/1234/31820/test.png",
"srcSet": "/static/1234/c6018/test.png 25w,
/static/1234/c752e/test.png 50w,
/static/1234/31820/test.png 100w",
"srcSetType": "image/png",
"tracedSVG": undefined,
}
Expand Down
22 changes: 22 additions & 0 deletions packages/gatsby-plugin-sharp/src/__tests__/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,28 @@ describe(`gatsby-plugin-sharp`, () => {
const result = await fluid({ file, args })
expect(result).toMatchSnapshot()
})

it(`creates two different images for different duotone settings`, async () => {
const testName = `duotone-digest-test`
const firstImage = await fluid({
file: getFileObject(path.join(__dirname, `images/test.png`), testName),
args: {
maxWidth: 100,
width: 100,
duotone: { highlight: `#BBFFE6`, shadow: `#51758D` },
},
})
const secondImage = await fluid({
file: getFileObject(path.join(__dirname, `images/test.png`), testName),
args: {
maxWidth: 100,
width: 100,
duotone: { highlight: `#F1D283`, shadow: `#000000` },
},
})

expect(firstImage.src).not.toEqual(secondImage.src)
})
})

describe(`stats`, () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby-plugin-sharp/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ function calculateImageDimensionsAndAspectRatio(file, options) {
}

function prepareQueue({ file, args }) {
const { pathPrefix, ...options } = args
const { pathPrefix, duotone, ...rest } = args
const options = Object.assign(rest, duotone)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct ? It seems to me like it would "flatten" duotone:

// some example
const args = { foo: `bar`, duotone: { wat: true }, a: 5 }
const { pathPrefix, duotone, ...rest } = args
const TEST = Object.assign(rest, duotone)
// TEST = {foo: 'bar', a: 5, wat: true}

Copy link
Contributor Author

@LekoArts LekoArts Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

options is only used for the digest, args is used for options return so should be fine

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, right - then my suggestions would be to rename options to something else just to indicate it's not really "options" anymore as shape is different (just to avoid confusion as I had above if someone read it some time later). Ideally with some comment with context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pieh Done

const argsDigestShort = createArgsDigest(options)
const imgSrc = `/${file.name}.${options.toFormat}`
const outputDir = path.join(
Expand Down