codeant-ai-for-open-source[bot] commented on code in PR #41958:
URL: https://github.com/apache/superset/pull/41958#discussion_r3563991918
##########
superset-frontend/src/components/CopyToClipboard/index.tsx:
##########
@@ -86,8 +86,16 @@ function CopyToClip({
cursor,
},
onClick: disabled ? undefined : onClick,
+ onKeyDown: disabled
+ ? undefined
+ : (event: KeyboardEvent<HTMLElement>) => {
+ if (event.key === 'Enter' || event.key === ' ') {
+ event.preventDefault();
+ onClick();
+ }
+ },
Review Comment:
**Suggestion:** The new keyboard handler triggers `onClick()` for every
custom element, including native interactive controls like `button` that
already fire click on Enter/Space. This causes duplicate copy executions
(double toast / double callback) for existing call sites that pass button-based
`copyNode`s. Only synthesize keyboard activation for non-native interactive
elements (or when role-based button behavior is being polyfilled), and let
native controls keep their default keyboard-to-click behavior. [incorrect
condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ SQL Lab result copy fires twice on keyboard activation.
⚠️ Duplicate LOG_ACTIONS_SQLLAB_COPY_RESULT_TO_CLIPBOARD entries logged.
⚠️ Keyboard users see double success toasts for copy actions.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In SQL Lab, the `ResultSet` component at
`superset-frontend/src/SqlLab/components/ResultSet/index.tsx` lines ~448–476
renders a
`CopyToClipboard` instance whose `copyNode` is a `Button` from
`@superset-ui/core/components` (see `copyNode={<Button ... />}` and
`onCopyEnd={() =>
logAction(LOG_ACTIONS_SQLLAB_COPY_RESULT_TO_CLIPBOARD, {})}` in that file).
2. The `CopyToClipboard` implementation at
`superset-frontend/src/components/CopyToClipboard/index.tsx` uses
`cloneElement` in
`getDecoratedCopyNode()` (around lines 79–99) to decorate any element passed
via
`copyNode`, adding `onClick: disabled ? undefined : onClick` (line 88) and
the new
`onKeyDown` handler (lines 89–96) that calls `event.preventDefault();
onClick();` when
`event.key` is `'Enter'` or `' '`.
3. Run a query in SQL Lab so that `ResultSet` renders, then use keyboard Tab
navigation in
the browser to focus the "Copy to Clipboard" button produced by the `Button`
passed as
`copyNode` in `ResultSet/index.tsx` (this is the element being cloned and
decorated by
`CopyToClipboard`).
4. Press Space or Enter while that button is focused: the key press first
dispatches a
`keydown` event, invoking the injected `onKeyDown` and calling `onClick()`
once; on keyup,
native button semantics synthesize a `click` event, which then triggers the
same `onClick`
prop a second time. Because `onClick()` calls `copyToClipboard()` and
`copyToClipboard()`
runs `onCopyEnd` in its `finally` block (`CopyToClipboard/index.tsx` lines
46–61), the
copy-to-clipboard success toast and
`logAction(LOG_ACTIONS_SQLLAB_COPY_RESULT_TO_CLIPBOARD, {})` both execute
twice for a
single keyboard activation.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=efe73e2d6fb04956b9b549812667f551&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=efe73e2d6fb04956b9b549812667f551&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/CopyToClipboard/index.tsx
**Line:** 89:96
**Comment:**
*Incorrect Condition Logic: The new keyboard handler triggers
`onClick()` for every custom element, including native interactive controls
like `button` that already fire click on Enter/Space. This causes duplicate
copy executions (double toast / double callback) for existing call sites that
pass button-based `copyNode`s. Only synthesize keyboard activation for
non-native interactive elements (or when role-based button behavior is being
polyfilled), and let native controls keep their default keyboard-to-click
behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41958&comment_hash=483bd4525ffa906b8b53206479e81250e5579241049563605d528c420c372520&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41958&comment_hash=483bd4525ffa906b8b53206479e81250e5579241049563605d528c420c372520&reaction=dislike'>👎</a>
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
