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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃摉 amp-list docs: increase clarity of overflow section #32619

Merged
merged 2 commits into from
Feb 12, 2021

Conversation

samouri
Copy link
Member

@samouri samouri commented Feb 11, 2021

summary

Addresses #32601

Note: I've never heard of this rule The bottom is within 15% of the height of the entire page and within 1000px of the end of the page.

@morsssss
Copy link
Contributor

I'm confused about how this coexists with #32601 ...

@samouri
Copy link
Member Author

samouri commented Feb 11, 2021

Wow. I thought #32601 was an Issue and didn't realize it was actually a PR.

I've made a huge mistake

@zhangsu
Copy link
Member

zhangsu commented Feb 11, 2021

Your version seems better since it also talks about the initial height of the amp-list. so let's go with yours :) I can drop my PR.

extensions/amp-list/amp-list.md Outdated Show resolved Hide resolved
extensions/amp-list/amp-list.md Outdated Show resolved Hide resolved
extensions/amp-list/amp-list.md Outdated Show resolved Hide resolved
- The bottom of `amp-list` is within 15% of the height of the entire page and within 1000px of the end of the page.
- The size specified for the `amp-list` was not large enough to fit its rendered contents.
- The bottom of `amp-list` is within viewport.
- The bottom of `amp-list` is not within 15% of the height of the entire page and within 1000px of the end of the page.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this really... what does "within 15% of the height of the entire page" mean precisely?

I imagine that "within 1000px of the end of the page" means that it can't be >1000px above the end of the page?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its freakin weird. I didn't know about this either:

elementNearBottom_(resource, opt_layoutBox, opt_initialLayoutBox) {
const contentHeight = this.viewport_.getContentHeight();
const threshold = Math.max(contentHeight * 0.85, contentHeight - 1000);
const box = opt_layoutBox || resource.getLayoutBox();
const initialBox = opt_initialLayoutBox || resource.getInitialLayoutBox();
return box.bottom >= threshold || initialBox.bottom >= threshold;
}
.

Essentially, if the element "is near bottom" of the document then it is eligible for resize. And near bottom is defined as "below the top 85% OR within the last 1000px of the doc".

Copy link
Member Author

@samouri samouri Feb 12, 2021

Choose a reason for hiding this comment

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

Can reword to: "The bottom of amp-list is not near the bottom of the page (defined as being in the top 85% of the document and not in the bottom 1000px)."

馃檧

Copy link
Member Author

Choose a reason for hiding this comment

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

Still wordsmithing 馃

Copy link
Member Author

Choose a reason for hiding this comment

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

giving up

Copy link
Contributor

@morsssss morsssss left a comment

Choose a reason for hiding this comment

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

This is brilliant!

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

5 participants