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

Design Review 2019-05-22 15:30 UTC (wg-infra, shadow dom stability, blurry image opt out, separate ads css) #21931

Closed
mrjoro opened this issue Apr 22, 2019 · 7 comments
Assignees
Milestone

Comments

@mrjoro
Copy link
Member

mrjoro commented Apr 22, 2019

Time: 2019-05-22 15:30 UTC (add to Google Calendar)
Location: Video conference via Google Meet

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.

@mrjoro mrjoro added this to the Docs Updates milestone Apr 22, 2019
@mrjoro mrjoro self-assigned this Apr 22, 2019
@mrjoro
Copy link
Member Author

mrjoro commented Apr 26, 2019

The Infrastructure Working Group will be presenting their periodic update at this Design Review (10-15 minutes).

/cc @rsimha @ampproject/wg-infra

@prateekbh
Copy link
Member

prateekbh commented May 7, 2019

I'd like to present our future steps to make shadow DOM more stable and have better testing strategy around the same.

@jridgewell
Copy link
Contributor

jridgewell commented May 21, 2019

I'd like to discuss how we'd like to enable Blurry Image Placeholder for the AMP Cache.

@mrjoro
Copy link
Member Author

mrjoro commented May 22, 2019

wg-infra update

@mrjoro
Copy link
Member Author

mrjoro commented May 22, 2019

shadow runtime

  • @aghassemi: for the most common problem (using the wrong document), start with conformance tests or lint rule?
      - conformance test will be source of truth, want to derive lint rules from those; first step is the conformance tests
      - lint rules aren't blocking
      - build every extension, do a layout, look at window to see if it's calling a bad method?  first let's see if we can use the current infra, otherwise we may have to do this; there are some classes where this might be possible
  • @dvoytenko: on runtime we're working on replacing ampdoc with a different abstraction that's more precise for ads and shadow; goal is to have an interface that scopes things more correctly
  • @dvoytenko: used to be /pwa/ local proxy; this loads whatever AMP page you give it (from examples); the goal here is to load any external parent page with your current local build
  • @jpettitt: there was some bug with two shadow docs live on the same page (Republica?) so this is a case to keep in mind; took a while to debug since on local we couldn't load the parent page
  • @lannka: could be race conditions in some scenarios, so navigation might be tricky to test; @lannka will send details to @prateekbh

@mrjoro
Copy link
Member Author

mrjoro commented May 22, 2019

blurry image placeholder in caches

  • @aghassemi: we are doing a similar thing for loaders (spinners) without opt-out or origin trial; we'll just do this (but that's behavior on the page vs. behavior in the Google AMP cache)
  • @jpettitt: we turned on image converter without an opt out
  • final presentation to users doesn't change, it's the loading behavior that changes
  • @cramforce: no opt out, but we should do a call for opinions before we launched
  • @jpettitt: could opt out with a placeholder
  • @jridgewell: can opt out per image (noloading attribute is respected, the same attribute that says not to show the loader)
  • @aghassemi: should loader go on top of blurry image?  check with Andrew (UX)
  • optimizer is part of toolbox; you could do the same transform yourself (so other AMP caches could do this; this is all well documented)
  • @aghassemi: does signed exchange packager use the optimizer?  not yet

@mrjoro
Copy link
Member Author

mrjoro commented May 22, 2019

separate CSS for amp4ads runtime (#22418)

  • @aghassemi/@kristoferbaxter: all CSS has a dependency on ordering, so we'd need to be careful when splitting css
  • @kristoferbaxter: why not do no splitting (one for runtime and one for ads)
      - if there's no ads runtime, the amp runtime renders the ad, so it needs to contain ads css as well  
  • @dvoytenko: basically element CSS and document CSS
      - @kristoferbaxter: thinks about this the same way, so the naming proposed is confusing
      - base CSS contains more than elements so the name is more generic (but is mostly elements); @dvoytenko: we could just call it element for now to make it more readable
  • is v0.css a private API?  yes; we should we consider removing it if no one is using it?; @jridgewell: can't remove it, signed exchanges is using it
  • @aghassemi: even if one of the css files is empty (ads) maybe we should just have an empty placeholder file to make it more clear what's going on

@mrjoro mrjoro changed the title Design Review 2019-05-22 15:30 UTC (Africa/Europe/western Asia) Design Review 2019-05-22 15:30 UTC (wg-infra, shadow dom stability, blurry image opt out, separate ads css) May 22, 2019
@mrjoro mrjoro closed this as completed May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants