fix(site): report copy failures instead of claiming success - #5
Merged
Merged
Conversation
document.execCommand('copy') signals failure by returning false, not by
throwing. The legacy copy path called it inside a try with an empty catch
and then ran done() unconditionally, so the button showed "copied ✓"
whether or not anything reached the clipboard — and the clipboard-API
rejection handler was an empty function, silent in the same way.
The button now has a failure state. done and fail are declared in the IIFE
scope rather than inside the click handler, so neither is redefined per
click. Both route through one settle() that also writes the outcome to
aria-label: the button's label is what a screen reader announces, so
updating only textContent would have left non-sighted users with exactly
the silence this commit is removing.
The reset timer is now cancelled before a new one starts. Without that, a
second click inherits the first click's pending timer and the button
reverts early.
ta.remove() replaces removeChild and sits outside the try, so the textarea
is torn down on both paths.
Also switches a deprecated `word-break: break-word` to
`overflow-wrap: anywhere` — not break-all, which would break the curl
command mid-character.
Leaves the contrast warning and the role="img" terminal alone: both are
false positives, and "fixing" the latter makes a screen reader read the
ASCII art aloud.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AaPvsFt1xYoXErXaZ4Kk8A
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



document.execCommand('copy')signals failure by returning false, not by throwing. The legacy copy path called it inside atrywith an emptycatchand then randone()unconditionally:So the button showed "copied ✓" whether or not anything reached the clipboard. The clipboard-API path was silent in the same way — its rejection handler was
function () {}.This is
javascript:S2486("handle this exception, don't catch it at all, or explain why it is ignored"), and it is a genuine bug rather than a style finding: the one thing this button exists to tell the user is exactly what it got wrong.Changes
copy failed ✗with a.copy.errcolour, alongside the existing.copy.ok.doneandfaildeclared in the IIFE scope, not inside the click handler, so neither is redefined per click. This also clearsjavascript:S7721, and declaring both together avoids raising a fresh S7721 on the new function.aria-label, not justtextContent. The button carries a staticaria-label="Copy install command", which is what a screen reader announces — updating only the visible text would have left non-sighted users with precisely the silence this PR removes.ta.remove()replacesremoveChild(javascript:S7762) and sits outside thetry, so the textarea is torn down on both paths.overflow-wrap: anywherereplaces the deprecatedword-break: break-word(css:S1874). Notbreak-all, which would break the curl command mid-character.Four outcomes, all now distinct: secure-context success and failure, legacy success and failure.
Deliberately untouched
css:S7924contrast at line 262var(--bg)and composited the 5% white overlay onto white. "Fixing" it means hardcoding an opaque background, which kills the gradient the card is built around.Web:S6819role="img"at line 416<img>means rasterizing the transcript. Naive fixing is an accessibility regression.Both should be resolved in the SonarCloud UI rather than in code.
Verification
site/index.htmlis static, with no build step and no HTML or JS linter in CI, so there is nothing to run —nodeis not available in this environment, sonode --checkwas not run. The changed block was traced by hand through both copy paths and all four outcomes, and a bracket/string/comment-aware balance scan over the file passes. Worth a manual click on both paths before merge.Closes 4 code smells.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AaPvsFt1xYoXErXaZ4Kk8A