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

Intent to implement: Iframe resizing #728

Closed
cramforce opened this issue Oct 22, 2015 · 14 comments
Closed

Intent to implement: Iframe resizing #728

cramforce opened this issue Oct 22, 2015 · 14 comments
Assignees
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request

Comments

@cramforce
Copy link
Member

The following applies to both amp-iframe, amp-ad and a future amp-template tag.

We are planning to allow resource resizing in a way that is not disruptive to UX. In this new world

  • iframes that the user interacted with may resize themselves.
  • iframes below the the current viewport may resize themselves.
  • other iframes can show a button to execute the resizing.

This represents an alternative strategy to #571 which is only applicable for some use cases. This implements the remaining feature request in #495

Iframes will be able to resize themselves using this piece of JavaScript.

parent.postMessage({
  sentinel: 'amp',
  type: 'resize',
  width: '400' /* px */,
  height: '500',  /* px */
}, '*');

For amp-template resizing happens automatically when auto-resize is set on the tag and the contents overflow.

The following algorithm determines what actually happens upon resize:

  • If the current window is not visible it will be resized immediately.
  • If the iframe is below the current viewport it will be resized in the next animation frame.
  • If none of the above is true and the user is currently scrolling changes are delayed until they stopped scrolling.
  • If the user previously interacted with the iframe (see paragraph on interaction) the iframe is resized in the next animation frame.
  • If the iframe is currently equal to or ever was equal to document.activeElement the resize is executed in the next animation frame. This covers all cases where the resize was requested due to user action.
  • Otherwise a author stylable element is shown at the bottom of the iframe asking the user to tap it to execute the resize operation.

CC @rudygalfi

@cramforce cramforce added the INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code label Oct 22, 2015
@cramforce cramforce self-assigned this Oct 22, 2015
@banderson623
Copy link

👍

@dvoytenko dvoytenko assigned dvoytenko and unassigned cramforce Oct 27, 2015
@johnduffell
Copy link
Contributor

This sounds a great refinement of the previous behaviour! I was just wondering if this would cope with the situation where

  • an element is expanded for whatever reason
  • you scrolled below it
  • you click a link elsewhere
  • you hit the back button

Will it remember the size of the element when you left the page and make sure you are still in the same place or will the element revert to the placeholder size and bump you down the page slightly?

@cramforce
Copy link
Member Author

@johnduffell It very much depends on the browser and on whether an onunload handler is installed. If we can get the analytics people to not do that, we are good :)

@johnduffell
Copy link
Contributor

great thanks @cramforce

@dvoytenko
Copy link
Contributor

Follow up items: #811 and #812

@cramforce
Copy link
Member Author

Also #814

@coreybyrnes
Copy link

I'm getting an error with this resize implementation:

Uncaught TypeError: Cannot read property 'compareDocumentPosition' of null

I'm also getting an undefined error whenever I try to reference window.context (ex: I tried using window.context.requestResize(300,600) to resize my ad).

Here's my simple demo page for reference:
http://rev.cbsi.com/corey/test/amp/amp_demo_resize.html#development=1

Can anyone assist on either of these issues?

@cramforce
Copy link
Member Author

@coreybyrnes Could you file a new issue including a repro case. I'd not expect resize to ever succeed on your demo page, because we do not allow resizing of ads that are in the middle of the viewport.

@coreybyrnes
Copy link

@cramforce Can you explain further on what the limitations are for ad resize? I'm looking here and don't see any specific rules: https://github.com/ampproject/amphtml/tree/master/ads#ad-resizing

@cramforce
Copy link
Member Author

@coreybyrnes End of that paragraph. I just clarified it slightly.

@coreybyrnes
Copy link

@cramforce Can you confirm some findings of mine that, if true, would be helpful to include in the documentation?

  • if the resize action is triggered by user action it will always be executed
  • if the resize action is triggered upon ad load, it will only be executed if the ad is below or above the viewport
  • if the resize action is triggered upon ad load, it will not be executed if the ad is within the viewport

@dvoytenko
Copy link
Contributor

@coreybyrnes you are correct. This is documented here: https://github.com/ampproject/amphtml/blob/master/extensions/amp-iframe/amp-iframe.md#iframe-resizing

/cc @Meggin to see if this is also documented in docs and maybe needs some work.

/cc @sriramkrish85

@coreybyrnes
Copy link

@dvoytenko you referenced iframe resizing but I'm specifically inquiring about ad resizing and the documentation isn't clear there. I really think it needs to be more specific and re-worded for clarification. Let me know if I can help with the edit.

@dvoytenko
Copy link
Contributor

Makes sense. I opened #2594 to track documentation for ad resizing behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request
Projects
None yet
Development

No branches or pull requests

6 participants