You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "amaannawab923 (via GitHub)" <gi...@apache.org> on 2026/06/24 12:56:28 UTC
[PR] fix(plugin-chart-ag-grid-table): select cell on click instead of its text [superset]
amaannawab923 opened a new pull request, #41392:
URL: https://github.com/apache/superset/pull/41392
### SUMMARY
Clicking a cell in the interactive table selected the *text* inside the cell rather than selecting the cell itself, which makes the usual click-to-select / copy workflow feel broken.
The grid had `enableCellTextSelection` turned on, which forces browser text selection on click and suppresses AG Grid's normal cell-focus behavior. Turning it off restores click-to-select-the-cell.
One caveat worth calling out: full multi-cell range selection is an AG Grid Enterprise feature and isn't available in the Community build, so this restores single-cell selection only.
### TESTING INSTRUCTIONS
1. Open an interactive table on a dashboard or in explore.
2. Click a cell.
3. The cell should get a focus/selection outline rather than putting a text cursor in the cell content.
Existing render tests for the table still pass.
### ADDITIONAL INFORMATION
- [x] Changes UI
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41392:
URL: https://github.com/apache/superset/pull/41392#issuecomment-4902957871
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41392:
URL: https://github.com/apache/superset/pull/41392#issuecomment-4789600052
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41392?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 64.42%. Comparing base ([`a57b5f6`](https://app.codecov.io/gh/apache/superset/commit/a57b5f6078056e411278cbb93d3595d4cdebe62a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bae3a1c`](https://app.codecov.io/gh/apache/superset/commit/bae3a1cb9023a3c39da1ca5c2c9facab70129879?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41392 +/- ##
=======================================
Coverage 64.42% 64.42%
=======================================
Files 2654 2654
Lines 145386 145386
Branches 33557 33557
=======================================
Hits 93658 93658
Misses 50031 50031
Partials 1697 1697
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41392/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/41392/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.73% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41392?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41392:
URL: https://github.com/apache/superset/pull/41392#discussion_r3534752571
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/test/cellSelectionWiring.test.tsx:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, waitFor } from '@superset-ui/core/spec';
+import { ProviderWrapper } from '../../plugin-chart-table/test/testHelpers';
+import testData from '../../plugin-chart-table/test/testData';
+
+// Capture the props the grid is rendered with, so we can assert the
+// cell-selection wiring without depending on AG Grid's DOM rendering.
+const captured: { props?: Record<string, any> } = {};
Review Comment:
**Suggestion:** Replace the `any` in the captured props record with a concrete props/value type so the test maintains strict typing. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The file introduces a TypeScript `any` in `Record<string, any>`, which directly violates the no-any-types rule for new or changed TypeScript code.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1c2bd553e0484ff9a7733dc837607bb0&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=1c2bd553e0484ff9a7733dc837607bb0&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/plugins/plugin-chart-ag-grid-table/test/cellSelectionWiring.test.tsx
**Line:** 25:25
**Comment:**
*Custom Rule: Replace the `any` in the captured props record with a concrete props/value type so the test maintains strict typing.
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%2F41392&comment_hash=95beed5c25e947baa5a544cdc272f9c8a0b58ed0ea94d68cf99fc405349e6517&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41392&comment_hash=95beed5c25e947baa5a544cdc272f9c8a0b58ed0ea94d68cf99fc405349e6517&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/test/cellSelectionWiring.test.tsx:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, waitFor } from '@superset-ui/core/spec';
+import { ProviderWrapper } from '../../plugin-chart-table/test/testHelpers';
+import testData from '../../plugin-chart-table/test/testData';
+
+// Capture the props the grid is rendered with, so we can assert the
+// cell-selection wiring without depending on AG Grid's DOM rendering.
+const captured: { props?: Record<string, any> } = {};
+
+// Mock the narrow ThemedAgGridReact module (which the components barrel
+// re-exports) rather than the whole barrel, to avoid its circular-init.
+jest.mock('@superset-ui/core/components/ThemedAgGridReact', () => ({
+ __esModule: true,
+ ThemedAgGridReact: (props: Record<string, any>) => {
Review Comment:
**Suggestion:** Change the mocked component parameter type from `any` to a specific AG Grid props interface (or a narrowed custom type) to satisfy strict typing requirements. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The mocked component parameter is typed with `Record<string, any>`, which includes `any` and therefore violates the no-any-types rule.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
.cursor/rules/dev-standard.mdc (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0df9fbb5c5a6442db87840621a8e5a0a&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=0df9fbb5c5a6442db87840621a8e5a0a&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/plugins/plugin-chart-ag-grid-table/test/cellSelectionWiring.test.tsx
**Line:** 31:31
**Comment:**
*Custom Rule: Change the mocked component parameter type from `any` to a specific AG Grid props interface (or a narrowed custom type) to satisfy strict typing requirements.
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%2F41392&comment_hash=3a1855581e7d5d88e03ce57df12bdbe4833add989e96beeb5e57be5b86fe8066&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41392&comment_hash=3a1855581e7d5d88e03ce57df12bdbe4833add989e96beeb5e57be5b86fe8066&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #41392:
URL: https://github.com/apache/superset/pull/41392#issuecomment-4834294379
## EnxDev's Review Agent — apache/superset#41392 · HEAD bae3a1c
comment — The one-line change fixes click-to-focus, but it likely removes the only way to copy a cell value in the Community build. Worth confirming before merge.
### 🔴 Functional
- **`plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:528`** · _Medium_ — Setting `enableCellTextSelection={false}` disables native browser text selection inside cells. Only Community modules are registered (`AllCommunityModule` + `ClientSideRowModelModule`, line 113); the clipboard/`copy`/`copyWithHeaders`/`paste` items (lines 601–603) are AG Grid Enterprise and inert here. So before this PR users could highlight cell text and Ctrl+C to copy a value — the only copy path in this build — and after it they can't copy at all (focused cell + no clipboard module = Ctrl+C does nothing). Net: the table loses copy-a-value entirely to gain a focus outline. Confirm the product intent is to drop text-copy; if not, this trade needs a different fix (e.g. a custom copy handler, or scoping the selection change to only what #106389 needs). **regression test:** render the grid, focus a cell, assert text-copy still works (or that the intended cell-selection state is set) — a Jest/RTL
test asserting the `enableCellTextSelection` prop value would at least pin the decision.
### 🟡 Should-fix
- No regression test ships with the behavioral change. "Existing render tests pass" doesn't guard this — a test should assert the new click-selects-cell behavior (or the prop value) so the choice isn't silently flipped back later.
### 🔵 Nits
- PR is still marked draft (`review:draft`) — flip to ready when you want a formal maintainer review.
<!-- enxdev-review-agent:bae3a1c -->
_Reviewed by EnxDev's Review Agent — @EnxDev · HEAD bae3a1c._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev merged PR #41392:
URL: https://github.com/apache/superset/pull/41392
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41392:
URL: https://github.com/apache/superset/pull/41392#discussion_r3534769732
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/copyCellValue.ts:
##########
@@ -0,0 +1,156 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * The Interactive Table selects the cell (not its text) on click (#106389).
+ * Because only AG Grid Community modules are registered, the Enterprise
+ * clipboard module is unavailable, so the browser's native text-selection was
+ * previously the *only* way to copy a cell value. To keep copy-a-value working,
+ * we provide a small Ctrl/Cmd+C handler that copies the focused cell's value.
+ */
+
+/**
+ * Keyboard fields we read to detect a copy shortcut. AG Grid types the event as
+ * the DOM `Event`, so we accept that and read the keyboard fields optionally
+ * (at runtime a cell key-down carries a `KeyboardEvent`).
+ */
+export type CopyKeyboardEvent = Partial<
+ Pick<KeyboardEvent, 'key' | 'ctrlKey' | 'metaKey'>
+> &
+ Partial<Event>;
+
+/**
+ * Minimal shape of the AG Grid `CellKeyDownEvent` we rely on. The event carries
+ * no pre-formatted value, so we re-run the column's `valueFormatter` ourselves.
+ */
+export interface CopyableCellParams {
+ event?: CopyKeyboardEvent | null;
+ value?: unknown;
+ colDef?: {
+ valueFormatter?: unknown;
+ } | null;
+ // Forwarded to the valueFormatter; Superset's reads `value` and `node`.
+ node?: unknown;
+ column?: unknown;
+ data?: unknown;
+ api?: unknown;
+ context?: unknown;
+}
+
+/** True when the event is Ctrl+C (Win/Linux) or Cmd+C (macOS). */
+export function isCopyShortcut(event?: CopyKeyboardEvent | null): boolean {
+ if (!event) {
+ return false;
+ }
+ const key = (event.key ?? '').toLowerCase();
+ return (event.ctrlKey === true || event.metaKey === true) && key === 'c';
+}
+
+/**
+ * Resolves the text to copy so it matches what the user sees: runs the column's
+ * `valueFormatter` (as the grid does when painting the cell) to copy the
+ * displayed value (`2,871`, `$2.87K`, `N/A`, ...). Falls back to the raw value
+ * when there is no formatter or it throws; null/undefined copy as empty string.
+ */
+export function getCellCopyText(params?: CopyableCellParams): string {
+ if (!params) {
+ return '';
+ }
+
+ const { valueFormatter } = params.colDef ?? {};
+ if (typeof valueFormatter === 'function') {
+ try {
+ const formatted = valueFormatter({
+ value: params.value,
+ node: params.node,
+ column: params.column,
+ colDef: params.colDef,
+ data: params.data,
+ api: params.api,
+ context: params.context,
+ });
+ if (formatted !== null && formatted !== undefined) {
+ return String(formatted);
+ }
+ } catch {
+ // Fall through to the raw value below.
+ }
+ }
+
+ const { value } = params;
+ if (value === null || value === undefined) {
+ return '';
+ }
+ return String(value);
+}
+
+/**
+ * Writes text to the clipboard, using the async Clipboard API when available
+ * and falling back to a hidden textarea + execCommand for non-secure contexts.
+ * Returns whether the write is believed to have succeeded.
+ */
+export async function writeTextToClipboard(text: string): Promise<boolean> {
+ try {
+ if (
+ typeof navigator !== 'undefined' &&
+ navigator.clipboard &&
+ typeof navigator.clipboard.writeText === 'function'
+ ) {
+ await navigator.clipboard.writeText(text);
+ return true;
+ }
+ } catch {
+ // Fall through to the legacy fallback below.
+ }
+
+ if (typeof document === 'undefined') {
+ return false;
+ }
+
+ try {
+ const textarea = document.createElement('textarea');
+ textarea.value = text;
+ textarea.setAttribute('readonly', '');
+ textarea.style.position = 'fixed';
+ textarea.style.top = '-1000px';
+ textarea.style.opacity = '0';
+ document.body.appendChild(textarea);
+ textarea.select();
+ const succeeded = document.execCommand('copy');
+ document.body.removeChild(textarea);
+ return succeeded;
+ } catch {
+ return false;
+ }
Review Comment:
**Suggestion:** The fallback clipboard path leaks a hidden textarea when `textarea.select()` or `document.execCommand('copy')` throws, because cleanup is only performed on the success path. Move DOM removal into a `finally` block (or equivalent guaranteed cleanup) so the temporary node is always removed even on exceptions. [missing cleanup]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Interactive table copy shortcut leaks hidden textarea nodes.
- ⚠️ Long-running dashboards may accumulate unnecessary DOM elements.
- ⚠️ Clipboard failure path not fully cleaned up.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a dashboard or Explore view that renders the Interactive Table chart, which uses
`AgGridDataTable` from
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx` (see
component definition at lines 120-153 and the `handleCellKeyDown` callback at lines
241-246 wiring `onCellKeyDown`).
2. Run the app in an environment where the async Clipboard API path is unavailable or
failing, e.g. `navigator.clipboard` is undefined or `navigator.clipboard.writeText`
rejects, which causes `writeTextToClipboard` in
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/copyCellValue.ts:108-121`
to fall through to the legacy `document.execCommand('copy')` fallback.
3. With the interactive table focused, press Ctrl+C (or Cmd+C on macOS) on a cell;
`onCellKeyDown` on the grid (wired at `src/AgGridTable/index.tsx:520-528`) calls
`handleCellKeyDown`, which invokes `copyCellValueOnKeyDown`
(`src/utils/copyCellValue.ts:148-155`), which in turn calls `writeTextToClipboard` hitting
the fallback block at lines 127-140 where a hidden textarea is appended and
`document.execCommand('copy')` is invoked.
4. Simulate or encounter a runtime where `textarea.select()` or
`document.execCommand('copy')` throws (mirroring the Jest test `writeTextToClipboard
returns false when both clipboard paths fail` at
`superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/copyCellValue.test.ts:39-52`,
where `execCommand` is mocked to throw); the exception causes control to jump to the
`catch` block at lines 138-140, which returns `false` without running
`document.body.removeChild(textarea)`, leaving the hidden textarea in the DOM. Repeating
the copy shortcut accumulates multiple hidden textarea elements, which can be observed in
browser devtools under the document body.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=50728f5a7a884e15a98e20eaa7d7f15f&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=50728f5a7a884e15a98e20eaa7d7f15f&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/plugins/plugin-chart-ag-grid-table/src/utils/copyCellValue.ts
**Line:** 133:140
**Comment:**
*Missing Cleanup: The fallback clipboard path leaks a hidden textarea when `textarea.select()` or `document.execCommand('copy')` throws, because cleanup is only performed on the success path. Move DOM removal into a `finally` block (or equivalent guaranteed cleanup) so the temporary node is always removed even on exceptions.
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%2F41392&comment_hash=e74e4370db89db010daf12345222fd458a48a215fb9438529f861d1627f78641&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41392&comment_hash=e74e4370db89db010daf12345222fd458a48a215fb9438529f861d1627f78641&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-ag-grid-table): select cell on click instead of its text [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41392:
URL: https://github.com/apache/superset/pull/41392#issuecomment-4789516011
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(ag-grid): select cell on click instead of its text [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41392:
URL: https://github.com/apache/superset/pull/41392#issuecomment-4877245761
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 03bf1c4e81068cb316ea22373111cf1b79b7901d |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a47c68ef933fb0008ac42e3 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41392--superset-docs-preview.netlify.app](https://deploy-preview-41392--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcqxDYAgEADAXb6WiEIQ3MAx8HkiCQJBCo1xd-muuBdKzWdpsAIMsFeb8Oj24R4nroQ2DClGdlEkbCGnnhyVmJ_N9aasXFBp8kYIv3POtUU5k4DvBw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41392--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org