mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.91k stars 32.27k forks source link

[docs-infra] Have inline previews for most of the demos #22484

Open oliviertassinari opened 4 years ago

oliviertassinari commented 4 years ago

Summary 💡

Bring "inline previews" for as many demos as possible.

Examples 🌈

Capture d’écran 2020-09-04 à 16 12 19

https://next.material-ui.com/components/buttons/#contained-buttons

Motivation 🔦

Inline previews were introduced in #17831 by @mbrookes. We have done a couple of iteration it increases their usage in the codebase. However, I believe we have never pushed it to its full potential. For instance: https://next.material-ui.com/components/accordion/.

This would match with the answers of the survey: docs - smaller demos.

Benchmark

eps1lon commented 4 years ago

For instance: next.material-ui.com/components/accordion.

What specific demo should be inlined in your opinion?

This would match with the answers of the survey: docs - smaller demos.

Is this about having smaller demos or inlining more demos? It seems like you want smaller demos which incidentally also inlines them.

Making demos smaller so that they're inlined seems like a bad heuristic. Documentation shouldn't be about having the shortest demo possible but the clearest one. In the end you don't write code to be short but to be easy to change and reason about

oliviertassinari commented 4 years ago

Documentation shouldn't be about having the shortest demo possible but the clearest one. In the end you don't write code to be short but to be easy to change and reason about

@eps1lon Agree. Let's untangle it. From what I understand, developers struggle to follow the sources of a demo from a combination of:

  1. there are too many different "elements" showcased at the same. This can create off-by-one errors when trying to match a visual case a developer is interested in and the line he copies & pastes.
  2. there are too many lines of code. This makes it hard to load everything in a developer's head.

Improving these two makes using the examples of the documentation easier. I would expect 1. to be more important than 2. in this context.

The last element to take into account is that removing the need to click on the expand code button also makes it easier for developers to use the example of the documentation. For vertical space usage considerations, the "inline preview" is only triggered if the render block has no more than 17 lines of code. This gives 2. importance.

https://github.com/mui-org/material-ui/blob/a927c9f842cbebed7edffe58dee51f08e0ad8054/docs/src/modules/components/Demo.js#L751


What specific demo should be inlined in your opinion?

If we take https://next.material-ui.com/components/accordion/#basic-accordion as a case study. Maybe like this?

diff --git a/docs/src/modules/components/Demo.js b/docs/src/modules/components/Demo.js
index 7f5f366cb8..1f845b4bdc 100644
--- a/docs/src/modules/components/Demo.js
+++ b/docs/src/modules/components/Demo.js
@@ -748,7 +748,7 @@ function Demo(props) {
     !demoOptions.hideToolbar &&
     demoOptions.defaultCodeOpen !== false &&
     jsx !== demoData.raw &&
-    jsx.split(/\n/).length <= 17;
+    jsx.split(/\n/).length <= 20;

   const [demoKey, resetDemo] = React.useReducer((key) => key + 1, 0);

diff --git a/docs/src/modules/utils/getJsxPreview.js b/docs/src/modules/utils/getJsxPreview.js
index 8d9b843c80..89f7dacbde 100644
--- a/docs/src/modules/utils/getJsxPreview.js
+++ b/docs/src/modules/utils/getJsxPreview.js
@@ -17,7 +17,7 @@ export default function getJsxPreview(code) {
   jsx = jsx ? jsx[1] : code;

   // Remove leading spaces from each line
-  return jsx.split(/\n/).reduce(
+  jsx = jsx.split(/\n/).reduce(
     (acc, line) =>
       `${acc}${line.slice(
         // Number of leading spaces on the first line
@@ -25,4 +25,9 @@ export default function getJsxPreview(code) {
       )}\n`,
     '',
   );
+
+  // Remove leading blank lines
+  jsx = jsx.replace(/[\r\n]+$/, '').replace(/^[\r\n]+/, '');
+
+  return jsx;
 }
diff --git a/docs/src/pages/components/accordion/SimpleAccordion.tsx b/docs/src/pages/components/accordion/SimpleAccordion.tsx
index 8b0ecea979..5bd21e18cd 100644
--- a/docs/src/pages/components/accordion/SimpleAccordion.tsx
+++ b/docs/src/pages/components/accordion/SimpleAccordion.tsx
@@ -21,6 +21,13 @@ const useStyles = makeStyles((theme: Theme) =>
 export default function SimpleAccordion() {
   const classes = useStyles();

+  const details = (
+    <Typography>
+      Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse
+      malesuada lacus ex, sit amet blandit leo lobortis eget.
+    </Typography>
+  );
+
   return (
     <div className={classes.root}>
       <Accordion>
@@ -29,14 +36,9 @@ export default function SimpleAccordion() {
           aria-controls="panel1a-content"
           id="panel1a-header"
         >
-          <Typography className={classes.heading}>Accordion 1</Typography>
+          <Typography>Accordion 1</Typography>
         </AccordionSummary>
-        <AccordionDetails>
-          <Typography>
-            Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse
-            malesuada lacus ex, sit amet blandit leo lobortis eget.
-          </Typography>
-        </AccordionDetails>
+        <AccordionDetails>{details}</AccordionDetails>
       </Accordion>
       <Accordion>
         <AccordionSummary
@@ -44,25 +46,9 @@ export default function SimpleAccordion() {
           aria-controls="panel2a-content"
           id="panel2a-header"
         >
-          <Typography className={classes.heading}>Accordion 2</Typography>
-        </AccordionSummary>
-        <AccordionDetails>
-          <Typography>
-            Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse
-            malesuada lacus ex, sit amet blandit leo lobortis eget.
-          </Typography>
-        </AccordionDetails>
-      </Accordion>
-      <Accordion disabled>
-        <AccordionSummary
-          expandIcon={<ExpandMoreIcon />}
-          aria-controls="panel3a-content"
-          id="panel3a-header"
-        >
-          <Typography className={classes.heading}>
-            Disabled Accordion
-          </Typography>
+          <Typography>Accordion 2</Typography>
         </AccordionSummary>
+        <AccordionDetails>{details}</AccordionDetails>
       </Accordion>
     </div>
   );
Capture d’écran 2020-09-04 à 23 08 13

Then, moving the disabled case to a new demo.


Actually, this makes me wonder about the default of the accordion, shouldn't the accordion have icons by default? e.g. Google featured snippet:

Capture d’écran 2020-09-04 à 23 12 59
vicasas commented 3 years ago

I want to start taking this task #25268 (comment) and I have the following proposal for the page Chip https://gist.github.com/vicasas/d2ca635459db9139756c4fda0382a63c

oliviertassinari commented 3 years ago

@vicasas It llooks good. Will we render a standard and outlined chip in each section?

vicasas commented 3 years ago

@oliviertassinari If exact, one of each variant, something like TextField.

I will try to upload a PR as Draft as soon as possible so that we can review a preview

oliviertassinari commented 3 years ago

This issue is related to #24640, this issue empowers it. The more inline previews we have, the more interesting it will be to be able to edit the demos live.