Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Adding loading state to psammead-media-player #3600

Merged
merged 21 commits into from
Jul 28, 2020

Conversation

RichardPK
Copy link
Contributor

@RichardPK RichardPK commented Jul 13, 2020

Resolves #bbc/simorgh#7185

Overall change:

Part 3/3 to resolve #7185

The final PR to add a dark-mode compatible image placeholder to the psammead-media-player.

1. Add dark mode version of BBC_BLOCKS . #3610
2. Implement dark mode for psammead-image-placeholder. #3611
3. Implement image placeholder in psammead-media-player. (this one)

Overall change:

  • Use z-index to place media player on top of loading placeholder - once it's loaded, the placeholder is masked.
  • Allow darkMode props to be passed to the media player and propagated to the placeholder image
  • Add MAP media player example to Storybook and enable darkMode knob
  • Note AMP already has a placeholder experience implemented, this is achieved by nesting a child AmpImg directly below the parent amp-iframe. It uses the media's thumbnail as the image, and has a small loading animation while the media initialises. Once it is initialised, the placeholder is given the visibility: hidden property. Documentation of this is available here https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/placeholders/?format=websites & the code is here
  • Version bumps for packages

  • I have assigned myself to this PR and the corresponding issues
  • Automated jest tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@RichardPK RichardPK added ws-media The World Service media stream ux To be reviewed by UX before merging labels Jul 13, 2020
@RichardPK RichardPK self-assigned this Jul 13, 2020
@RichardPK
Copy link
Contributor Author

RichardPK commented Jul 16, 2020

[ARCHIVE - Explanation of the approach taken]

Broadly speaking, there are two approaches for adding a loading placeholder while only accessing the outer iframe

  1. Use state and the iFrame's onLoad event to conditionally render the ImagePlaceholder, or animate it away
  2. Use the player's z-index to place it on top of the ImagePlaceholder when it loads in, obscuring the loading screen.
  • This early experiment suggests a thorough 'loading' state integration, would require communing 🙏 with SMP directly via Morph, or removing the iFrame on canonical. There is, however, an easy win we can try out in approach 2.

Approach 1:

A. Using onLoad to conditionally render the component, without animation
Initial video comparing the two different states. With the loading component on the left, without it on the right: https://www.loom.com/share/ebca1fccceaa4e0e828c824b91cb8cb9

  • It appears that this approach is adding around 1s to the component load time. Notice the component without the loading screen is refreshed second, but finishes first.
  • There is still a flash of white after the iFrame has loaded & the loading screen is no longer rendered (icky).
  • Slightly unscientific as this is loading in Storybook too, but the total network load times give us some indication of the impact on page speed:

Without onLoad state change
Screenshot 2020-07-14 at 09 28 55

With onLoad state change
Screenshot 2020-07-14 at 09 29 11

B. Using onLoad, plus animation ✨ 😮
The onLoad state triggers a transition, making the loading screen fade out. Video: https://www.loom.com/share/db26658d0a854f9e987a3ea86c44618d

  • As before this approach adds about 1s to the page load time.
  • The SMP 'pop-in' is masked by the animation only firing once it's fully loaded, but:
  • As the onLoad func on the outer iframe triggers before SMP is ready to be clicked, I had to manually add a time delay to the animation... of 1.6s. 🤢
  • This is essentially adding visual fidelity at the expense of performance.

Approach 2 (I'd advocate for this ✅ ):

Use z-index to place media player on top of loading placeholder - once it's loaded, the placeholder is masked.
Video: https://www.loom.com/share/8e4113a230cb45ac80c9523742eda85b

  • No impact on load times
  • Does successfully paint the previously white space with placeholder while SMP is still loading in.
  • Minor pop-in from SMP
  • This is an easy-win, to test impact on lighthouse, bounce rates & playcounts

@RichardPK RichardPK changed the title Adding loading state for MAP media player Adding loading state for psammead-media-player Jul 16, 2020
@RichardPK RichardPK added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jul 16, 2020
@RichardPK RichardPK removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jul 23, 2020
@RichardPK RichardPK marked this pull request as ready for review July 28, 2020 08:53
@RichardPK RichardPK changed the title Adding loading state for psammead-media-player Adding loading state to psammead-media-player Jul 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ux To be reviewed by UX before merging ws-media The World Service media stream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show placeholder while MAP player is loading
3 participants