Improve Story block media loading #16663

Tug · 2020-07-31T10:48:37Z

Addresses part of #16591:

Show a progress indicator when uploading media from the block editor, prevent story playback while its uploading
Improve loading of media files
Better fit media on the player, use heuristics to crop some part of the image to make it fit a user screen (for instance allow 10% cropping max, after which margins can be displayed)
Refine UX
Scale down story embeds (make it 50% smaller?)

With this change we stop rendering all the slides dynamically from the server. Instead we only render the first one and serialize the media into the markup so the client can retrieve it and load slides dynamically. Slides are rendered as the story is played, with a preload of the next slide.

It also applies the latest changes in design regarding stories:

Smaller embeds
Shadow and animation on hover
Play and replay button removed, replaced with transparent overlay
Displaying an expand icon on the top right

This doesn't update buttons to use @wordpress/element nor fixes accessibility issues, those are tackled in this PR #16852

Here is how it looks like:

Frontend:

Block Editor:

Changes proposed in this Pull Request:

In this PR we lazy load media while stories are being played. This way, we improve the initial page load speed.
We also define a list of alternative images for each slide using srcset and force the browser to load the right image for the current viewport.

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Load a page with a story in it. Notice in the network tab only the fist slide is loaded.
Disable JS and reload the page, notice the story looks almost identical as the version with JS

Proposed changelog entry for your changes:

Improve loading of media files for the Story block

matticbot · 2020-07-31T10:48:43Z

Caution: This PR has changes that must be merged to WordPress.com
Hello Tug! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D47362-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

jetpackbot · 2020-07-31T10:52:05Z

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16663

Scheduled Jetpack release: September 1, 2020.
Scheduled code freeze: August 25, 2020

Generated by 🚫 dangerJS against bf7fef5

… images when available, hide play button by default

…ot reset story on slides change

… the content fits the screen its viewed on

…ry on the page

…filter support on safari

…play poster image on load if first slide is a videopress media

jeherve

This is getting really nice! I tested with WP 5.4.2 and no Gutenberg.

When I first upload images, I get the muliple image indicators:

But if I then add an additional image to that block, I lose the indicators:

Things don't quite work on the frontend though:

jeherve · 2020-08-18T15:38:13Z

extensions/blocks/story/story.php

 }
 add_action( 'init', __NAMESPACE__ . '\register_block' );

+/**


This works for now, but I wonder if we could simplify some of this by relying on existing Jetpack features (namely Photon) to do all that work for us.

Good point. I think photon could simplify some of the logic here. I'm still not fully convinced this block should depend on jetpack features as it could actually be moved to gutenberg core or somewhere else. For now I think trying to simply reuse the already available versions of an image makes sense.

jeherve · 2020-08-18T15:53:09Z

extensions/blocks/story/story.php

-function render_slide( $media, $index ) {
+function render_slide( $media, $index = 0 ) {
+	$media_template = '';
+	switch ( ! empty( $media['type'] ) && $media['type'] ) {


Maybe we could return early if we don't have that information? No need to even output an empty div.

Thanks, indeed I updated this while you were reviewing 👍

jeherve · 2020-08-18T15:53:37Z

extensions/blocks/story/story.php

+					<path d="M0 0h24v24H0z" fill="none"></path>
+					<path fill-rule="evenodd" clip-rule="evenodd" d="M6 3H14V17H6L6 3ZM4 3C4 1.89543 4.89543 1 6 1H14C15.1046 1 16 1.89543 16 3V17C16 18.1046 15.1046 19 14 19H6C4.89543 19 4 18.1046 4 17V3ZM18 5C19.1046 5 20 5.89543 20 7V21C20 22.1046 19.1046 23 18 23H10C8.89543 23 8 22.1046 8 21H18V5Z"></path>
+				</svg>
+				<span>%s</span>


Suggested change

<span>%s</span>

<span>%d</span>

extensions/blocks/story/story.php

jeherve · 2020-08-18T15:59:48Z

extensions/blocks/story/story.php

+ */
+function render_pagination_bullet( $slide_index ) {
+	return sprintf(
+		'<button class="wp-story-pagination-bullet" aria-label="Go to slide %d">


I think <button> may cause issues on WordPress.com, I'm not sure it's allowed there. It may be best to have an a instead.

I didn't know, do you know more about this?
In general, I try to stay away from anchor tags when there is no href attribute to point to, which is the case here

This may tell you a bit more:

p5TWut-gm-p2

Slideshow: Replace Buttons With A Tags wp-calypso#31175 (that's back when Jetpack blocks were still developed in the Calypso monorepo, but the code still uses a tags to this day:

jetpack/extensions/blocks/slideshow/slideshow.js

Line 169 in 3b5cf19

<a

As you can see, the no href issue was discussed in that PR as well.

Interesting, will convert every button, just to be on the safe side

jeherve · 2020-08-18T15:59:59Z

extensions/blocks/story/story.php

+ */
+function render_pagination_bullet( $slide_index ) {
+	return sprintf(
+		'<button class="wp-story-pagination-bullet" aria-label="Go to slide %d">


I'd recommend translating that string.

jeherve · 2020-08-19T09:21:25Z

extensions/blocks/story/player/components/calypso-spinner.js

+ */
+import { createElement } from '@wordpress/element';
+
+const CalypsoSpinner = () => (


Maybe we should use Core's spinner here instead?
https://github.com/WordPress/gutenberg/tree/master/packages/components/src/spinner

We explored this with Megs but she preferred we use the calypso spinner which looks much better for our case here.

Tug · 2020-08-19T11:43:46Z

Thanks for the review @jeherve I'm going to address your comments.

But if I then add an additional image to that block, I lose the indicators:

Oh right I can reproduce when using drag n drop. Fixing this 👍

Things don't quite work on the frontend though:

Not sure how you ended up with this, it seems like nor the CSS nor the JS is loaded, do you have any error in the console?

jeherve · 2020-08-19T11:57:11Z

do you have any error in the console?

No error in the console, no. Not quite sure what happened. this may have been a fluke of my testing site, I'll try again in my next review.

…aram (even though it's not a gallery)

Tug · 2020-08-19T15:56:08Z

@jeherve Thanks, I addressed most of your concerns. I only replaced the button elements on the server side for now. #16852 is supposed to replace the custom buttons with @wordpress/components ones and it needs updating so I'll investigate those as part of that PR.

Other than that, I'm not sure what your error is, are we talking about the site frontend, or the calypso reader (the latter is still in progress here)? Could you double check that beta blocks is enabled and that story/view.css and story/view.js both load on the page? What browser are you using?

jeherve

This works well now, and I can't reproduce the issues I was facing earlier. 🚢

* master: (23 commits) Premium Blocks: set blocks availability (#16898) Compat Package: Fix method declaration compatibility (#16900) Jetpack Dashboard: More meaningful error notices. (#16883) Connection REST API: Unit test for the `remote_authorize` request. (#16879) use blog token to request jetpack.updateBlog (#16698) Improve Story block media loading (#16663) Simplify error notices for broken connections (#16655) Use new heartbeat package (#16285) wrap-paid-block: remove component. deprecated. (#16895) Social Previews: improve preview description handling (#16889) Stats module use blog token (#16727) Form Block: add a new Consent Field, a new Newsletter setting, and a new newsletter variation (#16808) AAG: Backup card, fall back to VP content in case of /rewind API error. (#16867) Donations: Fix dependencies (#16892) Creative Mail: update option to lowercase (#16861) Premium Blocks: Implement the new design (#16611) Requests to Stats CSV use the blog token (#16716) Update spacing around sharing buttons to avoid no bottom margin below the customize link. (#16811) Jetpack SSO: Cleaning up the `requestNonce` API request. (#16830) Donations: Update plans when currency changes (#16844) ...

* Load media dynamically to improve TTI (Time To Interaction) on page load * Show loading screen in block editor while media is uploading. Display images when available, hide play button by default * Remove comment * Make sure loading screen is 100% width in fullscreen * Allow previewing images while uploading by locking post save and do not reset story on slides change * Try to load the full video before playing using fetch * Only try full loading videos if no CORS * Show loading state if ANY slide is still uploading * [Design update] Reduce embed size and show slide count * Box shadow animation on hover * Disable box shadow animation in fullscreen * Reset overflow slide wrapper and fix blur for IE * Fix replay when exitFullscreenOnEnd is false and cleanup * Disable tap to pause * Disable highlight on overlay press on android * Use layout effect for launching into fullscreen * Minor style fix in block editor to align controls * Render slide count from the server * Show slide count when block is not selected in the block editor * Add srcset and sizes attributes dynamically for slides with images so the content fits the screen its viewed on * Auto crop sides of images if ratio is close to 9:16 * Save width and height as part of the media attributes * Fix missing closing tag * Auto crop videos as well * Try to fit media in screens close to 9:16 * Refine autocrop and default it to 20% * Add expand icon alternative to slide count and allow customization of the player from the block settings attribute * Server side crop as well * Default to the expand icon version * Zoom on embed on hover and display pagination from server * Remove replay button on end * Hide mute icon for images * Fix minor UI glitches * Try tweaking animation * Tweak embed animation * Fix and refine UI for story block on the block editor * Get width and height from media_details on upload * Fix next slide in block editor * Cleanup unused dependencies * Allow modifying the story settings from url when there's only one story on the page * Override button CSS shadow properties as used on P2s * Display story in fullscreen over WordPress admin bar * Add setting to autoplay story on load * Make background transparent instead of using opacity to fix backdrop filter support on safari * Videopress: fix video url if incorrect in the block attribute and display poster image on load if first slide is a videopress media * Make the story a link to the post itself until the JS is loaded * Fix adding new slides using drag n drop by setting the addToGallery param (even though it's not a gallery) * Replace button tags with a on the server side and allow empty stories * Return early if no media type for a slide

Tug added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Block] Story labels Jul 31, 2020

Tug added this to the 8.9 milestone Jul 31, 2020

Tug self-assigned this Jul 31, 2020

matticbot added the Touches WP.com Files label Jul 31, 2020

Load media dynamically to improve TTI (Time To Interaction) on page load

a94e373

Tug force-pushed the update/story-block-loading-media branch from 27619fe to a94e373 Compare July 31, 2020 12:28

Tug added 20 commits August 3, 2020 17:06

Show loading screen in block editor while media is uploading. Display…

2314699

… images when available, hide play button by default

Remove comment

329f31e

Make sure loading screen is 100% width in fullscreen

5a5b2cf

Allow previewing images while uploading by locking post save and do n…

0011283

…ot reset story on slides change

Try to load the full video before playing using fetch

729c515

Only try full loading videos if no CORS

ef1101c

Show loading state if ANY slide is still uploading

34e640a

[Design update] Reduce embed size and show slide count

40189c3

Box shadow animation on hover

a89b5a7

Disable box shadow animation in fullscreen

cccab6c

Reset overflow slide wrapper and fix blur for IE

9337f7d

Fix replay when exitFullscreenOnEnd is false and cleanup

e5cf751

Disable tap to pause

a12f350

Disable highlight on overlay press on android

9f5a0bd

Use layout effect for launching into fullscreen

38d68ee

Minor style fix in block editor to align controls

e37ed87

Render slide count from the server

d0baed7

Show slide count when block is not selected in the block editor

3bbbda2

Add srcset and sizes attributes dynamically for slides with images so…

6838d79

… the content fits the screen its viewed on

Auto crop sides of images if ratio is close to 9:16

e9e48bb

Tug requested a review from jeherve August 14, 2020 21:48

Cleanup unused dependencies

5a35ef5

Tug added the [Status] Needs Review This PR is ready for review. label Aug 17, 2020

Tug added 7 commits August 18, 2020 12:06

Allow modifying the story settings from url when there's only one sto…

b609246

…ry on the page

Override button CSS shadow properties as used on P2s

92204ea

Display story in fullscreen over WordPress admin bar

029f1f0

Add setting to autoplay story on load

e6e62b5

Make background transparent instead of using opacity to fix backdrop …

4e3f153

…filter support on safari

Videopress: fix video url if incorrect in the block attribute and dis…

d81748c

…play poster image on load if first slide is a videopress media

Make the story a link to the post itself until the JS is loaded

e166270

jeherve reviewed Aug 19, 2020

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Aug 19, 2020

Tug added 3 commits August 19, 2020 17:14

Fix adding new slides using drag n drop by setting the addToGallery p…

6538a7d

…aram (even though it's not a gallery)

Replace button tags with a on the server side and allow empty stories

29bf8ae

Return early if no media type for a slide

bf7fef5

jeherve approved these changes Aug 19, 2020

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Aug 19, 2020

Tug merged commit 69e403f into master Aug 19, 2020

Tug deleted the update/story-block-loading-media branch August 19, 2020 17:43

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 19, 2020

jeherve removed the [Status] Needs Changelog label Aug 24, 2020

Tug restored the update/story-block-loading-media branch September 8, 2020 10:47

Improve Story block media loading #16663

Improve Story block media loading #16663

Uh oh!

Conversation

Tug commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Does this pull request change what data or activity we track or use?

Testing instructions:

Proposed changelog entry for your changes:

Uh oh!

matticbot commented Jul 31, 2020

Uh oh!

jetpackbot commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeherve left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tug commented Aug 19, 2020

Uh oh!

jeherve commented Aug 19, 2020

Uh oh!

Tug commented Aug 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Tug commented Jul 31, 2020 •

edited

Loading

jetpackbot commented Jul 31, 2020 •

edited

Loading

jeherve left a comment •

edited

Loading

Tug commented Aug 19, 2020 •

edited

Loading