Skip to content

Commit

Permalink
🐛 [Story loading] Send storyContentLoaded during prerendering (amppro…
Browse files Browse the repository at this point in the history
…ject#36307)

* Initial changes

* Not wait for storyLayoutPromise

* Resolve medialayout on prerender

* Use innerMedia readystate

* Remove unused import

* Fixed tests

* Check innerMediaEl exists

* Dialog visible

* Revert visual tests

* Added timer for vis test info dialog

* Added timer to visual test

* Remove delay from visual tests
  • Loading branch information
mszylkowski authored Oct 14, 2021
1 parent 272d342 commit 73201d9
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 17 deletions.
4 changes: 2 additions & 2 deletions extensions/amp-story/1.0/amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -688,11 +688,11 @@ export class AmpStoryPage extends AMP.BaseElement {
break;
case 'amp-audio':
case 'amp-video':
if (mediaEl.readyState >= 2) {
const innerMediaEl = mediaEl.querySelector('audio, video');
if (innerMediaEl && innerMediaEl.readyState >= 2) {
resolve();
return;
}

mediaEl.addEventListener('canplay', resolve, true /* useCapture */);
break;
default:
Expand Down
23 changes: 12 additions & 11 deletions extensions/amp-story/1.0/amp-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -990,14 +990,10 @@ export class AmpStory extends AMP.BaseElement {

// Do not block the layout callback on the completion of these promises, as
// that prevents descendents from being laid out (and therefore loaded).
storyLayoutPromise
.then(() =>
this.whenInitialContentLoaded_(INITIAL_CONTENT_LOAD_TIMEOUT_MS)
)
.then(() => {
this.markStoryAsLoaded_();
this.initializeLiveStory_();
});
this.whenInitialContentLoaded_(INITIAL_CONTENT_LOAD_TIMEOUT_MS).then(() => {
this.markStoryAsLoaded_();
this.initializeLiveStory_();
});

this.maybeLoadStoryEducation_();

Expand Down Expand Up @@ -1084,9 +1080,14 @@ export class AmpStory extends AMP.BaseElement {
* @private
*/
whenInitialContentLoaded_(timeoutMs = 0) {
const storyLoadPromise = this.pages_[0].element
.signals()
.whenSignal(CommonSignals.LOAD_END);
const initialPageEl = this.element.querySelector(
`amp-story-page#${escapeCssSelectorIdent(this.getInitialPageId_())}`
);
const storyLoadPromise = whenUpgradedToCustomElement(initialPageEl).then(
() => {
return initialPageEl.signals().whenSignal(CommonSignals.LOAD_END);
}
);

return this.timer_
.timeoutPromise(timeoutMs, storyLoadPromise)
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-story/1.0/test/test-amp-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -1032,7 +1032,7 @@ describes.realWin(
const clickEvent = new MouseEvent('click', {clientX: 200});
story.activePage_.element.dispatchEvent(clickEvent);
await waitFor(() => {
if (sendMessageStub.calledOnce) {
if (sendMessageStub.called) {
expect(sendMessageStub).to.be.calledWithExactly(
'selectDocument',
{
Expand Down Expand Up @@ -1088,7 +1088,7 @@ describes.realWin(
const clickEvent = new MouseEvent('click', {clientX: 10});
story.activePage_.element.dispatchEvent(clickEvent);
await waitFor(() => {
if (sendMessageStub.calledOnce) {
if (sendMessageStub.called) {
expect(sendMessageStub).to.be.calledWithExactly(
'selectDocument',
{
Expand Down
4 changes: 2 additions & 2 deletions test/visual-diff/visual-tests
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@
"loading_complete_selectors": [
".i-amphtml-story-loaded",
".i-amphtml-story-info-dialog"
]
],
},
{
"url": "examples/visual-tests/amp-story/amp-story-consent.html",
Expand Down Expand Up @@ -529,7 +529,7 @@
"loading_complete_selectors": [
".i-amphtml-story-loaded",
".i-amphtml-story-info-dialog"
]
],
},
{
"url": "examples/visual-tests/amp-story/amp-story-consent.rtl.html",
Expand Down

0 comments on commit 73201d9

Please sign in to comment.