lizhimins commented on PR #5801:
URL:
https://github.com/apache/rocketmq-dashboard/pull/5801#issuecomment-6093316635
> All three defects check out against trunk. `navigator.clipboard.writeText`
can reject outside non-secure contexts too (denied permission, unfocused
document, browser gesture rules), and the existing `.catch(() => undefined)`
followed by `return` meant the textarea fallback was unreachable — the reader
clicked copy and got silence. `document.execCommand('copy')` was also called
without an optional chain, and `finally { document.body.removeChild(textarea)
}` throws a second time when `appendChild` itself threw. Moving to an async
`handleCopy` with the fallback outside the `try`,
`document.execCommand?.('copy')`, and `textarea.remove()` in `finally` is the
right shape, and surfacing `copied` through the `aria-label` as well as the
tooltip/icon is a genuine accessibility improvement (`ai.bubble.copied` already
exists at `translations.ts:1800`, so no i18n work was needed). > > The tests
are worth calling out: trunk's `AssistantBubble.test.tsx` has nineteen cases
and none of them to
uch the copy button, so this is net-new coverage rather than a rewrite. They
are mutation-sensitive in the right places — reverting the Clipboard
`try/catch` reddens the fallback case, dropping the optional chain reddens the
`missing` case's `windowErrors` assertion, and restoring `removeChild` reddens
the `throwing` case's cleanup assertion. Asserting that a failed copy does
*not* claim success is exactly the assertion we want. > > The blocker is
ownership: #5799 fixes the same issue (#5796), landed 17 minutes earlier on a
newer base, and genuinely conflicts with this branch in both
`AssistantBubble.tsx` and its test file. It also adds eight cases rather than
six and verifies the copied markdown itself — that it contains the answer text
and excludes reasoning blocks — which this branch does not assert. We will pick
one of the two and close the other with a pointer. If you would like this
branch to be the one that survives, rebasing onto current `rocketmq-studio` and
adding a
copied-content assertion would be what tips it.
---
**Decision**: we are taking #5799 - it is earlier and broader, and it also
asserts the copied content itself rather than only that the clipboard call
happened. Closing this one as a duplicate. For next time the pointer above
still holds: a rebase onto current `rocketmq-studio` plus a copied-content
assertion is what would have tipped a close call like this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
