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

Refactor updateSize in amp-iframe #3858

Merged
merged 3 commits into from
Jul 7, 2016

Conversation

muxin
Copy link
Contributor

@muxin muxin commented Jun 30, 2016

  • Refactor code that responds to embed-size request into updateSize_ function
  • Only update width and height of amp-iframe if attemptChangeSize successfully
  • Add error log if embed-size request has no width or height value provided
  • Add manual test for amp-iframe resize
    Disallow resizing amp-iframe to below certain threshold #3746

@muxin muxin added this to the Current milestone Jun 30, 2016
if (!this.isResizable_) {
user.warn(TAG_,
'ignoring embed-size request because this iframe is not resizable',
this.element);
return;
}
this.attemptChangeSize(newHeight, newWidth);
Copy link
Contributor

Choose a reason for hiding this comment

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

where did this go?

muxin added 3 commits July 6, 2016 17:14
* Refactor code that responds to embed-size request into updateSize_ function
* Only update width and height of amp-iframe if attemptChangeSize successfully
* Add error log if embed-size request has no width or height value provided
@muxin
Copy link
Contributor Author

muxin commented Jul 7, 2016

I've updated the iframe resize manual test, PTAL

@dvoytenko
Copy link
Contributor

This is LGTM from me, but please wait for the assigned reviewers.

@camelburrito
Copy link
Contributor

LGTM

@muxin muxin added LGTM and removed NEEDS REVIEW labels Jul 7, 2016
@muxin muxin merged commit c1e341c into ampproject:master Jul 7, 2016
ariangibson pushed a commit to Mixpo/amphtml that referenced this pull request Sep 7, 2016
* Refactor updateSize in amp-iframe
* Refactor code that responds to embed-size request into updateSize_ function
* Only update width and height of amp-iframe if attemptChangeSize successfully
* Add error log if embed-size request has no width or height value provided
@muxin muxin deleted the amp-iframe-updatesize branch May 16, 2017 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants