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] [inline-page-attachments] Open to max of 80% #33423

Merged
merged 28 commits into from Mar 25, 2021

Conversation

processprocess
Copy link
Contributor

@processprocess processprocess commented Mar 22, 2021

Edits apply to mobile view only.

Context / Fixes #32760 & #32761

  • Opens to max of 80% height of screen using a spacer element.
  • Spacer element is used to offset drag-to-open events (so element is immediately visible).
  • Closes when click is outside of attachment content.
  • Adds .5 opacity background behind attachment.

All edits are hidden behind the amp-story-page-attachment-ui-v2 flag

demo pages 1 and 2.
Copy paste the following in the console of the demo page to turn the experiment on:
AMP.toggleExperiment('amp-story-page-attachment-ui-v2');

Mar-23-2021 10-56-35

@amp-owners-bot
Copy link

amp-owners-bot bot commented Mar 22, 2021

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

extensions/amp-story/1.0/amp-story-draggable-drawer.css
extensions/amp-story/1.0/amp-story-draggable-drawer.js
extensions/amp-story/1.0/amp-story.css
extensions/amp-story/1.0/amp-story.js

@gmajoulet
Copy link
Contributor

Thank you for posting a demo!
The one comment I have is that you have to swipe these 20% before the content enters the screen. It's more obvious with the smaller content on page 2.

@processprocess processprocess marked this pull request as draft March 23, 2021 13:44
@processprocess processprocess marked this pull request as ready for review March 23, 2021 18:14
Copy link
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.

Approve with 1 comment

@processprocess processprocess merged commit 90769d4 into ampproject:master Mar 25, 2021
@processprocess processprocess deleted the 80-percent branch March 25, 2021 15:57
rochapablo pushed a commit to rochapablo/amphtml that referenced this pull request Aug 30, 2021
…t#33423)

* Basic implementation.

* Calc using page height var

* Lint fixes. Update template and css.

* Check if parent is page attachment.

* Offset open drag by spacer element height.

* Update comment

* Update comment.

* Use resizeObserver to get reference to spacer ehight.

* Css and querySelecotors.

* CSS targetting.g

* Reset scroll position after close transition.

* Specify close handler.

* Adjust overlay opacity.

* Use closest method.

* Remove css.

* Mark scrollTop as OK.

* Update all draggable drawers.

* Fix merge conflict.

* Fix merge conflict.

* Fix merge conflict.

* rebase

* Remove getPage, remove repeated CSS.

* revert z-index

* Subscribe only if in experiment.

* Lint fix

* Update z-index

* Use insertBefore instead of prepend.
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] [Inline] Open to max of 80% of screen
4 participants