Skip to content
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

Improve pattern rendering sequence #44750

Open
mtias opened this issue Oct 6, 2022 · 11 comments
Open

Improve pattern rendering sequence #44750

mtias opened this issue Oct 6, 2022 · 11 comments
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Enhancement A suggestion for improvement. [Type] Performance Related to performance efforts

Comments

@mtias
Copy link
Member

mtias commented Oct 6, 2022

At the moment, patterns seem to go through a few rendering adjustments before settling into their final form. I'm spotting at least three but there could be more.

It'd be great to solve this as it makes the pattern inserter and the pattern experience feel more fragile.

First Render Second Render Third Render
image image image

cc @youknowriad @tyxla @jsnajdr as it also relates to #42525

@mtias mtias added [Type] Enhancement A suggestion for improvement. [Type] Performance Related to performance efforts [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Oct 6, 2022
@youknowriad
Copy link
Contributor

youknowriad commented Jul 15, 2024

This is the same as #38911

Unfortunately, we don't have a good way yet to isolate resolvers or apiFetch requests or any sync behavior when loading patterns (in order to show the loaders properly).

Any ideas @tyxla @jsnajdr This could have a big impact on UX in a lot of places.

@tyxla
Copy link
Member

tyxla commented Jul 15, 2024

I haven't dug in-depth here just yet, but at first glance, this looks like a missed chance to preload some menus or pages' endpoint data. Or it's possible that the pre-existing navigation preloading doesn't work for some reason—IIRC, the endpoint parameters on the backend needed to match the ones on the front end perfectly for the preloading middleware to work. If that data is correctly preloaded, these blocks should load properly on the first render. @jsnajdr is this something you could investigate?

Alternatively, we could experiment with implementing a loading spinner for each pattern that needs more time to load, although ideally, they shouldn't take that long. That would capture any requests that blocks inside the pattern preview perform after the iframe loads, though. But it might require some additional tooling: as @youknowriad mentioned we currently don't have a good way to know which requests are blocking a particular pattern preview. This could be another area to experiment in.

Furthermore, there could be some little improvements we could do here. What if the pattern previews fade in with a small delay, concealing at least some of the initial rendering glitches or FOUC? Something like this:

Screen.Recording.2024-07-15.at.17.51.28.mov

@youknowriad
Copy link
Contributor

I haven't dug in-depth here just yet, but at first glance, this looks like a missed chance to preload some menus or pages' endpoint data.

I don't really think that's the main issue here, remember that the site editor is basically an SPA and that pages and templates can contain any random number of menus, template parts, reusable blocks... In other words, preloading means preloading the whole database is not the right solution for me.

Alternatively, we could experiment with implementing a loading spinner for each pattern that needs more time to load, although ideally, they shouldn't take that long. That would capture any requests that blocks inside the pattern preview perform after the iframe loads, though.

I think we keep avoiding this approach but I feel this is the right thing to do, it's not easy obviously but I wonder if we can find a creative way to do that.

@tyxla
Copy link
Member

tyxla commented Jul 16, 2024

I don't really think that's the main issue here, remember that the site editor is basically an SPA and that pages and templates can contain any random number of menus, template parts, reusable blocks... In other words, preloading means preloading the whole database is not the right solution for me.

I agree. There might be some low-hanging fruits there, though, since we're already preloading some navigation endpoints, and navigations still take a few requests to load.

I think we keep avoiding this approach but I feel this is the right thing to do, it's not easy obviously but I wonder if we can find a creative way to do that.

Indeed. I believe it's totally doable, and I'm confident we can give it a try.

To be fair, I was hoping we were able to pull static block preview rendering off, as that would solve most of the performance and user experience issues: #54999

If we can achieve it, that remains the best alternative since we won't have to think about hacks and smart loading experiences that don't substantially improve the end-user experience.

@jsnajdr
Copy link
Member

jsnajdr commented Jul 19, 2024

Preloading can work, but it's a different kind of preloading than createPreloadingMiddleware. It's the _embed query param for REST API requests.

When loading /posts/123 the response has a numeric author field that contains only the author ID. But with /posts/123?_embed=author you'll get the full author object in _embedded.author.

Similar kind of preloading could also work for entities referenced from the content. Patterns often link to other patterns (wp:pattern { slug="..." }), the post-author-name block uses the current user resource... The server could know these dependencies, by parsing the content, and from block.json info, and when ?_embed=content:blocks is requested, it could send it all along.

@tyxla
Copy link
Member

tyxla commented Jul 19, 2024

Makes sense @jsnajdr. Is this preloading something you wish to experiment with?

@jsnajdr
Copy link
Member

jsnajdr commented Jul 22, 2024

This could be interesting for multiple people:

  • extracting multiple resources from a single HTTP response and resolving multiple resolvers at once: @Mamaduka recently implemented this for the canUser selector, extracting separate info from the Allow header. Now it would be generalized to extract everything possible from the _embedded field.
  • parsing the content block markup on the server and doing something with the parsed blocks -- did @ockham do this when implementing block hooks? Also, can @dmsnell's HTML API be useful here?

@Mamaduka
Copy link
Member

Thanks for the ping, @jsnajdr!

I like the idea of resolving embedded resources, though it might be difficult without tacking relations between entities/resources (I'm not sure if the query builder concept could be applied here).

parsing the content block markup on the server and doing something with the parsed blocks

I think @ellatrix implemented this for the patterns in the recent release - #60349.

@tyxla
Copy link
Member

tyxla commented Jul 23, 2024

This one also comes to mind: #54999 cc @draganescu and @kevin940726

@ockham
Copy link
Contributor

ockham commented Jul 24, 2024

parsing the content block markup on the server and doing something with the parsed blocks -- did @ockham do this when implementing block hooks?

Thank you for the ping, @jsnajdr! Yeah, for Block Hooks, we are parsing, traversing, and re-serializing templates, template parts, patterns, and wp_navigation objects on the server. This could possibly be extended to run some additional processing. You might want to have a look at traverse_and_serialize_block plus some of the visitors (or rather, visitor factories) we're using.

Also, can @dmsnell's HTML API be useful here?

FWIW, for any operations that need to happen at block level (vs at HTML tag level), Dennis has advised against using the HTML API, and to use a block-level parser instead (as the rules that govern block syntax are much more rigid and thus easier to implement -- and more performantly so -- than is possible with the notoriously complex HTML spec). For Block Hooks, we started experimenting with a lazy parser a while ago, which should improve performance over the parse/traverse/serialize dance. The experiment has been stalled for a while, but I'm hoping to pick it up in the next couple of weeks.

@tyxla
Copy link
Member

tyxla commented Aug 8, 2024

I've been tinkering with a naive and presumptuous alternative, where we're loading the patterns behind the screen based on the presence of currently resolving requests:

Obviously, it needs lots of love, but I'm curious to hear early thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Enhancement A suggestion for improvement. [Type] Performance Related to performance efforts
Projects
None yet
Development

No branches or pull requests

6 participants