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

✨ ♻️ [amp story] [Page attachments] Pre tap animations #34142

Merged
merged 13 commits into from May 4, 2021

Conversation

processprocess
Copy link
Contributor

@processprocess processprocess commented Apr 30, 2021

Context / fixes #33261

Animate pre-tap state of attachment CTA buttons after 1s.
The inline "no image" variation shares animation with the images version and required a structural change, which makes this a good opportunity to refactor.

  • Pre tap animations after 1s delay.
  • Remove animate on landing.
  • Use one template for inline attachments.
  • Make arrow one shape.
  • Active voice and punctuation in comments.
  • Rename template render and build functions.
  • Get win from from node.
  • Shared styles for for label elements.

Demo

@processprocess processprocess requested a review from raxsha May 3, 2021 13:15
@processprocess processprocess marked this pull request as ready for review May 3, 2021 13:15
@amp-owners-bot
Copy link

amp-owners-bot bot commented May 3, 2021

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

extensions/amp-story/1.0/amp-story-open-page-attachment.css
extensions/amp-story/1.0/amp-story-open-page-attachment.js
extensions/amp-story/1.0/amp-story-page-attachment.css
extensions/amp-story/1.0/amp-story-page.js
extensions/amp-story/1.0/test/test-amp-story-page.js

@@ -37,7 +37,7 @@ const CtaAccentElement = {
* @param {!Element} element
* @return {!Element}
*/
export const buildOpenDefaultAttachmentElement = (element) =>
export const buildOldAttachmentElement = (element) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure 'Old' is the right name for this. Technically the old UI is still in use in the case of inline attachments that have no images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

buildOpenInlineAttachmentElement would be the codepath for with and without images with the work in this PR.

The benefit of this is that they can share structure, style and animations.

@processprocess processprocess merged commit ce8fe2d into ampproject:main May 4, 2021
@processprocess processprocess deleted the pre-tap branch May 4, 2021 19:19
rochapablo pushed a commit to rochapablo/amphtml that referenced this pull request Aug 30, 2021
)

* pre-tap animation.

* Comments. Get win from element.

* Animation variables

* Remove whitespace.

* Trigger animations when active.

* Update test. Comments.

* Comment.

* Lint

* Pseudo element.
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.

[Page attachments] Animate all pre-tap UI
3 participants