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

[gatsby-source-contentful] Remove default crop focus #2917

Merged

Conversation

sarahatwork
Copy link
Contributor

First of all, thanks for the great library! Three weeks into a Gatsby project and loving it so far.

On my current project, I ran into an issue where I'm unable to specify that Contentful use its default (center) focus for cropping images. When I don't apply a focus property, faces is applied automatically 😬

This PR removes the default faces crop focus applied to Contentful image queries. The original commit where this content was added mentioned it was necessary to trigger a crop, but I haven't found this to be true. Let's look at the image from the tests. Here's the original:

original

The code on master generates a query like this: http://images.contentful.com/ubriaw6jfhm1/10TkaLheGeQG6qQGqWYqUI/5421d3108cbb699561acabd594fa2cb0/ryugj83mqwa1asojwtwb.jpg?w=450&h=399&q=50&fit=fill&f=faces

focus-faces

When we remove the focus option, we still get a cropped photo, but it's cropped to the center of the image: http://images.contentful.com/ubriaw6jfhm1/10TkaLheGeQG6qQGqWYqUI/5421d3108cbb699561acabd594fa2cb0/ryugj83mqwa1asojwtwb.jpg?w=450&h=399&q=50&fit=fill

no-focus

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 14, 2017

Deploy preview ready!

Built with commit cb1bf35

https://deploy-preview-2917--gatsbygram.netlify.com

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit 05ed07d

https://deploy-preview-2917--using-drupal.netlify.com

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit cb1bf35

https://deploy-preview-2917--using-drupal.netlify.com

@KyleAMathews
Copy link
Contributor

Makes sense! Which commit added the faces fallback!

@KyleAMathews
Copy link
Contributor

Glad to hear Gatsby is working well for ya!

@sarahatwork
Copy link
Contributor Author

@KyleAMathews they were added here: 14bfcb1

@KyleAMathews
Copy link
Contributor

Huh, no idea what I was seeing!

Thanks for the PR!

@KyleAMathews KyleAMathews merged commit 3b40e91 into gatsbyjs:master Nov 15, 2017
@sarahatwork sarahatwork deleted the topics/contentful-default-crop branch November 15, 2017 21:20
@jlengstorf
Copy link
Contributor

Hiya @sarahatwork! 👋

This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here.

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 t-shirts and hats, plus some socks that are really razzing our berries right now.)
  2. If you’re not already part of it, we just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again! 💪💜

@sarahatwork
Copy link
Contributor Author

Thanks @jlengstorf! I claimed some discount codes and have joined the org 🙌

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.

4 participants