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

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

Open
1 task done
oliviertassinari opened this issue Sep 4, 2020 · 6 comments
Open
1 task done

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

oliviertassinari opened this issue Sep 4, 2020 · 6 comments
Labels
priority: important This change can make a difference scope: docs-infra Specific to the docs-infra product

Comments

@oliviertassinari
Copy link
Member

  • I have searched the issues of this repository and believe that this is not a duplicate.

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

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation priority: important This change can make a difference labels Sep 4, 2020
@oliviertassinari oliviertassinari changed the title [docs] Have inline preview for most of the demos [docs] Have inline previews for most of the demos Sep 4, 2020
@eps1lon
Copy link
Member

eps1lon commented Sep 4, 2020

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
Copy link
Member Author

oliviertassinari commented Sep 4, 2020

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
Copy link
Member

vicasas commented Apr 10, 2021

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
Copy link
Member Author

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

@vicasas
Copy link
Member

vicasas commented Apr 12, 2021

@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
Copy link
Member Author

oliviertassinari commented Apr 13, 2021

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.

@samuelsycamore samuelsycamore added scope: docs-infra Specific to the docs-infra product and removed docs Improvements or additions to the documentation labels Nov 28, 2023
@samuelsycamore samuelsycamore changed the title [docs] Have inline previews for most of the demos [docs-infra] Have inline previews for most of the demos Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: important This change can make a difference scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

No branches or pull requests

4 participants