-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[pls review] Fix contentful image handling #4207
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,4 @@ | ||
| /gatsby-node.js | ||
| /fragments.js | ||
| /extend-node-type.js | ||
| /normalize.js | ||
| /fetch.js | ||
| /__tests__ | ||
| /yarn.lock | ||
| /*.js | ||
| !index.js |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| // no-op | ||
| module.exports.schemes = require(`./schemes.js`) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,7 @@ describe(`contentful extend node type`, () => { | |
| width: 450, | ||
| height: 399, | ||
| quality: 50, | ||
| background: `rgb:000000`, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's this for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The background is set when doing cropping operations where the source image does not completely cover the result. See:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah cool 👍 |
||
| }) | ||
| expect(resp.srcSet.length).toBeGreaterThan(1) | ||
| expect(resp).toMatchSnapshot() | ||
|
|
@@ -96,6 +97,7 @@ describe(`contentful extend node type`, () => { | |
| maxWidth: 450, | ||
| maxHeight: 399, | ||
| quality: 50, | ||
| background: `rgb:000000`, | ||
| }) | ||
| expect(resp.srcSet.length).toBeGreaterThan(1) | ||
| expect(resp).toMatchSnapshot() | ||
|
|
@@ -117,6 +119,7 @@ describe(`contentful extend node type`, () => { | |
| width: 450, | ||
| height: 399, | ||
| quality: 50, | ||
| background: `rgb:000000`, | ||
| }) | ||
| expect(resp).toMatchSnapshot() | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| const { | ||
| GraphQLEnumType, | ||
| } = require(`graphql`) | ||
|
|
||
| const ImageFormatType = new GraphQLEnumType({ | ||
| name: `ContentfulImageFormat`, | ||
| values: { | ||
| NO_CHANGE: { value: `` }, | ||
| JPG: { value: `jpg` }, | ||
| PNG: { value: `png` }, | ||
| WEBP: { value: `webp` }, | ||
| }, | ||
| }) | ||
|
|
||
| const ImageResizingBehavior = new GraphQLEnumType({ | ||
| name: `ImageResizingBehavior`, | ||
| values: { | ||
| NO_CHANGE: { | ||
| value: ``, | ||
| }, | ||
| PAD: { | ||
| value: `pad`, | ||
| description: `Same as the default resizing, but adds padding so that the generated image has the specified dimensions.`, | ||
| }, | ||
|
|
||
| CROP: { | ||
| value: `crop`, | ||
| description: `Crop a part of the original image to match the specified size.`, | ||
| }, | ||
| FILL: { | ||
| value: `fill`, | ||
| description: `Crop the image to the specified dimensions, if the original image is smaller than these dimensions, then the image will be upscaled.`, | ||
| }, | ||
| THUMB: { | ||
| value: `thumb`, | ||
| description: `When used in association with the f parameter below, creates a thumbnail from the image based on a focus area.`, | ||
| }, | ||
| SCALE: { | ||
| value: `scale`, | ||
| description: `Scale the image regardless of the original aspect ratio.`, | ||
| }, | ||
| }, | ||
| }) | ||
|
|
||
| const ImageCropFocusType = new GraphQLEnumType({ | ||
| name: `ContentfulImageCropFocus`, | ||
| values: { | ||
| TOP: { value: `top` }, | ||
| TOP_LEFT: { value: `top_left` }, | ||
| TOP_RIGHT: { value: `top_right` }, | ||
| BOTTOM: { value: `bottom` }, | ||
| BOTTOM_RIGHT: { value: `bottom_left` }, | ||
| BOTTOM_LEFT: { value: `bottom_right` }, | ||
| RIGHT: { value: `right` }, | ||
| LEFT: { value: `left` }, | ||
| FACES: { value: `faces` }, | ||
| }, | ||
| }) | ||
|
|
||
| module.exports = { | ||
| ImageFormatType, | ||
| ImageResizingBehavior, | ||
| ImageCropFocusType, | ||
| } |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like something broke this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the outcome of the bugfix for this result
w=450&h=399b0c658cIt returned the original aspect ratio while the resulting one should be returned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhh :-) cool!