Design Review 2018-01-17 (premutate for amp-bind, predefined transition effects naming, amp-mathml resizing)

[mrjoro]

The AMP Project holds weekly engineering design reviews. We encourage everyone in the community to participate in these design reviews.

Time: Wednesdays @ 1pm Pacific
Location: Video conference via Google Hangouts

If you are interested in bringing your design to design review, read the design review documentation and add a link to your design doc by the Monday before your design review.

When attending a design review please read through the designs before the design review starts. This allows us to spend more time on discussion of the design.

If you are unable to make the 1pm Pacific time due to time zone issues but you have a design you would like to present please make a note of that and we will try to find a time that works.

[josh313]

I'd like to discuss Premutate amp-bind state (#12811)

[aghassemi]

I got couple of things to chat about:

1) Name of an extension for predefined transition effects:

• We like to support simple-to-use transition effects through attributes, e.g.

<div amp-fx-parallax>
<div amp-fx-slide-in>
<div amp-fx-fade-out>
etc...

• Having multiple extensions for each type of effect seems excessive as they all would share lots of the same underlying code.
• What should the name of a single extension be?

1. amp-fx-0.1.js
2. amp-fx-predefined-0.1.js
3. amp-fx-named-effects-0.1.js
4. amp-fx-basic-effects-0.1.js
5. ?

2) amp-mathml resizing. Container or not?

Render math formulas

It can output svg, DOM+CSS or native browser rendering.

What't the right layout for this component and how should resizing work?

Issues:

• Hard to guess the output's height and width.
• Should work 'inline', so width needs to be re-sizable as well.

Options:

1- Support FixedSizedLayouts, and force resize both height and width. (can cause jumps)
2- Support FixedSizedLayouts, and attempt to resize both height and width (can cause hidden overflow if in viewport and author got dimensions wrong)
3- Support Container and force resize (can cause jumps).
4- Support Container and force resize and make render blocking (no jump but impacts perf)

Note: Usage of this component likely won't be significant.

[mrjoro]

Premutate for amp-bind

• namespacing; would run up into the 50 limit

  • need two amp-states to add an overridable and non-overridable
  • depends on how developers use it; overridable is envisioned to not a huge change to the workflow but it's more of a sanity check
  • also some external feedback that 50 operands is not sufficient, and calculation is not very precise, so we may need to revisit this
  • in the example in the issue you have the same key in both overridable and non-overridable, but in practice you wouldn't necessarily have that
  • for developers the message is: write your page as you would, then just mark whatever needs to be overridable as overridable

• timing

  • if it's sent after the UI is viewable it could violate user expectations as the UI changes
  • can we restrict to only apply before the document is viewable?
  • current plan is that it's up to the viewer to not show the iframe until the data is passed in
  • could reject if it's already viewable to force viewers to implement a good UI (that hides iframe); yes
    • Malte: it's fine to just let viewers handle this, since it's never used outside of a viewer context, and avoids us complicating AMP
  • in this case the viewer needs to know when it's done
  • visibility isn't necessarily the right thing, it's a separate lifecycle step; fits more like render delaying extensions, or could be an entirely new concept

• why are updatedKeys / ignoredKeys needed?

  • so developer can respond to things
  • could instead just produce console messages or a more generic error API (there are other failure cases like self reference)

[mrjoro]

Name of extension for predefined transition effects

• should have a single attribute?

  • could simplify discovery
  • amp-fx="parallax" and then configuration specific to the type of fx (amp-fx-config="...")
  • could have multiple effects on the same div, need to allow for that; could use json/comma separated/etc.
  • our API design is all over the place right now, e.g. amp-ads
  • decision: use amp-fx="parallax", for multiple space-separate them (other attributes use spaces or commas, but space is the right way to do it); then have secondary attributes

• filename: amp-fx-presets?

  • amp-fx-flying-carpet has nothing to do with this, and you don't get it by including this js
  • anything except option Add commit hooks #1

[mrjoro]

amp-mathml resizing

• this doesn't really fit into our current thinking of layout; this is a component that has inherent size and there's no need to resize (it can be as big as it wants to be)

• since this is very special purpose, so it's fine to say that by default it has inherent sizing and may cause jumps (Option amp-pixel / ads / iframes: ensure they it's not triggered on preload #3); can document that you should use Container

• Justin: we have a lot of components now that can force a resize (can cause a page jump any time they want)

  • Malte: doesn't feel like it's a problem qualitatively; it's fine to worry about it but there are scenarios when we're okay with it
  • usually the existing ones aren't in the first viewport)
  • a lot of non-ads 3P stuff
  • notice it mostly when there are a lot on the page and you scroll around quickly

• before this exists people would probably use images

• having an attribute that says whether it creates a new block or stays inline would be very useful

  • block by default, have inline attribute (or inline-block in CSS terminology)