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.

Sign up for GitHub

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

Jump to bottom

📖 Clarify autoexpansion of amp-lists overflow element #24290

Merged
merged 1 commit into from
Sep 5, 2019
Merged

📖 Clarify autoexpansion of amp-lists overflow element #24290

merged 1 commit into from
Sep 5, 2019

Conversation

dritter
Copy link
Contributor

@dritter dritter commented Aug 29, 2019

This PR clarifies the autoexpansion of the overflow element in amp-list.

I used the following code to figure out the behavior:

<!doctype html>
<html ⚡>
<head>
  <meta charset="utf-8">
  <title>amp-list - Example 2</title>
  <script async src="https://cdn.ampproject.org/v0.js"></script>
  <script custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-0.1.js" async></script>
  <script custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.2.js" async></script>
  <link rel="canonical" href="https://preview.amp.dev/documentation/components/amp-list.example.2.html">
  <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
  <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
  <style amp-custom>
    .list-overflow[overflow] {
      color: red;
      position: absolute;
      bottom: 0;
      left: 0;
      right: 0;
    }
  </style>
</head>
<body>
  <amp-list width="auto"
  height="100"
  layout="fixed-height"
  src="/documentation/examples/api/slow-json/?delay=1000&rand=RANDOM">
  <template type="amp-mustache">
    <div class="image-entry">
      <amp-img src="/static/inline-examples/images/image1.jpg"
    width="450"
    height="300"></amp-img>
  <amp-img src="/static/inline-examples/images/image2.jpg"
    width="450"
    height="300"></amp-img>
  <amp-img src="/static/inline-examples/images/image3.jpg"
    width="450"
    height="300"></amp-img>
      <span class="image-title">{{title}}</span>
    </div>
</template>
  <div overflow
    class="list-overflow">
    See more
  </div>
</amp-list>

  <div style="height: 210vh">
    Mega-Div Top
  </div>
    
  
<amp-list width="auto"
  height="100"
  layout="fixed-height"
  src="/documentation/examples/api/slow-json/?delay=1000&rand=RANDOM">
  <template type="amp-mustache">
    <div class="image-entry">
      <amp-img src="/static/inline-examples/images/image1.jpg"
    width="450"
    height="300"></amp-img>
  <amp-img src="/static/inline-examples/images/image2.jpg"
    width="450"
    height="300"></amp-img>
  <amp-img src="/static/inline-examples/images/image3.jpg"
    width="450"
    height="300"></amp-img>
      <span class="image-title">{{title}}</span>
    </div>
</template>
  <div overflow
    class="list-overflow">
    See more
  </div>
</amp-list>

 <div style="height: 210vh">
    Mega-Div Bottom
 </div>
  
</body>
</html>

cathyxz
cathyxz reviewed Aug 29, 2019
View reviewed changes
extensions/amp-list/amp-list.md
@@ -127,7 +127,7 @@ In `<amp-list>`, you can use the [`items`](#items-optional) attribute to render

### Specifying an overflow

Optionally, the `<amp-list>` element can contain an element with an `overflow` attribute. This element is shown if the AMP Runtime cannot resize the `<amp-list>` element as requested.
Optionally, the `<amp-list>` element can contain an element with an `overflow` attribute. This element is shown if the AMP Runtime cannot resize the `<amp-list>` element as requested, which is the case if the list is inside the viewport. If the list is outside the viewport, it will be automatically expanded.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think this has historically confused a lot of folks including the case where we discussed it to death in #14060 (comment).

Basically how overflow and resizing works is, it will automatically resize under any of the following conditions:

  1. If the bottom of the element requesting a resize is outside the viewport.
  2. If the bottom of the element is within 15% of the height of the entire page AND the bottom of the element is within 1000px of the end of the page.
    Otherwise, it will show the overflow element and resize when the user clicks the overflow element.

/cc @jridgewell to keep me honest (I believe this is how resizing works across all of AMP not just amp-list) and @CrystalOnScript, if this is the case, is there a good place to document this in general and maybe link out from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! Thanks for the link. I'll probably take the weekend off to read all that comments. 😄

But seriously, I second the idea of moving that to a more general place. 👍

cathyxz
cathyxz approved these changes Sep 5, 2019
View reviewed changes
Copy link
Contributor

@cathyxz cathyxz left a comment

Choose a reason for hiding this comment

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

Since this is a fairly strict improvement, I'm going to say let's merge it for now. I will file a follow up issue for documenting overflow in general.

@cathyxz cathyxz merged commit 43bc20a into ampproject:master Sep 5, 2019
@cathyxz cathyxz mentioned this pull request Sep 5, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cathyxz cathyxz cathyxz approved these changes

@CrystalOnScript CrystalOnScript Awaiting requested review from CrystalOnScript

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dritter @cathyxz @googlebot