Skip to content

amp-consent iframe ui 100% height#28290

Merged
micajuine-ho merged 3 commits intoampproject:masterfrom
micajuine-ho:ui_Bug
May 11, 2020
Merged

amp-consent iframe ui 100% height#28290
micajuine-ho merged 3 commits intoampproject:masterfrom
micajuine-ho:ui_Bug

Conversation

@micajuine-ho
Copy link
Copy Markdown
Contributor

@micajuine-ho micajuine-ho commented May 8, 2020

Closes #28133.

Based on https://developers.google.com/web/updates/2016/12/url-bar-resizing (This means 100vh will be larger than the visible height when the URL bar is shown...For example, if its height is 100% it will always fill exactly the visible height, whether or not the URL bar is shown.), we should be using 100% instead of 100vh.

Retains the changes made in #20100
ui_bug2

@micajuine-ho
Copy link
Copy Markdown
Contributor Author

/cc @MaximePlancke I believe this fixes your issue, but if you could create a jsbin with an example that is similar to https://sandbox.didomi.io/amp.html, I can test and ensure that it fixes it.

Copy link
Copy Markdown
Contributor

@zhouyx zhouyx left a comment

Choose a reason for hiding this comment

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

Thank you for the investigation and fix!

@MaximePlancke
Copy link
Copy Markdown
Contributor

@micajuine-ho thank you!

Sure, is this ok? https://jsbin.com/vawekakina/1/edit?html,output

I tried on my end with your changes on Xcode and it seems to fix the issue.

@micajuine-ho
Copy link
Copy Markdown
Contributor Author

@MaximePlancke

Tested and it fixes it. Thanks for the jsbin.

@MaximePlancke
Copy link
Copy Markdown
Contributor

Hi @micajuine-ho

Do you know when this change will be part of the stable release?

I saw that it was but has been rolled back 23 days ago.

Thank you!

@micajuine-ho
Copy link
Copy Markdown
Contributor Author

Hi @MaximePlancke this change is in stable. Are you not seeing a fix?

@MaximePlancke
Copy link
Copy Markdown
Contributor

@micajuine-ho

My bad, I see it now. I got confused with the labels.
Thank you!

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.

Incorrect height of amp-consent when navigation bar is displayed

5 participants