[docs-infra] Use icons instead of words for the code copy button #37664

danilo-leal · 2023-06-21T12:14:07Z

I have followed (at least) the PR section of the contributing guide.

What the title says 😅 Thought we could swap that out for icons to make less use of space in code blocks. What triggered me to do that was the code snippet within Joy's intro demos, which could use more breathing room.

Before	After

danilo-leal · 2023-06-21T12:17:52Z

⚠️ Help needed: There are some blocks still using words, though, particularly the installation commands. I figure I need to change something in the CodeCopy.tsx file, specifically this section:

          textNode.nodeValue = textNode.textContent?.replace('Copy', 'Copied') || null;
          trigger.dataset.copied = 'true';
          setTimeout(() => {
            if (trigger) {
              textNode.nodeValue = textNode.textContent?.replace('Copied', 'Copy') || null;
              delete trigger.dataset.copied;

But swapping these strings for icons doesn't work as it seems it only welcomes strings. How to solve this issue? 😬

alexfauquette · 2023-06-21T12:36:13Z

The one that cause problems to you are the codes coming from the markdown tokens

```
npm add library
```

It's not react elements, but HTML rendered from the markdown parser. Here is the line
https://github.com/mui/material-ui/blob/c726ed646cfd2c83c0798d29b3277f5d5a58677b/packages/markdown/parseMarkdown.js/#L317-L320

 return `<div class="MuiCode-root"><pre><code class="language-${escape(lang, true)}">${
        escaped ? code : escape(code, true)
      }</code></pre><button data-ga-event-category="code" data-ga-event-action="copy-click" aria-label="Copy the code" class="MuiCode-copy">Copy <span class="MuiCode-copyKeypress"><span>(or</span> $keyC<span>)</span></span></button></div>\n`;

If you want to modify the button, it's here you should do it

docs/src/modules/components/MarkdownElement.js

danilo-leal · 2023-06-21T13:19:22Z

@alexfauquette Thanks for pointing this file out! Though it seems that importing the icon on this file doesn't work as usual? Also, where would I go to change the "Copied" string, too? The parserMarkdown file just shows "Copy" 🤔

alexfauquette · 2023-06-21T14:29:31Z

Effectively you will not be able to put some React components in it.
It's pure HTML, and the line you tried to modify before are the one responsible to handle the interaction in old fashion way (by updating textNode.nodeValue).

The solution could be to put the HTML content of your icons in the string. And instead of modifying textNode.nodeValue you could modify style attributes of SVG elements, or the class name of the button with classList.replace(removed, added) such that only one of them is displayed.

 return `<div class="MuiCode-root"><pre><code class="language-${escape(lang, true)}">${
        escaped ? code : escape(code, true)
      }</code></pre>
<button data-ga-event-category="code" data-ga-event-action="copy-click" aria-label="Copy the code"0
class="MuiCode-copy">
-  Copy
+ <svg class="MuiCode-copy-icon"><path ... /></svg>
+ <svg class="MuiCode-copied-icon"><path ... /></svg>
 <span class="MuiCode-copyKeypress">
	<span>
		(or</span> $keyC<span>)</span>
	</span>
</button></div>\n`;

@danilo-leal After some trial and error here is a solution. I do not manage to commit on top of your branch, so I just copy past the diff

diff --git a/docs/src/modules/components/MarkdownElement.js b/docs/src/modules/components/MarkdownElement.js
index 8465638e81..6468446a19 100644
--- a/docs/src/modules/components/MarkdownElement.js
+++ b/docs/src/modules/components/MarkdownElement.js
@@ -400,6 +400,15 @@ const Root = styled('div')(
       borderColor: lightTheme.palette.primaryDark[500],
       backgroundColor: lightTheme.palette.primaryDark[600],
       color: lightTheme.palette.primaryDark[50],
+      '& svg': {
+        userSelect: 'none',
+        width: '1em',
+        height: '1em',
+        display: 'inline-block',
+        fill: 'currentcolor',
+        flexShrink: 0,
+        fontSize: '18px',
+      },
       '&:hover, &:focus': {
         opacity: 1,
         color: '#fff',
@@ -417,11 +426,20 @@ const Root = styled('div')(
       '& .MuiCode-copyKeypress': {
         display: 'none',
       },
+      '& .MuiCode-copied-icon': {
+        display: 'none',
+      },
       '&[data-copied]': {
         // style of the button when it is in copied state.
         borderColor: lightTheme.palette.primary[700],
         color: '#fff',
         backgroundColor: lightTheme.palette.primaryDark[600],
+        '& .MuiCode-copy-icon': {
+          display: 'none',
+        },
+        '& .MuiCode-copied-icon': {
+          display: 'block',
+        },
       },
       '&:focus-visible': {
         outline: '2px solid',

diff --git a/packages/markdown/parseMarkdown.js b/packages/markdown/parseMarkdown.js
index bca606f483..439d2ec739 100644
--- a/packages/markdown/parseMarkdown.js
+++ b/packages/markdown/parseMarkdown.js
@@ -343,7 +343,8 @@ function createRender(context) {
 
       return `<div class="MuiCode-root"><pre><code class="language-${escape(lang, true)}">${
         escaped ? code : escape(code, true)
-      }</code></pre><button data-ga-event-category="code" data-ga-event-action="copy-click" aria-label="Copy the code" class="MuiCode-copy">Copy <span class="MuiCode-copyKeypress"><span>(or</span> $keyC<span>)</span></span></button></div>\n`;
+      }</code></pre><button data-ga-event-category="code" data-ga-event-action="copy-click" aria-label="Copy the code" class="MuiCode-copy">
+      <svg focusable="false" aria-hidden="true" viewBox="0 0 24 24" data-testid="ContentCopyRoundedIcon"><use class="MuiCode-copy-icon" xlink:href="#copy-icon" /><use class="MuiCode-copied-icon" xlink:href="#copied-icon" /></svg> <span class="MuiCode-copyKeypress"><span>(or</span> $keyC<span>)</span></span></button></div>\n`;
     };
 
     const markedOptions = {
@@ -550,8 +551,18 @@ ${headers.hooks
 <symbol id="comment-link-icon" viewBox="0 0 24 24">
   <path d="M20 2H4c-1.1 0-1.99.9-1.99 2L2 22l4-4h14c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zM6 14v-2.47l6.88-6.88c.2-.2.51-.2.71 0l1.77 1.77c.2.2.2.51 0 .71L8.47 14H6zm12 0h-7.5l2-2H18v2z" />
 </symbol>
+</svg>`);
+      rendered.unshift(`<svg style="display: none;" xmlns="http://www.w3.org/2000/svg">
+<symbol id="copy-icon" viewBox="0 0 24 24">
+  <path d="M15 20H5V7c0-.55-.45-1-1-1s-1 .45-1 1v13c0 1.1.9 2 2 2h10c.55 0 1-.45 1-1s-.45-1-1-1zm5-4V4c0-1.1-.9-2-2-2H9c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h9c1.1 0 2-.9 2-2zm-2 0H9V4h9v12z" />
+</symbol>
 </svg>`);
 
+      rendered.unshift(`<svg style="display: none;" xmlns="http://www.w3.org/2000/svg">
+<symbol id="copied-icon" viewBox="0 0 24 24">
+  <path d="M20 2H8c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h12c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zm-8.24 11.28L9.69 11.2c-.38-.39-.38-1.01 0-1.4.39-.39 1.02-.39 1.41 0l1.36 1.37 4.42-4.46c.39-.39 1.02-.39 1.41 0 .38.39.38 1.01 0 1.4l-5.13 5.17c-.37.4-1.01.4-1.4 0zM3 6c-.55 0-1 .45-1 1v13c0 1.1.9 2 2 2h13c.55 0 1-.45 1-1s-.45-1-1-1H5c-.55 0-1-.45-1-1V7c0-.55-.45-1-1-1z" />
+</symbol>
+</svg>`);
       docs[userLanguage] = {
         description,
         location,

danilo-leal · 2023-06-21T22:32:43Z

@alexfauquette Ah, nice! Really appreciate the detailed instructions & attempts at solving it! Just pushed exactly what you proposed but it's not quite there yet... could you take a look when you have time? I think the key is not being caught and the icon is not changing either when clicking 🤔

mui-bot · 2023-06-21T22:34:02Z

Netlify deploy preview

https://deploy-preview-37664--material-ui.netlify.app/

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against a16a595

packages/markdown/parseMarkdown.js

docs/src/modules/components/MarkdownElement.js

danilo-leal · 2023-06-22T17:42:01Z

@alexfauquette I think we're looking good now aside from this on command code blocks:

alexfauquette · 2023-06-23T07:58:44Z

@danilo-leal
The logic was done here. I assume the space around the SVG added a few nodes which broke btn.childNodes[1]?.childNodes[1].

  const btn = elm.querySelector('.MuiCode-copy') as HTMLButtonElement | null;
  if (btn) {
    const keyNode = btn.childNodes[1]?.childNodes[1];
    if (!keyNode) {
      // skip the logic if the btn is not generated from the markdown.
      return;
    }
    keyNode.textContent = keyNode?.textContent?.replace('$key', key) || null;
    btn.addEventListener('click', handleClick);
    listeners.push(() => btn.removeEventListener('click', handleClick));
  }
});

Should be fixed

danilo-leal · 2023-06-23T22:49:08Z

@alexfauquette Sweet! I think we're good to go ⎯ really appreciate all of your help here and the thorough info explaining to me what was wrong, I really learn from it! I'm only missing your approval now if we're done 😬

There's just this text_unit-1 check that's not turning green and I have no idea why 🤔

…n-adjustment

alexfauquette · 2023-06-26T08:18:12Z

It's because theme.transitions.create expect to see the props in an array. So get fixed by the next diff.

- theme.transitions.create('background', 'borderColor', 'display', options)
+ theme.transitions.create(['background', 'borderColor', 'display'], options)

@siriwatknp could you have a look at the new error message?

I added it because before the message was invalid options [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10] which is not super helpful.

This array comes from the 10 letters of the string 'borderColor'

danilo-leal · 2023-06-26T12:03:12Z

@alexfauquette is it good to merge or do we need to wait for Jun's perspective?

alexfauquette · 2023-06-26T12:04:24Z

I would prefer to get Jun's perspective before merging

michaldudak · 2023-07-03T07:11:32Z

@siriwatknp, could you please take a look? I'd like to include it in the release if possible.

alexfauquette · 2023-07-03T08:19:54Z

@michaldudak If you want, it's about the following diff on the styling helpers: packages/mui-material/src/styles/createTransitions.js

danilo-leal · 2023-07-04T14:38:14Z

@alexfauquette In these last few commits of mine, I made the button always visible, regardless if the code block is hovered or not... but I thought I had fixed its moving positioning 😅 Would you mind taking a look? I'm a bit unsure why that is happening, given the border added on hover is actually using box-shadow which shouldn't add to the box model, right?

Screen.Recording.2023-07-04.at.11.36.12.mov

alexfauquette · 2023-07-04T14:49:41Z

When you do the hover the .MuiCode-copy button movex from default display to inline-flex which is modifying SVG placement

Screencast.from.04-07-2023.16.47.17.mp4

danilo-leal · 2023-07-05T12:09:51Z

@alexfauquette oh, thanks for the tip! It's weird, though, because when running locally, that wasn't happening... only on the deploy preview. Anyway, just pushed the fix removing the display change on hover, hopefully, it solves this out!

Are still waiting for @siriwatknp's perspective on that question you had before merging it? 😬

danilo-leal · 2023-07-05T12:22:17Z

@alexfauquette Uhm, the problem seems to persist even after the above fix 😅 Not really sure why, given the display is now the same. I'm supposing it can have something to do with the added space the <span> elements adds?

And for some reason, again, don't see this behavior locally, just on the deploy preview. 🤔

Screen.Recording.2023-07-05.at.09.20.05.mov

alexfauquette · 2023-07-05T13:02:52Z

Should be good now. The button had some incoherent instruction: width: 26 and children having more than 26px width when the text is visible. So I use position absolute to remove this text from the normal document flow

danilo-leal · 2023-07-05T13:21:02Z

@alexfauquette Awesome, thank you again for helping me out 🙏 What we're missing to move forward? 😬

use icons instead of words

bac90d4

danilo-leal added docs Improvements or additions to the documentation scope: docs-infra Specific to the docs-infra product labels Jun 21, 2023

danilo-leal requested review from siriwatknp and alexfauquette June 21, 2023 12:14

alexfauquette reviewed Jun 21, 2023

View reviewed changes

docs/src/modules/components/MarkdownElement.js Outdated Show resolved Hide resolved

follow Alex's suggestions

376850c

danilo-leal requested a review from alexfauquette June 21, 2023 22:32

alexfauquette reviewed Jun 22, 2023

View reviewed changes

packages/markdown/parseMarkdown.js Show resolved Hide resolved

docs/src/modules/components/MarkdownElement.js Outdated Show resolved Hide resolved

Alex's code suggestions and fixes

7ff0fc4

danilo-leal and others added 3 commits June 22, 2023 16:12

refine styles a bit more (simplify and smooth it out)

2cdcf7b

clean the code used to render the HTML

d292c56

make code slightly more robust

92bef9a

danilo-leal self-assigned this Jun 23, 2023

margin for the copy label

5d288f2

danilo-leal requested a review from alexfauquette June 23, 2023 14:30

alexfauquette added 2 commits June 26, 2023 09:35

Merge remote-tracking branch 'upstream/master' into copy-button-desig…

51d1529

…n-adjustment

fix bug introduced in another PR

caf2d1a

alexfauquette approved these changes Jun 26, 2023

View reviewed changes

danilo-leal requested a review from bharatkashyap June 30, 2023 21:37

danilo-leal mentioned this pull request Jul 2, 2023

[docs-infra] Add header to editable code blocks #37648

Closed

1 task

danilo-leal mentioned this pull request Jul 3, 2023

[docs-infra] Copy button conflicting with the scrollbar #37556

Closed

danilo-leal added 2 commits July 3, 2023 19:49

make it always visible + tweak styles

bdefba5

further adjustments to spacing and sizing

eead9cf

fix moving icon on hover

917a694

fix placement

a16a595

alexfauquette merged commit 8b8136f into master Jul 5, 2023

alexfauquette deleted the copy-button-design-adjustment branch July 5, 2023 15:05

oliviertassinari mentioned this pull request Nov 22, 2024

[docs] Fix template page issues #44466

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs-infra] Use icons instead of words for the code copy button #37664

[docs-infra] Use icons instead of words for the code copy button #37664

danilo-leal commented Jun 21, 2023 •

edited

Loading

danilo-leal commented Jun 21, 2023

alexfauquette commented Jun 21, 2023

danilo-leal commented Jun 21, 2023

alexfauquette commented Jun 21, 2023 •

edited

Loading

danilo-leal commented Jun 21, 2023

mui-bot commented Jun 21, 2023 •

edited

Loading

danilo-leal commented Jun 22, 2023

alexfauquette commented Jun 23, 2023

danilo-leal commented Jun 23, 2023 •

edited

Loading

alexfauquette commented Jun 26, 2023 •

edited

Loading

danilo-leal commented Jun 26, 2023

alexfauquette commented Jun 26, 2023

michaldudak commented Jul 3, 2023

alexfauquette commented Jul 3, 2023

danilo-leal commented Jul 4, 2023

alexfauquette commented Jul 4, 2023

danilo-leal commented Jul 5, 2023

danilo-leal commented Jul 5, 2023

alexfauquette commented Jul 5, 2023

danilo-leal commented Jul 5, 2023

[docs-infra] Use icons instead of words for the code copy button #37664

[docs-infra] Use icons instead of words for the code copy button #37664

Conversation

danilo-leal commented Jun 21, 2023 • edited Loading

danilo-leal commented Jun 21, 2023

alexfauquette commented Jun 21, 2023

danilo-leal commented Jun 21, 2023

alexfauquette commented Jun 21, 2023 • edited Loading

danilo-leal commented Jun 21, 2023

mui-bot commented Jun 21, 2023 • edited Loading

Netlify deploy preview

Bundle size report

danilo-leal commented Jun 22, 2023

alexfauquette commented Jun 23, 2023

danilo-leal commented Jun 23, 2023 • edited Loading

alexfauquette commented Jun 26, 2023 • edited Loading

danilo-leal commented Jun 26, 2023

alexfauquette commented Jun 26, 2023

michaldudak commented Jul 3, 2023

alexfauquette commented Jul 3, 2023

danilo-leal commented Jul 4, 2023

alexfauquette commented Jul 4, 2023

danilo-leal commented Jul 5, 2023

danilo-leal commented Jul 5, 2023

alexfauquette commented Jul 5, 2023

danilo-leal commented Jul 5, 2023

danilo-leal commented Jun 21, 2023 •

edited

Loading

alexfauquette commented Jun 21, 2023 •

edited

Loading

mui-bot commented Jun 21, 2023 •

edited

Loading

danilo-leal commented Jun 23, 2023 •

edited

Loading

alexfauquette commented Jun 26, 2023 •

edited

Loading