Design Review 2018-01-31 16:30 UTC (AMP-to-AMP linking syntax, reporting JS errors to viewer, amp-list client side filtering, Lightbox 2.0 naming, amp-story autoplay)

[mrjoro]

Time: 2018-01-31 16:30 UTC (add to Google Calendar)
Location: Video conference via Google Hangouts

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

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.

We rotate our design review between times that work better for different parts of the world as described in our design review documentation, but you are welcome to attend any design review. If you cannot make any of the design reviews but have a design to discuss please let mrjoro@ know on Slack and we will find a time that works for you.

[dreamofabear]

• Syntax for A2A linking: AMP-to-AMP (A2A) linking #12496 /cc @danielrozenberg
• Reporting JS errors to viewer: Allow reporting errors via the AMP Viewer #12950 /cc @raywainman
• Client-side filtering with amp-list: Prototype: Client-side filtering w/ amp-list #13028

[cathyxz]

• Naming Lightbox 2.0: Lightbox 2.0: name the component and clean up experimental flags #13051

[newmuis]

• VisibilityState.PREVIEW (for amp-story autoplay mode) Add autoplay mode for amp-story #12815

[mrjoro]

Syntax for AMP-to-AMP linking

• Malte: what about rel="amphtml" since that's really what the rel attribute is for (to say something about the destination)
  • we already have it at the document level so semantically it fits the existing usage
• can the cache add them automatically?
  • that's a separate project, not sure if it'll actually be done because there are some issues
  • this is an opt-in approach and it seems straightforward
• definitely don't call it a2a :)
• why do the URL lookup in the viewer instead of the runtime
  • the viewer facilitates the navigation
  • if it's not an AMP page it errors
• for whole-page opt-in, do we need a way to say to opt out for certain things on the page?
  • Malte: not a huge fan of the default option; maybe could flip the default for special document types
  • if we use rel="amphtml" there would be a natural "noamphtml" (like noreferer)
  • could make it a special per-integration thing instead of adding this attribute, but things in the future may need it
  • for now, let's just use rel attribute, then we can experiment with flipping the default for special use cases (e.g. an AMP flow where most of the pages will be AMP links, you'll have more things that are AMP than non-AMP)
• document level attribute vs. meta tag?
  • meta tags are good for more complex data, attributes can be okay if they're boolean
  • there are some other html attributes
  • Malte and Ali lean towards it being a meta tag
  • perma experiment? probably not a good idea
• Will will take this advice and decide

[mrjoro]

Reporting JS Errors to Viewer

• why the single document restriction?
  • the current use case that motivated this feature
  • error reporting does not go to the document, so okay with lifting this restriction

[mrjoro]

Client-side filtering with amp-list

• instead of allowing the expression to be created dynamically (essentially evaling user input) page author would create a boolean expression that contains all possible filter variations with booleans that toggle whether certain subexpressions are being used, so it's all within a single (potentially big) boolean expression
• you can create these expressions today, so the feature is really adding the filter attribute to use this boolean expression
• maybe sort is a more compelling use case for this?
• more useful when you can slightly overfetch (which you can today); news publishers do this a lot today ("give me the 10 recent articles, but exclude the one I'm looking at")

[mrjoro]

Lightbox v2.0 naming

• this isn't a component, it's attributes you can add to other things; that attribute is called lightbox (but it could be called gallery)
• main objection: there's already amp-lightbox and amp-image-viewer
• lightbox 2.0 implies it's removing an existing option..
  • this is actually a strict superset of amp-
  • want to be able to lightbox more than images (e.g. video)
  • a combination of lightbox attribute and amp-image-lightbox
• currently amp-lightbox and amp-image-lightbox, but there's a separate amp-image-viewer
  • should we just deprecate amp-image-lightbox? we deprecated amp-slides and amp-embed; amp-image-lightbox is used a lot, but deprecation would be automatically switching to this one
• Abby: some feedback during hackathon that people had hard time finding components because we use different names than other libraries
  • gallery explains more what the component does than lightbox
  • probably a good idea to tag the documentation with alternate names to help people find them
  • @sanjsanj suggests adding a "you may like" feature on component documentation to help people find related components on ampbyexample
• amp-lightbox-gallery? do we need lightbox?
• amp-lightbox-gallery vs. amp-carousel? carousel slides between images and gallery is small thumbnails; really this is amp-carousel lightbox
• you could imagine an amp-gallery that's not a lightbox, so lightbox 2.0 should be amp-gallery with lightbox attribute
• we should definitely document "you want to display an image/images, here are all of your options"; @cathyxz will do this
• @cathyxz and @ericlindley-g will discuss the options and decide offline

[mrjoro]

amp-story autoplay mode

• we didn't cover this topic in the design review (sorry @newmuis!), but Jon & @aghassemi
  discussed offline
• seems useful in other situations that embed AMP + places where autoplaying videos/animations should not run
• we may want to change the behavior of other components in the preview mode as well, so we should make sure to document the changes that amp-story has
  • actually the behavior may change based on the embedder as well; the autoplay behavior for amp-story mentioned in the issue (pushing the user automatically through the story) makes sense for some products with feeds, but other products may want different behavior
  • in that case we'd need to have more modes if we went with VisibilityState, so Option 2 (new fragment param/request name) is preferable