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

✨ [bento][amp-sidebar] Update animations to obey APIs mid-animation #32584

Merged
merged 14 commits into from
Feb 12, 2021

Conversation

krdwan
Copy link
Contributor

@krdwan krdwan commented Feb 10, 2021

Sidebar tracker: #31366


open, close, toggle should update / reverse animations mid-animation

Prior to this change, if close was called while the sidebar was animating open, it would snap to a fully opened state and begin closing. Now, it will begin animating closed from whatever position it was in (so it will reverse from the current position). This can be done multiple times in a single animation by repeatedly calling close, open, close, ... or toggle, toggle, toggle....

Works in both directions: opening and closing.

How it works:

  1. Tracks the animation object (in a state variable) while animating and calls reverse() on it when the sidebar is told to animate in a different direction.

to do:

  • update tests (animation tests will need to be updated)

extensions/amp-sidebar/1.0/sidebar.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar-hooks.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar-hooks.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar-hooks.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar-hooks.js Outdated Show resolved Hide resolved
extensions/amp-sidebar/1.0/sidebar-hooks.js Outdated Show resolved Hide resolved
Copy link
Contributor

@caroqliu caroqliu left a comment

Choose a reason for hiding this comment

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

Had a few questions

extensions/amp-sidebar/1.0/sidebar.js Outdated Show resolved Hide resolved
src/preact/index.js Outdated Show resolved Hide resolved
src/preact/value-ref.js Outdated Show resolved Hide resolved
@krdwan krdwan merged commit 6130ded into ampproject:master Feb 12, 2021
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.

None yet

4 participants