diff options
| author | Yuriy Andamasov <yuriy@vyos.io> | 2026-05-06 01:59:34 +0300 |
|---|---|---|
| committer | Yuriy Andamasov <yuriy@vyos.io> | 2026-05-06 01:59:34 +0300 |
| commit | f37c272ff976fe84636e7a0dcbe9a429cb097c86 (patch) | |
| tree | a84c4f9435fcef4a1da3b7eaf95257a01d3a98e4 /docs | |
| parent | 21a554bd4f9156e41f1c73ba6b7223bb63b3a4ef (diff) | |
| download | vyos-documentation-f37c272ff976fe84636e7a0dcbe9a429cb097c86.tar.gz vyos-documentation-f37c272ff976fe84636e7a0dcbe9a429cb097c86.zip | |
fix(codecopier): exclude Copy label from clipboard and stop showing false success
Two real bugs in the docs site copy-to-clipboard handler, both flagged by
copilot review on PR #1886 (deferred there because that PR is scoped to typos).
Bug 1: Copy label leaked into copied snippets on narrow screens
The handler read `currentTarget.offsetParent.innerText`. The same handler
appends a visible `.copyDiv` (with a 'Copy' <p>) into that container.
Below the 992px breakpoint the label is visible (see code-snippets.css
.copyDiv > p) so users got 'Copy' appended to every copied snippet.
Fix: extract text from the <pre> element inside the container instead,
which excludes the injected button. Also switched the lookup root from
`offsetParent` to `parentElement` so the source-of-truth is the DOM
relationship (the .copyDiv is inserted as a beforeend child of the inner
.highlight div via insertAdjacentHTML), not CSS positioning.
Bug 2: Failed clipboard writes still showed 'Copied!'
The try/catch only logged on failure but the surrounding code still flipped
the button into the copiedNotifier success state. Users got false success
when writeText rejected (insecure context, permission denied, etc.).
Fix: move the success UI flip inside the try, add an explicit failure UI
flip ('Failed' text + new `.copyFailedNotifier` class with red background)
in the catch. setTimeout still reverts both classes after 2s.
Verified in browser with a sphinx-rendered fixture (jQuery 3.7 + Pygments
output): both snippets copy their own text without the Copy label, success
flow shows Copied!, simulated writeText rejection shows Failed, both states
revert after 2s.
🤖 Generated by [robots](https://vyos.io)
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/_static/css/code-snippets.css | 7 | ||||
| -rw-r--r-- | docs/_static/js/codecopier.js | 46 |
2 files changed, 40 insertions, 13 deletions
diff --git a/docs/_static/css/code-snippets.css b/docs/_static/css/code-snippets.css index 555b80d7..ccdb415b 100644 --- a/docs/_static/css/code-snippets.css +++ b/docs/_static/css/code-snippets.css @@ -69,13 +69,18 @@ margin-top: 4px; } -.copiedNotifier > span { +.copiedNotifier > span, +.copyFailedNotifier > span { font-size: 14px !important; color: #fff !important; text-align: center; margin-bottom: 0; } +.copyFailedNotifier { + background-color: #b3261e; +} + .highlight { & .kn { diff --git a/docs/_static/js/codecopier.js b/docs/_static/js/codecopier.js index bf0b3b4d..ae069ba0 100644 --- a/docs/_static/js/codecopier.js +++ b/docs/_static/js/codecopier.js @@ -20,6 +20,24 @@ function formDiv(id) { ` } +// Extract the snippet text from a code-block container while excluding the +// injected .copyDiv button. On viewports below the breakpoint the visible +// "Copy" label otherwise leaks into the clipboard via innerText. +function extractSnippetText(container) { + if (!container) { + return '' + } + const preElement = container.querySelector('pre') + if (preElement) { + return preElement.innerText + } + // Fallback for any structure without a <pre>: clone the container, + // remove every .copyDiv, then read innerText off the clone. + const clone = container.cloneNode(true) + clone.querySelectorAll('.copyDiv').forEach((node) => node.remove()) + return clone.innerText +} + $(document).ready(async function () { const codeSnippets = $( '.rst-content div[class^=highlight] div[class^=highlight], .rst-content pre.literal-block div[class^=highlight], .rst-content pre.literal-block div[class^=highlight]' @@ -34,26 +52,31 @@ $(document).ready(async function () { copyButton.click(async ({ currentTarget }) => { - // we obtain text and copy it const id = currentTarget.dataset.identifier + const divWithNeededId = $(`div[data-identifier='${id}']`) + // Read from the .copyDiv's parentElement (the inner .highlight div it was + // injected into via insertAdjacentHTML('beforeend', ...)) rather than + // currentTarget.offsetParent — offsetParent depends on CSS positioning + // and would walk past .highlight if its `position: relative` is ever + // dropped, picking up the wrong snippet. + const textToCopy = extractSnippetText(currentTarget.parentElement) try { - await navigator.clipboard.writeText(currentTarget.offsetParent.innerText) + await navigator.clipboard.writeText(textToCopy) + // Only flip the button into the success state when the write actually + // resolves — earlier versions showed "Copied!" even on failure. + divWithNeededId.addClass('copiedNotifier') + divWithNeededId.html('<span>Copied!</span>') } catch (error) { - console.log('Copiing text failed, please try again', { - error - }) + console.error('Copying text failed, please try again', error) + divWithNeededId.addClass('copyFailedNotifier') + divWithNeededId.html('<span>Failed</span>') } - // we edit the copyDiv connected to copied text - const divWithNeededId = $(`div[data-identifier='${id}']`) - divWithNeededId.addClass('copiedNotifier') - divWithNeededId.html('<span>Copied!</span>') - setTimeout(() => { divWithNeededId.html(innersOfCopyDiv) divWithNeededId.removeClass('copiedNotifier') - + divWithNeededId.removeClass('copyFailedNotifier') }, 2000) }) @@ -64,4 +87,3 @@ $(document).ready(async function () { navbar.append(readTheDocsButton) }); - |
