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

Fixes multisize creative centering. #10414

Merged
merged 6 commits into from Jul 18, 2017
Merged

Fixes multisize creative centering. #10414

merged 6 commits into from Jul 18, 2017

Conversation

glevitzky
Copy link
Contributor

Changes made in #10351 caused the creative to resize to the size of the slot rather than the creative, forcing the creative to appear left-top justified rather than centered when occupying a space smaller than the slot.

this.size_ = null;
this.initialSize_ = null;

/** @private {?{width, height}} */
Copy link
Member

Choose a reason for hiding this comment

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

Please specify types 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.

Done.

@glevitzky
Copy link
Contributor Author

This is ready to be merged whenever.

@erwinmombay erwinmombay merged commit 34e218a into ampproject:master Jul 18, 2017
jridgewell pushed a commit that referenced this pull request Jul 18, 2017
* Iframe is resized to size of CREATIVE not SLOT.

* Fixes case where creative size is not returned.

* Expanded on comment.

* Fixed type decl.

* Fixed other type while at it.

* Fixes SRA test.
erwinmombay pushed a commit that referenced this pull request Jul 18, 2017
* Iframe is resized to size of CREATIVE not SLOT.

* Fixes case where creative size is not returned.

* Expanded on comment.

* Fixed type decl.

* Fixed other type while at it.

* Fixes SRA test.
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