[docs] Improve loading experience

[eps1lon]

The idea is the let react yield earlier. The components that are not pre-rendered don't have to be rendered synchronously because the user will notice the shift anyway. But we can increase TTI by defering them with a passive effect.

Note to self: While we do implement a fallback for NoSsr and have a component dedicated to fallback UIs in our library we do not even use these ourselves.

[mui-pr-bot]

No bundle size changes comparing 8d12012...161b736

Generated by 🚫 dangerJS against 161b736

[oliviertassinari]

I have used this trick in the past to speed up the page transition speed. It sounds promising. I'm not sure we will measure a difference on the initial load though (I would expect more an impact in client side navigation).

[mbrookes]

Not sure about the loading indicator for the search textfield, as it isn't visible for long enough to see that that's what it is. Perhaps it could be a div styled the same as the input?

[eps1lon]

as it isn't visible for long enough to see that that's what it is.

Yeah the idea is more to have some shape in place that outlines the actual import. At some point NoSsr will become React.Suspense anyway and then we don't have to care about the displayed duration being too short.

I think the div suggestion is indicative of shortcomings of the Skeleton. Maybe add color variants that have better contrast e.g. <Skeleton on="primary" />?

[oliviertassinari]

Removed the fallback from the server-rendered output. Suspense will handle the heavy lifting regarding display duration.

Following the latest version of the React's guide on suspense, it seems that delaying is not something they support natively nor they plan to? That it will be up to UI libraries to handle it? https://reactjs.org/docs/concurrent-mode-patterns.html#delaying-a-pending-indicator.

I have tried the following diff, it seems to provide a great DX (+ the diff on the toolbar I haven't shared) 😍.

diff --git a/docs/src/modules/components/AppFrame.js b/docs/src/modules/components/AppFrame.js
index f6ccd0608..550d13ca3 100644
--- a/docs/src/modules/components/AppFrame.js
+++ b/docs/src/modules/components/AppFrame.js
@@ -51,10 +51,29 @@ Router.onRouteChangeError = () => {
   NProgress.done();
 };

+const Delay = withStyles(theme => ({
+  root: {
+    opacity: 0,
+    animation: `${theme.transitions.duration.shorter}ms linear 0s forwards $makeVisible`,
+  },
+  '@keyframes makeVisible': {
+    '100%': {
+      opacity: 1,
+    },
+  },
+}))(({ timeout = 500, classes, ...other }) => (
+  <div className={classes.root} style={{ animationDelay: `${timeout}ms` }} {...other} />
+));
+
 const AppSearch = React.lazy(() => import('docs/src/modules/components/AppSearch'));
 function DeferredAppSearch(props) {
   const { className } = props;

+  const fallback = (
+    <Delay timeout={1000}>
+      <Skeleton className={className} height={40} variant="rect" width={200} />
+    </Delay>
+  );
   const [mounted, setMounted] = React.useState(false);
   React.useEffect(() => {
     setMounted(true);
@@ -69,12 +88,12 @@ function DeferredAppSearch(props) {
       />
       {/* Suspense isn't supported for SSR yet */}
       {mounted ? (
-        <React.Suspense
-          fallback={<Skeleton className={className} height={40} variant="rect" width={200} />}
-        >
+        <React.Suspense fallback={fallback}>
           <AppSearch className={className} />
         </React.Suspense>
-      ) : null}
+      ) : (
+        fallback
+      )}
     </React.Fragment>
   );
 }

I think that we can keep exploring the problem, I suspect we can release a new module to address it. We have a prior work on the delaying problem in https://material-ui.com/components/progress/#delaying-appearance. However, I have noticed two issues with the Fade component:

1. It doesn't work server-side. In our case, it's important, we can notice a flash of skeleton for the demo's toolbar.
2. It doesn't work with the Skeleton as it already leverages the opacity prop.

[oliviertassinari]

@eps1lon I can see two paths to move forward here.

1. We remove the Skeleton from the demo's toolbar, I think that no flash (no distraction) is better than a skeleton, especially for a part of the UI that isn't required during the first seconds of rendering. I think that it's only once the developer has identified the demo he needs, that he might be interested in using the actions in the toolbar.
2. We push this idea of a Delay component forward, leveraging the documentation and integration with the Skeleton as a practical use case

[oliviertassinari]

Regarding, 1. I'm tempted to say the same thing for the search bar. The the case of the toolbar, we can only see two states, it's skeleton or toolbar. However, in the case of the search bar, we can notice 3 states: empty, skeleton, searchbar. In a fast computer, it twinkles. Making a stronger distraction (away from the content of the page) than the toolbar.

[eps1lon]

I think that no flash (no distraction) is better than a skeleton

So we should remove the Skeleton from the core since it's a bad component in your opinion?

especially for a part of the UI that isn't required during the first seconds of rendering

Which is any lazy loaded part. That is the whole point of lazy loading: prioritizing required parts while deferring optional parts. You're essentially saying that skeletons are useless because they should only be used for required UI elements which means your whole UI shouldn't be rendered since it's missing required parts.

Making a stronger distraction (away from the content of the page) than the toolbar.

A skeleton isn't distracting. That's the whole point of it. To prepare content while the user can scan different parts of the UI

We push this idea of a Delay component forward, leveraging the documentation and integration with the Skeleton as a practical use case

This would be a step back. We want to get rid of fixed loading indicators and let Suspense handle these.

[eps1lon]

Also rejecting this whole "flashing" argument. This PR is objectively reducing "flash" by displaying an intermediate skeleton that is close to the final UI.

[oliviertassinari]

So we should remove the Skeleton from the core since it's a bad component in your opinion?

Without a defer logic, I can cause more harm than good, yes. At least, it's my personal experience comparing A with B. This is why I think that we should explore how to defer on the Material-UI side. It's actually awesome that we start to use it for our own documentation, it's the best way to improve these building blocks, by using them :D.

You're essentially saying that skeletons are useless because they should only be used for required UI elements which means your whole UI shouldn't be rendered since it's missing required parts.

I think that YouTube and Facebook are great benchmarks for what the Skeleton component is best used for: keep the attention of the users on the important elements of the page, create a spatial referential for when the important data will be loaded, help end-users process the information faster.

Also rejecting this whole "flashing" argument. This PR is objectively reducing "flash" by displaying an intermediate skeleton that is close to the final UI.

That's a really interesting perspective as mine is completely opposed 😄. I feel more mentally loaded with these skeletons in place. On the other hand, using the Skeleton for the body of the page, like Vuetify is doing feels fine: https://vuetifyjs.com/en/styles/text/ (visible on client-side navigations). I think that we need more feedback cc @joshwooding and @mbrookes

We want to get rid of fixed loading indicators and let Suspense handle these.

Do you have a resource showing that Suspense will? I can no longer find a reference to a timeout option (outside useTransition that doesn't do what we look for) on this unstable feature.

[eps1lon]

good bot :)

[eps1lon]

Removed skeletons

Note to self: While we do implement a fallback for NoSsr and have a component dedicated to fallback UIs in our library we do not even use these ourselves.

[oliviertassinari]

@eps1lon Thank you!