Skip to content

✨ [Amp story panning media] Support portrait images#36053

Merged
processprocess merged 9 commits intoampproject:mainfrom
processprocess:panning-media
Sep 15, 2021
Merged

✨ [Amp story panning media] Support portrait images#36053
processprocess merged 9 commits intoampproject:mainfrom
processprocess:panning-media

Conversation

@processprocess
Copy link
Copy Markdown
Contributor

Adds support for portrait images by checking if the aspect ratio of the container is less than the image.
Updates example template.

Demo
Fixes #31515

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Sep 13, 2021

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story-panning-media/0.1/amp-story-panning-media.css
extensions/amp-story-panning-media/0.1/amp-story-panning-media.js
extensions/amp-story-panning-media/0.1/test/test-amp-story-panning-media.js

Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski left a comment

Choose a reason for hiding this comment

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

Looks good.

Just a note that the image looks like the image follows the cover CSS logic for background images (in terms of cover vs contain), so we could explain that in the documentation. A visualization would help as well understand this behavior.

@processprocess
Copy link
Copy Markdown
Contributor Author

Looks good.

Just a note that the image looks like the image follows the cover CSS logic for background images (in terms of cover vs contain), so we could explain that in the documentation. A visualization would help as well understand this behavior.

Good call. I'll update the docs with behavior and a diagram.

@processprocess processprocess merged commit 41d08bb into ampproject:main Sep 15, 2021
@processprocess processprocess deleted the panning-media branch September 15, 2021 18:23
caroqliu pushed a commit to caroqliu/amphtml that referenced this pull request Sep 16, 2021
* Handle container aspect ratio.

* Update template.

* Measure element instead of page.

* Remove whitespace.

* Revert template change.

* Update test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Panning media] Handle base zoom of aspect ratio wider than image

4 participants