[docs-infra] Fix code-block copy button broken on direct page load#48653
Conversation
brijeshb42
added
the
scope: docs-infra
Deploy previewhttps://deploy-preview-48653--material-ui.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
|
Is this related? |
|
Yeah. Did not see this one. Need to update the |
Closes mui#48629 ### Cause Markdown code blocks are static HTML injected via `dangerouslySetInnerHTML`. `InitCodeCopy` wired each button imperatively with `addEventListener`, but React replaces those DOM nodes on a re-render after hydration — orphaning the listener. The effect only re-ran on `router.pathname` change, so a same-page load never re-wired the button. ### Fix - Replace per-node wiring with a single delegated `click` listener on `document`, so copy survives React recreating the nodes (no per-node references to go stale, and `router.pathname` dependency dropped). - Resolve the `⌘`/`Ctrl` shortcut hint via CSS off a `data-mac` attribute set once on `<html>`, instead of mutating each button's text — also immune to node replacement. - Hover/focus tracking for keyboard copy now uses event delegation as well. ### Verified Click copy, keyboard `⌘C`, and the shortcut hint all work on direct load and after navigation; the React tabbed code blocks are unaffected (delegated handler ignores them).
brijeshb42
force-pushed
the
fix-hydration
branch
from
d636b83 to
3fa399b
Compare
| }, 2000); | ||
| try { | ||
| if (pre?.textContent) { | ||
| await clipboardCopy(pre.textContent); |
There was a problem hiding this comment.
Shouldn't the "copied" indicator logic go after this call? It shouldn't show "copied" when copy fails, right?
There was a problem hiding this comment.
Right. Fixed the existing bug.
There was a problem hiding this comment.
But now there's no feedback on failure ofcourse. Not sure when it would fail though.
There was a problem hiding this comment.
What feedback would be apt for failure ? IMO, its better to have no feedback than to show Copied even when nothing was copied.
|
|
||
| return undefined; | ||
| }, [rootNode, router.pathname]); | ||
| // Track the hovered/focused code block so `CodeCopyProvider` can copy it on Ctrl/⌘+C. |
There was a problem hiding this comment.
Is it common to have Ctrl+C copy the whole content of a focused element? My expectation is that it copies my selection, and if I haven't selected anything it doesn't copy anything at all.
There was a problem hiding this comment.
My assumption would be that explicit copy button in the UI would be for the whole block where it is. If I am selecting something, I'd also copy it with keyboard/mouse directly.
Do you have any example where it happens this way ? I saw 2-3 sites where copy button always copies the whole text regardless of selection (radix/shadcn etc).
There was a problem hiding this comment.
Yes copy button behavior makes sense, but mui.com also copies the whole content if my cursor is in there with nothing selected and I press cmd+c. it even seems to do it when I simply hover the code.
There was a problem hiding this comment.
On which page/link exactly ? Do you have a recording ? Seems a bug to me, especially copy just by hovering
There was a problem hiding this comment.
I mean hover+ctrl+c. Actually the behavior looks like:
- when I hover regular text, not in a codeblock and de cmd+c => nothing changes in my clipboard
- when I hover a code block and do the same => clipboard gets cleared
There was a problem hiding this comment.
Wait no, that's on the demos. Sry. On the regular codeblocks, when I hover+cm+c it copies the content.
Screen.Recording.2026-06-12.at.12.51.31.mov
There was a problem hiding this comment.
Its in this PR's docs as well, behaviour hasnt been updated in any way. Though I think its going way out of the way to copy stuff.
There was a problem hiding this comment.
Yeah, just flagging as we could clean this up now that we're here.
There was a problem hiding this comment.
I'll remove the copy on hover+cmd+c functionality
There was a problem hiding this comment.
Update: I'll probably do this as a followup PR based on what the conclusion is.
Also fix the blur so the hover item disappears
2 participants
Closes #48629
Introduced in #48261, which re-rendered the App component due to state update of
versionsafter fetch.Cause
Markdown code blocks are static HTML injected via
dangerouslySetInnerHTML.InitCodeCopywired each button imperatively withaddEventListener, but React replaces those DOM nodes on a re-render after hydration — orphaning the listener. The effect only re-ran onrouter.pathnamechange, so a same-page load never re-wired the button.Fix
clicklistener ondocument, so copy survives React recreating the nodes (no per-node references to go stale, androuter.pathnamedependency dropped).⌘/Ctrlshortcut hint via CSS off adata-macattribute set once on<html>, instead of mutating each button's text — also immune to node replacement.