ampproject / ampproject/amphtml
[amp-story-player] Replace iframe.onload() for storyContentLoaded
- Dominant language
- JavaScript
- Stars
- 14.9k
- Forks
- 4.1k
- PR merge metrics
- No merged PRs in 30d
Description
Currently the iframes inside the player have a handler for the `onload()` event which sets some loading-related CSS classes (they show the poster image / loading spinner).
https://github.com/ampproject/amphtml/blob/e7798f4fbd1128a5d3785ab1f098bf2ea2d4c782/src/amp-story-player/amp-story-player-impl.js#L637-L641
The issue with using `iframe.onload()` is that it could be fired twice (once when the element is attached to the DOM and a second time when the content loads ([see codepen](https://codepen.io/gmajoulet/pen/poNjVYm)).
We also have an existing native event from `amp-story` called `storyContentLoaded`. It is fired once meaningful story content has loaded, so we should wait for this event instead.
While we wait for this event, we should display the poster and/or loading spinner until the current story dispatched its `storyContentLoaded` on initial loading of the player and on navigation.
### Alternative solutions considered
This could be solved by attaching the `onload` event to the iframe after it's appended to the DOM or by always setting the `src` on the iframe before appending it to the DOM [see codepen](https://codepen.io/maenrique/pen/poNgJMb). Also by checking the [event.target.src](https://stackoverflow.com/questions/10781880/dynamically-created-iframe-triggers-onload-event-twice/38459639#38459639) on the iframe `onload` event.
But this will just make the player code harder to read and it's not optimized for the existing story heuristics (`storyContentLoaded`).
Contributor guide
Assessment
This issue has not been assessed yet.