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 logic for manually specifying the initial crop rect. #18

Merged
merged 4 commits into from
Apr 26, 2019

Conversation

sparhami
Copy link

@sparhami sparhami commented Mar 20, 2019

This allows developers to specify the cropping rect rather than using the bounds of the <img> element. So now you can do something like:

<div style="overflow: hidden">
  <img src="..." style="position: absolute; top: 0; left: 0; width: 100%; height: 100%; object-fit: cover; transform: scale(2);">
</div>

In this example, you will have something that renders like:

________________
|     ____     |
|    |    |    |
|    |    |    |                    
|     ‾‾‾‾     |
‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾

where the inner rectangle represents the cropping div, and the outer rectangle represents the <img> boundaries. With this, you can zoom in on a portion of an image in a way that object-position does not allow you to. As an alternative, you could use object-fit: none, object-position, and transform: scale, but this does not work well when trying to responsively size images (e.g. using the padding-bottom trick).

Without this change, the animation is off as the image portion escaping the overflow container is visible at the start/end on the first frame of the animation (instead of the crop growing),

A live demo of what this looks like: https://sparhami.github.io/animations/docs/demo/zoom-crop/

@sparhami sparhami requested a review from cathyxz March 20, 2019 01:02
Copy link

@cathyxz cathyxz left a comment

Choose a reason for hiding this comment

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

Maybe it would make more sense to take in a rect instead of a div for the cropping dimensions but otherwise LGTM. nvm that was your demo code, and not your API, LGTM.

@sparhami sparhami merged commit 03fd113 into ampproject:master Apr 26, 2019
@sparhami sparhami deleted the crop-container branch April 26, 2019 22:07
Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

LGTM, nice addition.

sparhami pushed a commit to sparhami/animations that referenced this pull request May 24, 2019
This allows developers to specify the cropping rect rather than using the bounds of the `<img>` element. So now you can do something like:

```html
<div style="overflow: hidden">
  <img src="..." style="position: absolute; top: 0; left: 0; width: 100%; height: 100%; object-fit: cover; transform: scale(2);">
</div>
```

In this example, you will have something that renders like:
```
________________
|     ____     |
|    |    |    |
|    |    |    |                    
|     ‾‾‾‾     |
‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾
```
where the inner rectangle represents the cropping div, and the outer rectangle represents the `<img>` boundaries. With this, you can zoom in on a portion of an image in a way that `object-position` does not allow you to. As an alternative, you could use `object-fit: none`, `object-position`, and `transform: scale`, but this does not work well when trying to responsively size images (e.g. using the `padding-bottom` trick).

Without this change, the animation is off as the image portion escaping the overflow container is visible at the start/end on the first frame of the animation (instead of the crop growing),
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