-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[amp-story-player] Entry point skeleton code and bundle #28999
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approval for build-system/tasks/
src/amp-story-player/amp-story-entry-point/amp-story-entry-point.js
Outdated
Show resolved
Hide resolved
1f1f1a9
to
5598634
Compare
this.rootEl_ = this.doc_.createElement('main'); | ||
|
||
// Create shadow root | ||
const shadowRoot = this.element_.attachShadow({mode: 'open'}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will fail on many browsers, we should have a fallback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll address as a follow-up since this is something that we also do for the player.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, is is tracked somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5598634
to
3ca5cfb
Compare
Related to #26308
Adds
<amp-story-entry-point>
skeleton files and generates bundle. I thought it would be a good idea to leverage our existing "runtime" (AmpStoryPlayerManager
) to handle the lifecycle of the entry points as well, but that's open for discussion.