这是indexloc提供的服务,不要输入任何密码
Skip to content

🐛 [Story animations] Center vertical panning #38017

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

Merged
merged 24 commits into from
Apr 5, 2022

Conversation

mszylkowski
Copy link
Contributor

@mszylkowski mszylkowski commented Apr 5, 2022

Fixes: #35016

The vertical panning used to align the images too much to the left, so portrait images would be placed outside of the page.
We need to place them centered vertically, for which I updated the offsetX to match the distance we want.

I also updated the images in the demo file to be higher resolution and be more symetric so we can tell the positioning better.

Before / After:

cc @danielstockton

@mszylkowski mszylkowski self-assigned this Apr 5, 2022
@amp-owners-bot
Copy link

amp-owners-bot bot commented Apr 5, 2022

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

extensions/amp-story/1.0/animation-presets.js
extensions/amp-story/1.0/test/test-full-bleed-animations.js

@processprocess
Copy link
Contributor

Awesome. It's especially noticeable with the pan-scaling-factor attribute.
After:
Screen Shot 2022-04-05 at 12 33 21 PM

Before:
Screen Shot 2022-04-05 at 12 37 39 PM

@mszylkowski mszylkowski merged commit d0dede1 into ampproject:main Apr 5, 2022
@mszylkowski mszylkowski deleted the centerVerticalPanning branch April 5, 2022 19:31
@jingfei
Copy link
Contributor

jingfei commented Apr 6, 2022

+1, thank s for the fix!

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.

[Amp story] [Animation presets] pan-down, pan-up portrait ratio image position not displaying correctly
4 participants