-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 runtime does not honor height constraints correctly for Responsive Layouts #37398
Comments
On #1 I see following warning in the chrome console when the height get's cut off. It seems the moment I do any DOM event, like click Hide/Show it shows the complete module (including the monkey emoji) - |
Screen grab of first issue - first_issue.mov |
Screen grab of second issue - second_issue.mov |
First issueI think this is working as intended and not a bug. By design, AMP forbids content jumping caused by layout shifts. If auto-expanding the amp-list would result in content below the amp-list pushed down, then AMP will not auto-expand it and instead will render the amp-list in its container with the initial size determined by the initial layout you specify on the amp-list. In this case, the amp-list has In your example, as illustrated by the first screen grab, there's some content (text and rectangle images with rounded corners) beneath the bottom of the amp-list. The bottom in this case refers to the initial bottom of the perfect square amp-list container before it auto-expanded itself. The first few refreshes you did in the video that resulted in the amp-list auto-expanding itself were done in one of the following scenarios:
The amp-list is allowed to auto-expand itself In these scenarios because doing that will either not cause any content jumping at all or only cause very minor content jumping that the user won't care about.
In the video, the refresh that resulted in the amp-list not auto-expanding was done when the amp-list doesn't meet any of the conditions above. As a result, the amp-list was not auto-expanded, because it would otherwise result in a visual content jump where the text and rectangles with rounded corners were first displayed at one placed and then suddenly jumped to another when the amp-list loads. This behavior is documented in the amp-list documentation:
https://amp.dev/documentation/components/amp-list/?format=email#specifying-an-overflow The documentation there talks about when the "overflow" element appears, but it's essentially documenting when the amp-list auto-expands itself during initial load of the page. The amp-list auto-expands itself during initial load if and only if the overflow element does not appear. You can specify the overflow element in cases like this. When the user clicks on the overflow element, it will expand the amp-list. The idea here is that content jumping as a result of a user gesture is okay, because the user does expect something to change in the UI after performing some actions. It's only a bad UX when the content jumps without the user gesturing it. This is also why you were able to expand the amp-list by hiding and re-showing the amp-list, as illustrated in the video. Another thing to note here is that amp-list can also auto-expand itself if there's no content beneath it, which is documented in the following section:
https://amp.dev/documentation/components/amp-list/?format=email#displaying-a-dynamic-list This is pretty much the only reliable way to ensure that amp-list auto-expands, since you can't predict the size of the user's viewport beforehand and thus cannot design the UI in a way that always meets the viewport conditions documented above. Second issue/to @samouri Not sure if there's GitHub issue tracking this but I think it's basically b/110198135. OP: I think you can actually work around this issue by switching from |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
Description
It seems the common AMP runtime is not honoring the height property for amp-list (when using amp-template and mustache) under following scenarios -
Sometimes using the code, if you go to http://127.0.0.1:5000/static/test.html
Note in the above module the emoji - see-no-evil-monkey does not shows up
But after a few refresh, the full module shows up with see-no-evil-monkey as the last image as shown below -
ie: After reducing the width of the chrome window to the minimum and expanding back to original size, there is a huge gap at the bottom of the module as show below -
Reproduction Steps
Use the attached code, and follow the instructions in README.md (you would need Python 3.6 and flask as mentioned in README)
Extract the following zip - how_tall_am_i.zip
how_tall_am_i.zip
Relevant Logs
Instructions in README.md (browser based testing)
Browser(s) Affected
Chrome, Firefox, Safari
OS(s) Affected
Mac
Device(s) Affected
Mac
AMP Version Affected
No response
The text was updated successfully, but these errors were encountered: