Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Hi,
This looks totally awesome.
To improve the user experience, is it planned (if it even makes sense) to display a discreet spinner / loader image by default for all the blocks loaded after the first flush?
Thanks
Related reading
- http://www.callumhart.com/blog/non-blocking-uis-with-interface-previews
- http://www.callumhart.com/demo/building-interface-previews-with-react
- https://calendar.perfplanet.com/2016/progressive-storyboards/
- https://github.com/egoist/vue-content-loader
- https://web.dev/layout-instability-api
- https://css-tricks.com/building-skeleton-screens-css-custom-properties/ via @ronaldtebrake in #72
Comment | File | Size | Author |
---|---|---|---|
#94 | SkeletonScreenReview.gif | 213.04 KB | ronaldtebrake |
#91 | interdiff_88-91.txt | 750 bytes | pooja saraah |
#91 | 2632750-91.patch | 7.45 KB | pooja saraah |
#88 | interdiff_76-88.txt | 2.53 KB | ronaldtebrake |
#88 | interface_previews-2632750-88.patch | 7.46 KB | ronaldtebrake |
Issue fork drupal-2632750
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 2632750-interface-previewsskeleton-screens changes, plain diff MR !3312
Comments
Comment #2
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedThe plan I originally formulated at DrupalCon Barcelona was to use the theme system and theme system suggestions for the BigPipe placeholders itself.
e.g.
Then it would be possible to just create a:
theme/templates/big_pipe--block--user_login.html.twig
to override that block easily.
Obviously the suggestions to use would need some help, but it would
And possibly we could even provide a loading / spinner by default.
Thoughts?
Comment #3
Wim LeersI'm not sure a spinner by default is a good idea. Imagine many simultaneous spinners. Imagine spinners in a
display: inline
situation. That'll be ugly and unusable.That being said, we definitely need interface previews to improve the UX. I forgot to create an issue for that, so I'll just repurpose this issue.
I think #2 sounds like a great path to a first implementation/iteration of interface previews.
Comment #4
darol100 CreditAttribution: darol100 as a volunteer and commentedI do not think this should be part of big_pipe module instead of part of the theme project. Because the looks of the interface preview is going to depend base on theme looks. I think we should handle this by providing good documentation on how to support "Interface Preview" in your theme and provide few examples on how to provide support for the big_pipe project in your theme.
What you guys think about this approach instead of providing a interface preview that might look bad on a different theme ?
Comment #5
krlucas CreditAttribution: krlucas at Genuine commentedIt seems like the first step is to make js placeholder markup theme-able. Patch attached.
I used the arguments for the #lazy_builder callback to create template suggestions and default classes which works OK for some things (like blocks), less OK for things like messages (it has no arguments).
Comment #6
Wim LeersWow, this is an awesome start! Thank you so much! Excited to have you work with us on this :)
This is very clever, but it also kind of breaks down as soon as the
#lazy_builder
callback argument order is unfortunate. For example, for the comment form, these are the suggestions:big_pipe_js_placeholder__node
big_pipe_js_placeholder__node__6
big_pipe_js_placeholder__node__6__comment
big_pipe_js_placeholder__node__6__comment__comment
And indeed, for
Drupal\Core\Render\Element\StatusMessages::renderMessages()
it flat out fails.I think using the
#lazy_builder
(and cleaning it up of course, removing colons and backslashes etc) instead would yield a more workable result. But even then it can be verbose.I wonder if we can think of a more elegant solution. But again, I love this first iteration :)
Comment #7
krlucas CreditAttribution: krlucas at Genuine commentedGlad to help!
Here's another iteration that uses the lazy_builder callback and cache keys (if available) for the suggestions. I also removed the class and id generation from the template preprocessor--it looks like most core modules just leave that up to the themes now.
Comment #8
krlucas CreditAttribution: krlucas at Genuine commentedI raised this issue at the Boston Drupal meetup, demoing the current state of Big Pipe using the Big Pipe demo module (yay!!). In particular, I surfaced the issue of how to render a sensible, visual placeholder--one that claims page space so the page doesn't dramatically reflow as "actual" content is replaced--when by definition you and the system don't know much about the ultimate content.
One suggestion that came up was allowing users or developers to specify or suggest how the placeholder is rendered. For instance, if one is creating a custom block, allow the user to specify the relative size--small, medium, large--of the placeholder and then allow the theme to determine what that means in a particular region. There's a bunch of UX considerations here--in particular that a custom block would only really need that setting if the "Roles" visibility setting was set. Also "interface previews" are generally only necessary for content that appears above the fold.
But I think it does suggest an API change: #lazy_builder needs to be a little more robust. It should be able to provide not just a callback for the "real" content, but also some clues on how to render the placeholder. So #lazy_builder could be an array:
Thoughts?
Comment #9
krlucas CreditAttribution: krlucas at Genuine commentedComment #10
Wim LeersComment #12
Wim LeersComment #13
PolRerolled patch, just testing.
Comment #14
PolNow trying to find out why this is not working anymore.
Comment #15
PolUpdated patch, working now.
If you want to see it working, edit the template and add some CSS:
<div{{ attributes }} style="background-color: #eeeeee; min-height: 100px;"></div>
Comment #16
Wim LeersI marked #2690199: Unify the placeholder HTML for better consistency and allow custom placeholder as a duplicate of this.
Comment #17
Wim LeersRelated to BigPipe because BigPipe can sometimes cause jumpiness, depending on the lazy-loaded content: https://developers.google.com/web/updates/2016/04/scroll-anchoring — Google Chrome is introducing scroll anchoring preventing jumpiness caused by reflowing.
Comment #18
krlucas CreditAttribution: krlucas commentedNote the latest patch is replacing explode() with DOMDocument parsing which I think is necessary to allow the placeholders to be fully theme-able (not dependent on the placeholder beginning exactly
<div data-big-pipe-placeholder-id="
). But there's discussion in issue #2678662: Ensure BigPipe does not break when HTML document contains CDATA sections or inline scripts matching certain patterns about whether DOMDocument is the best method.Comment #19
krlucas CreditAttribution: krlucas commentedRe-rolled relative to the drupal root.
Comment #20
Wim LeersI've thought about this some more. More importantly, I've been weighing whether this is something we should do in 8.1, or later. The thing is: we don't have enough insight or experience yet with how "interface previews" would and should work. I think #2702061: Unify & simplify render & theme system: component-based rendering (enables pattern library, style guides, interface previews, client-side re-rendering) and #2759849: Create a new user-facing core theme as part of a Drupal demo could and should inform how this works. Especially the latter.
What matter most at this time, is two things:
So, I set out to prove that. I did not look at any work done here so far, to see what I'd come up with independently, and in which ways that would be worse or better. My results:
Turns out that I independently came up with pretty much identical code to what #19 already contains :D
controller::foo
, then you can provide an interface preview for it by ensuringcontroller::fooPreview
exists. Very simple. And very logical for those writing lazy builders. Great for end users, great for developers, but bad for themers: they can't override it to match their theme.#type
and#theme
and just pass in placeholder variables… then we're again in a great place. When doing so, specify__preview
as a suffix, to indicate this is for rendering a preview. That means it is also overridable per theme: you can provide a Twig template to render a preview first. (In the case of comment links, a theme can providelinks--comment-preview.html.twig
.)This doesn't work for everything — it works for things like links and blocks, but it doesn't work for more complex things, like forms. Because forms are often highly customized on a per-site basis. And so the preview for one site won't be good for all sites. Although even there, we can have rough approximations, in templates that themes can override.
All this demonstrated in the included screencast (see below).
This patch shows that this can be done without breaking BC. It's a pure API addition. It's different from the prior patches in that it won't have a very tricky template name, and for the cases where a separate template is not necessary (like the comment links example), it's far, far simpler, and in fact doesn't require any work on the side of the themer: the preview will automatically match the final result! (This is also the direction #2702061: Unify & simplify render & theme system: component-based rendering (enables pattern library, style guides, interface previews, client-side re-rendering) takes.)
Comment #21
Wim LeersI've moved this to 8.2, because I think this is something that we should first gain a better understanding of. #2759849: Create a new user-facing core theme as part of a Drupal demo will focus on component-based themes, and I think we may get valuable insights there about interface previews, that could help this implementation to change direction.
What matters, is that we've proven in two independent implementations (see patches #19 and #20) that this can definitely be implemented without breaking BC.
Therefore, marking this postponed for now.
Comment #23
Wim LeersDammit, I left some "resetting" code in the patch to record my screencast!
Also, I realize that the visual end result of the placeholders is not very clear at this low resolution. But posting the full resolution video would probably be excessive. So here's a screenshot showing the full detail:
Comment #24
Wim LeersComment #25
Wim LeersI've had an interesting conversation with Kevin O'Leary about this.
Point 1 is addressed in #23.
Point 2 is very interesting. I contemplated doing that. There's one big problem with that: we don't know where it will be positioned: it could be towards the bottom, or towards the top, depending on which fields the site has configured, and which fields are accessible for the current user. This would be a quite prominent preview, so it'd be quite strange to see this prominent thing shift around significantly.
We can choose to go with more-generic-but-broadly-applicable or high-fidelity-but-narrowly-applicable. I went with the former. To put it in Callum Hart's terminology: the current patch does something between "barebones" and "aspiring". Kevin would like to see something between "aspiring" and "perfectionist".
I'm +1 to striving towards that, but in the current render system (i.e. before #2702061: Unify & simplify render & theme system: component-based rendering (enables pattern library, style guides, interface previews, client-side re-rendering)), it'd be quite a chore to maintain all those preview templates.
Finally, Kevin added that
. As long as those SVGs don't show too much detail, I think that could definitely be valuable.Interesting stuff to take into account when we continue working on this!
Comment #26
Wim LeersIn March, I noticed that the (excellent!) Belgian real estate website https://www.realo.be/en also uses interface previews. Screenshot included.
Comment #27
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedI don't think we need to use DOM parsing.
I think we should use HTML comments to wrap the theme output of the selector in addition to having an ID, that comment could also have the ID itself, so no further parsing needed.
Then we can continue to use simple explodes() for non-JS and the data-selector for JS.
It would put some restraints on the templates, but that should be okay - given the flexibility it gives.
Comment #28
Wim LeersI don't understand your comment.
Interface previews only ever make sense when using JS. When using no-JS, we can't first send a placeholder, because we can't replace it afterwards. Therefore we can't have previews in that case. And since we already generate no-JS BigPipe placeholders separately from JS BigPipe placeholders, we don't need to deal with the complexity that you are referring to in #27. :)
See my PoC patches above.
Comment #29
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commented#28 I don't understand it either.
I am sorry, what I wrote was completely bizarre.
What I meant to write is:
Lets use placeholders that look like:
which means the code to get the placeholder order does not need to use DOM parsing, but can continue to use explode().
But this time not on the
<div id="..."> or <span id="...">
, but on the HTML comment and as those are safe for JS to replace we know we can add HTML comments around it.And the user would never see those comments in HTML anyway, so its just helping markup.
Then the template can be whatever it wants to be - as long as it has the right data selector.
And then we don't need the DOM parsing and it might likely be more reliable, too.
Comment #30
Wim LeersHah :D
Ahhh! That 's an option, yes. We'll take that into account in the next iteration of this patch.
I won't be working on this for now: this issue is postponed, see #21 for the "why".
Comment #33
Wim LeersFirst, let's rebase this. These patches are identical to #23, but apply cleanly to
8.4.x
.Comment #34
Wim LeersMy main concern with #23 (and #33) is the TX/DX: it works, but it's not clear how you can make it work.
#19 and earlier patches definitely were simpler: they allow you to define a template, always, and call it a day. However, they do come with a significant disadvantage: they require you to duplicate potentially complex compositions of existing render elements. Because you're replacing an entire render array of arbitrary size with a single template.
This is why I took a different approach in #20. Let me quote a bit from that comment:
The key example of this benefit (as that quote already indicates) is the comment links example:
This is what the developer would need to provide. A themer can customize it by providing a
links--comment--preview.html.twig
template. But usually, that would not be necessary. Because in this example, the preview lies not in a complex rendering, but in showing bars/blocks in the place of the text that will appear.The patch also provides an example of something that is too complex to benefit from this approach: the comment form.
For the comment form, the interface preview callback does not even try to mimic the comment form (it has no way of remotely knowing what it would look like — because a site might e.g. have customized it with lots of additional fields).
So in this case, it makes sense to use that "interface preview template" to provide a rough approximation as the interface preview.
It's entirely possible that experience will tell us that option 1 is something we use so rarely that it is not worth it, and that we hence might as well go back to the approach in #19. That's fine. That's why this is not yet in the BigPipe module in core: because it needs experimentation.
However, in the mean time, the TX/DX should point people in the right direction: a developer (DX) needs to provide an "interface preview callback". And the themer then has the ability to provide a template to customize the default interface preview. That's what I'm doing in this iteration: generate HTML comments (much like Twig debug output) if Twig debug is enabled (duh!), to provide direction.
For the two examples above, that results in:
And for everything else, which doesn't have that interface preview callback, this is what you see:
Comment #35
Wim LeersOne of the most common things that you might want to create an interface preview for: a personalized or uncacheable block. As of this patch, blocks are supported.
This led me to discover an interesting limitation in the current approach: the "preview" callback does not receive any arguments… and hence it technically only allows a single "interface preview" for all blocks. That's of course then missing the point.
So, I had to make this slightly more flexible. It's working great :)
Now seeing output like this:
:)
Comment #36
Wim LeersRelated: https://calendar.perfplanet.com/2016/progressive-storyboards/
Comment #38
webchickWe reviewed this issue in the product management team meeting today.
We are confused on why this issue is postponed on #2759849: Create a new user-facing core theme as part of a Drupal demo. That issue (at least in this day and age) is specifically for the example profile, which will be experimental for at least a release or two. With Big Pipe moving into Standard profile (hopefully), the impact of interface previews is a lot more important, so postponing them on such an ambitious initiative seems sub-optimal, especially given there's been some great progress in the patches up above. We would love to see this un-postponed and active again; happy to provide reviews during UX hours if that's the predominant concern.
Comment #42
webchickComment #43
Wim Leers#2759849: Create a new user-facing core theme as part of a Drupal demo was originally bi-focal: one of its focuses was doing "discovery work" for #2702061: Unify & simplify render & theme system: component-based rendering (enables pattern library, style guides, interface previews, client-side re-rendering). That has since changed. So I agree it no longer makes sense to postpone this issue on that.
Comment #45
Wim LeersRelevant: https://github.com/egoist/vue-content-loader, and its sibling http://danilowoz.com/create-react-content-loader/ — the latter even has a UI to create placeholders!
Comment #47
Wim LeersLullabot's Mike Herchel blogged about this! See https://www.lullabot.com/articles/quick-tip-add-a-loading-animation-for-....
I left a comment asking him to chime in here: https://www.lullabot.com/articles/quick-tip-add-a-loading-animation-for-... 🙏
Comment #48
mherchelHey Everyone! Thanks for working on this!
Warning: I read the issue description, but haven't gone through all of the comments.
Anyway, my thought is that Wim is right that we need to make sure that we don't insert loading or placeholder content into content that is quickly loaded. When I first inserted the loading animation in, I was really surprised to see it pop up all over the place (keep in mind I have all types of caching disabled on my local environment). We definitely don't want that.
That being said, there's room for improvement! Here are some thoughts:
1. Instead of just inserting a hardcoded
span
, make this themable by using a template. If we have the ability to affect the markup, themers could then insert more complex loading animations (including SVGs, weird CSS stuff, etc). Maybe we could also change the markup based on node type, regions, etc?2. A non-💩 way to add a default loader is to have it not appear until a certain number of seconds has passed. I think 2 seconds would be reasonable. We could do this really cheaply with CSS only by animating the opacity of the loader, and then setting the
animation-delay
property to 2 seconds or whatever. This means that the animation will not show, unless 2 seconds goes by.3. As far as placeholder content, I don't really think core should insert anything unless we know what it's going to look like (which I don't think we can do). To me, this seems a bit too complicated, and if we implement the ability to affect placeholder markup, the themer could easily do this.
Thoughts?
Comment #49
Wim LeersChanging markup based on node type/region/… — I'm not sure what you mean exactly here. But I think you want the placeholder to be aware of its context. That's possible, if and only if the logic that renders the contents of the placeholder is itself also given information about its context, i.e. if one of the arguments to the placeholder's callback is this context. (Hope that made sense! That's going pretty deep into the weeds. I think we can tackle this in a later phase)
Comment #50
Wim LeersClosed #2992283: Allow custom markup as placeholder as a duplicate of this. Also improving the issue title to settle on what seems to be getting consensus: we want template support for this!
Comment #52
Wim LeersCrediting @mherchel and @yogeshchaugule8.
Comment #53
mherchel1. Exactly. I'd love to be able to affect the markup, and turn on/off loaders based on what is loading.
2. Sounds awesome. I can put together some initial CSS.
3. I still don't think we need placeholder content even for comment forms. To me, its not worth the complexity. In the use-case of comment form, they're generally loaded outside of the initial field of view, so it's not terribly important.
Thanks!
Comment #54
Wim LeersSo you're basically saying that:
?Comment #55
mherchelIMO yep. That being said, I really don't feel too strongly about it.
Comment #56
zaporyliereg #54 - good enough as default but customizable via templates, right? I don't think spinner will do for everyone.
Comment #57
yogeshchaugule8 CreditAttribution: yogeshchaugule8 commented+1 to @zaporylie say.
Actual content size will always be different for everyone, and having spinner loaded initially will cause re-arrange content after loading, which doesn't look good if content size is bigger.
Comment #58
Wim Leers#57: that's my concern exactly: content that's jumping around, lots of spinners.
Still, perhaps @mherchel's proposal would be a better default than today's?
What do you think, @zaporylie and @yogeshchaugule8?
P.S.: I really appreciate all of you sharing your opinions in this issue! 👌🙏
Comment #59
zaporylieMy thoughts:
I like the second option more. I think it could be a nice addition to the big_pipe's placeholder API. I added a patch with a PoC.
Comment #60
zaporylieOn the other hand...
Well, I kinda like the idea that module developers can provide the default interface preview, but in the end, it's all about making it look good in the theme context. So maybe templates _are_ enough?
I've implemented some basic block preview for Belgrade theme (commerce demo), just to see how it's gonna look like in actual context and... I'm very excited to see it done in the, hopefully, near future.
Without preview:
With preview:
Comment #61
Wim LeersI'm glad you're so excited by this, @zaporylie, because so am I! I will start pushing this forward again somewhere in the next few weeks. Coincidentally, I believe this can also solve #2992410: Provide placeholders for empty blocks (for example, an empty Views listing).
Comment #62
Wim Leers(Actually, let's start RIGHT NOW!)
@zaporylie Your approach seems very similar to the one I propose in #35, but simpler/less logic. I like that!
On the other hand, it seems like it's requiring more complex template names. Could you share the filenames of the template for the demo that you showed in #60?
My concern with #60 is that it completely couples these preview templates to BigPipe, which would prevent us from reusing those previews (those templates) for #2992410: Provide placeholders for empty blocks (for example, an empty Views listing).
Comment #64
Wim LeersQuoting #17:
I wrote that nearly three years ago.
Yesterday, Firefox implemented this too: https://hacks.mozilla.org/2019/03/scroll-anchoring-in-firefox-66/. This means Firefox is now also less adversely affected by not having accurate interface previews.
Comment #65
geerlingguy CreditAttribution: geerlingguy at Midwestern Mac, LLC commentedI wanted to chime in and just offer my support for at least making the themeability easier by default. I worked through this using @mherschel's blog post for a project, and that was adequate but it was very hard to try to understand how to target just the placeholders I wanted to target, and without that blog post I would've been lost.
It might be nice to offer a spinner of some sort (do we have a core convention for that) by default, or maybe even just make the div very light grey or something? But make it easy to disable that behavior.
Otherwise, just making this more themeable / discoverable would be great.
Comment #66
Wim LeersJust added a new https://www.drupal.org/docs/8/core/modules/big-pipe/overview (diff: https://www.drupal.org/node/2677712/revisions/view/10422380/11422628). It also references this issue.
section toComment #67
Wim LeersOops … that didn't happen 😢
Comment #68
Wim LeersRelated: https://web.dev/layout-instability-api
Comment #72
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and at Open Social for Open Social commentedThanks for all the great input so far, this is also really exciting me.
I'm trying to achieve Skeleton Screens, especially with different views blocks it's difficult to achieve.
Also with the amount of blocks we have on a single page throbbers are not a viable solution for me.
As per #60
is more than sufficient to achieve my goal.
However as noted in #62
This seems like the case as shown below:
For
For me simply removing the callback part in the theme_suggestions would already be a great improvement.
But I'm not sure where we stand with the rest of the open comments, e.g. re-use it for other preview functionality.
This is what I was able to do with / without this patch:
Proof of concept without & with templates:
Comment #73
Wim Leers😭
I got logged out just as I submitted a thoughtful and long comment … and now I have to re-enter it all. DRUPAL.ORG, thanks for making me lose half an hour of work. 😬
I don't have the time to reconstruct it all, so here's a terse version of it.
#72: awesome work! Thanks for https://css-tricks.com/building-skeleton-screens-css-custom-properties/, that looks great! Updating issue title & summary.
Coincidence: at https://twitter.com/wimleers/status/1328755710512013314, there's somebody else looking for exactly this!
So … let's get this done now that there's been quite a bit of validation.
#72 validated #60, which is a continuation of my patch from 3.5 years ago at #35.
The main concern in #72 is the verbosity. We cannot omit
callback
because the same arguments could exist for multiple callbacks, plus it's possible that you want to target all occurrences of a particular lazy builder's placeholder, regardless of its argument.So, I propose to special-case blocks, since they're the 90% use case anyway. Attached reroll goes from
to
Comment #74
Phil Wolstenholme CreditAttribution: Phil Wolstenholme as a volunteer commentedWim, your excitement about this is super contagious, and thanks to ronaldtebrake for reviving this issue too!
Google announced this month that next year their Core Web Vitals web performance metrics are going to become a ranking factor, and one of those metrics is Cumulative Layout Shift – or how visually stable a page's layout is while the page loads.
It looks like the ability to provide custom placeholder markup will be a really useful tool for preventing CLS issues, giving Drupal sites a UX benefit (layout instability can be annoying and cause clicks on the wrong elements when a layout shifts during load), a perceived performance benefit, and eventually a SEO benefit too!
Comment #75
Wim Leers#74: Great minds think alike 🤓 My link in #68 to https://web.dev/layout-instability-api now redirects to https://web.dev/cls/ 😄
So very much +1, and bumping to
priority because of this!Comment #76
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and at Open Social for Open Social commentedI can feel the effects of the excitement too! Thanks so much for getting this a step further and the additional clarifications on the priority and impact of this.
As per #73
Thanks for the clarification that definitely makes sense now and to special-case block would be definitely something I can live with.
I have updated the patch a tiny bit, cleaned up some unused CSS and changed
Due to the
$callback = preg_replace('/[^a-zA-Z0-9]/', '_', $variables['callback']);
happening afterwards.With that I can confirm it works now.
Example of my theme debug.
Comment #77
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and at Open Social for Open Social commentedYou know that feeling you press submit, all excited and.... you forgot the files.. 😬
Comment #78
Wim LeersHey, at least you tested your patch 😁
Comment #79
John Pitcairn CreditAttribution: John Pitcairn commentedI just want to say this is really cool. Thanks to all who have contributed.
Comment #80
saschaeggiThis is awesome! Love to see this!
Comment #81
acrollet CreditAttribution: acrollet at Agile Six Applications for Department of Veterans Affairs commentedThe code meets requirements, is clear to read, and does not introduce any coding standards errors. Tested working, makes a really nice DX improvement - kudos to all involved :-)
Setting RTBC.
Comment #82
catchThis needs some test coverage
big_pipe.css file is missing from the patch (or this hunk is out of date?).
We could use some test coverage for the logic here.
I'm also wondering whether it would be possible to implement this in core somewhere - Umami seems like a possible (probably only) place it could work?
Comment #87
RustedBucket CreditAttribution: RustedBucket commentedIs there any status on this? Any doc on how to actually make it load a preview? I tried the patch and spent several hours tinkering but it doesn't seem to do much of anything.
Comment #88
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and at Open Social for Open Social commentedI finally had some time to get back to this and see if we can get it a bit further.
As per #82
1.
That got introduced in #60 and I assumed it was there for demo purposes to showcase what we could achieve. Not as an actual default CSS implementation.
Please correct me if I'm wrong of course.
2.
I've tried to come up with a test covering the case we added for the simplified suggestion in the update patch.
Hope it covers it.
That sounds amazing! I couldn't really find a good place to start though curious if there are any insights in to how we can use this in Umami.
#87
I'll see if I can get some documentation started the coming days.
Some quick tips that might put you back on track.
<!-- THEME HOOK: 'big_pipe_interface_preview' -->
in your HTML which should show you what file name suggestions are available#lazy_builder
callback function giving you some additional time to check it out.big-pipe-interface-preview.html.twig
and using it correctly you have all the freedom to make some nice interface previews.Hope it helps!
Comment #90
bnjmnmComment #91
pooja saraah CreditAttribution: pooja saraah at Srijan | A Material+ Company for Drupal India Association commentedFixed failed commands on #88
Attached patch against Drupal 10.1.x
Attached interdiff
Comment #93
bnjmnmComment #94
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and at Open Social for Open Social commentedAwesome work, love the additional test coverage as well.
Can confirm it works,
1. i've added the recent content block to the sidebar
2. created:
core/themes/olivero/templates/block/big-pipe-interface-preview--block--views-block--content-recent-block-1.html.twig
with some barebones placeholders for the recent content block3. added a sleep to make sure we can see the preview
et voila;
Comment #95
lauriiiHaven't looked at the MR in detail yet but we need a change record for this since it's a new API.
Comment #96
bnjmnmAdded change record
Comment #98
lauriiiReviewed the code & tests in detail. I think we could potentially still add some more test coverage for the simplified suggestions, but I didn't think that was a hard requirement for this to be committed.
Committed aeac296 and pushed to 10.1.x. Thanks!
Tagging for 10.1.0 release highlights since this is a pretty big developer facing feature.
Comment #100
Wim LeersSuperb! 🤩 Thank you for making happen what I hoped to do many years ago and never got around to finishing! 🙏
The first use of this new infrastructure just landed in #2951268: Improve rendering account link in the toolbar! 🥳
Also, thanks to this issue, I was able to close #3162134: Lazy builder placeholders should show a loading indicator 👍