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

I2I: Introduce PREVIEW visibility state #37129

Closed
2 of 4 tasks
rcebulko opened this issue Dec 6, 2021 · 6 comments
Closed
2 of 4 tasks

I2I: Introduce PREVIEW visibility state #37129

rcebulko opened this issue Dec 6, 2021 · 6 comments
Assignees
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request WG: stories

Comments

@rcebulko
Copy link
Contributor

rcebulko commented Dec 6, 2021

AMP PREVIEW visibility state

Overview

First step in enabling an auto-play preview mode for AMP stories, as described in #12815.

PREVIEW mode should default to PRERENDER mode in most cases, but must be privacy-preserving. This means any external resource loading must either happen through the serving domain (ex. via cache rewrite of <amp-video> target) or by delaying loading the component entirely if there's no viable substitute (ex. <amp-analytics>).

Auto-play should be addressed separately while utilizing preview mode. This is to separate the set of preview-related concerns (primarily privacy and loading of external resources) from auto-play functionality (which may have more nuanced changes on how components function).

Approach

  1. Introduce PREVIEW visibility-state enum. Add previewAllowed method to BaseElement which defaults to returning prerenderAllowed. This change makes the PREVIEW visibility-state available and equivalent to PRERENDER.
  2. Update preact/base-element.js (prerenderAllowed), resource.js (prerenderAllowed), scheduler.js (maybeBuild), resource-impl.js (isLayoutAllowed_), and any remaining runtime+test files to handle new state.
  3. Return false from previewAllowed for any components which may load external resources, blocking them in preview for now; later, their implementation can be adjusted to handle preview specifically. Non-AMP tags such as <img> will require special handling (ex. cache rewrite) to avoid external resources, which will be the responsibility of the code/page presenting the story preview. An initial pass-through found the following tags requiring external loads, which should be initially disabled in preview:
    • <amp-analytics>
    • <amp-audio>
    • <amp-gist>
    • <amp-img>
    • <amp-install-serviceworker>
    • <amp-list>
    • <amp-live-list>
    • <amp-pixel>
    • <amp-story-360>
    • <amp-story-auto-analytics>
    • <amp-story-interactive-binary-poll>
    • <amp-story-interactive-poll>
    • <amp-story-interactive-quiz>
    • <amp-story-interactive-results>
    • <amp-story-panning-media>
    • <amp-twitter>
    • <amp-video>
    • <image>
    • <img>
    • <source>
    • <svg>
    • <track>
  4. Members of @wg-stories and @wg-components can separately prioritize updating the listed components to support preview visibility. This may include logic for visibility state changes from hidden --> preview --> prerender --> visible.

Milestones:

  • Introduce new visibility state and add previewAllowed to BaseElement
  • Update runtime code to handle new visibility state
  • Disable preview for external-loading tags/components
  • Expand preview support for listed components

These steps put us in a place where client code can start to display privacy-previews of stories, which a blocklist of unsupported elements.

@newmuis Does this all seem reasonable? It looks to be fairly straightforward, if I'm not missing too much.

@rcebulko rcebulko self-assigned this Dec 6, 2021
@rcebulko rcebulko added INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request WG: stories labels Dec 6, 2021
@gmajoulet
Copy link
Contributor

gmajoulet commented Dec 9, 2021

Thank you for this initial design! To comment on your numeric list:

This change makes the PREVIEW visibility-state available and equivalent to PRERENDER

This sounds good to me. I can think of a few instances that will allow PREVIEW but not PRERENDER (e.g. non first story pages), so that'll work.

and any remaining runtime+test files to handle new state

To call it out specifically, current prerender mode has a prerendering budget (how many elements can be built/laid out), that needs to be lifted for preview.

This list looks correct! But they should all NOT allow prerender, so we shouldn't need to change anything here.

This may include logic for visibility state changes from hidden --> preview --> prerender --> visible

The correct order, with the ways viewers are implemented today, would be prerender -> preview -> visible -> paused.
Today we go from prerender to preview using the ampdoc.whenFirstVisible() promise, some similar mechanism would be very useful.

cc @ampproject/wg-stories

@newmuis
Copy link
Contributor

newmuis commented Dec 14, 2021

@rcebulko is there a way for us to track the work for each of the milestones, given that this effort spans across multiple working groups? This list of features SGTM, but I don't have great visibility into how or when this work is planned

@rcebulko
Copy link
Contributor Author

@rcebulko is there a way for us to track the work for each of the milestones, given that this effort spans across multiple working groups? This list of features SGTM, but I don't have great visibility into how or when this work is planned

With the general approach validated, I'll work on putting together a clearer project plan in the new year.

@rcebulko
Copy link
Contributor Author

rcebulko commented Jan 5, 2022

@newmuis Updated description with milestone checkboxes for now, though it doesn't currently include anything about consumers of the feature (since that doesn't really live in AMP)

@samouri
Copy link
Member

samouri commented Jan 6, 2022

PREVIEW mode should default to PRERENDER mode in most cases, but must be privacy-preserving.

My understanding was that PRERENDER is currently privacy preserving.

@gmajoulet
Copy link
Contributor

Launched

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 WG: stories
Projects
None yet
Development

No branches or pull requests

4 participants