codeant-ai-for-open-source[bot] commented on code in PR #42006:
URL: https://github.com/apache/superset/pull/42006#discussion_r3572862871
##########
superset-frontend/packages/superset-ui-core/src/components/RefreshLabel/index.tsx:
##########
@@ -36,6 +36,7 @@ const RefreshLabel = ({
<Icons.SyncOutlined
iconSize="l"
role="button"
+ tabIndex={disabled ? -1 : 0}
Review Comment:
**Suggestion:** The new focusability makes this icon reachable via keyboard,
but there is still no keyboard activation handler, so pressing Enter/Space will
not trigger the refresh action on this non-button element. Add keyboard
handling for Enter/Space (or render a semantic button) so focused users can
activate it. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Keyboard users cannot refresh catalogs in selector.
- ⚠️ Catalog and schema refresh relies solely on mouse interaction.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Datasource Editor UI, which renders the TableSelector control
for physical
datasets
(superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:1995-2034
shows "<TableSelector>" used inside the form).
2. In the TableSelector component
(superset-frontend/src/components/TableSelector/index.tsx:320-359), tab into
the
table-select row until focus reaches the refresh icon created by the
RefreshLabel usage at
lines 332-336 (`const refreshLabel = !readOnly && (RefreshLabel onClick={()
=> refetch()}
tooltipContent=...)`).
3. Inspect the RefreshLabel definition
(superset-frontend/packages/superset-ui-core/src/components/RefreshLabel/index.tsx:30-46),
which renders Icons.SyncOutlined with `role="button"`, `tabIndex={disabled ?
-1 : 0}`, and
an `onClick` handler but no `onKeyDown`/`onKeyUp`/`onKeyPress` handlers.
4. With keyboard focus on the RefreshLabel icon, press Enter or Space;
because the
underlying element is a non-button with only a mouse click handler, no
refresh occurs for
keyboard users, even though the element is focusable and exposes
`role="button"`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e14da1b3fe7f43b38ea42409aab78200&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=e14da1b3fe7f43b38ea42409aab78200&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/packages/superset-ui-core/src/components/RefreshLabel/index.tsx
**Line:** 39:39
**Comment:**
*Incomplete Implementation: The new focusability makes this icon
reachable via keyboard, but there is still no keyboard activation handler, so
pressing Enter/Space will not trigger the refresh action on this non-button
element. Add keyboard handling for Enter/Space (or render a semantic button) so
focused users can activate it.
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%2F42006&comment_hash=79f4124a0b9d8e978515f054392b9711e5f0873329f4e1c435fc03f7cd2433e9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=79f4124a0b9d8e978515f054392b9711e5f0873329f4e1c435fc03f7cd2433e9&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/TableExploreTree/TreeNodeRenderer.tsx:
##########
@@ -239,6 +239,7 @@ const TreeNodeRenderer: React.FC<TreeNodeRendererProps> = ({
<div
className="side-action-container"
role="menu"
+ tabIndex={0}
onClick={e => e.stopPropagation()}
Review Comment:
**Suggestion:** Adding tab focus to this `role="menu"` container introduces
a keyboard stop on an element that has no keyboard behavior and no
focus-state-driven action visibility, so users can land on an apparently inert
control. Either remove it from tab order or implement proper menu keyboard
semantics/focus handling. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Extra inert tab stop in SQL Lab table tree.
- ⚠️ Menu role suggests keyboard behavior that is missing.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In SQL Lab, open a query editor so the left sidebar renders
`SqlEditorLeftBar`
(superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:200-239),
which
includes `<TableExploreTree queryEditorId={activeQEId} />` at line 8.
2. In TableExploreTree
(superset-frontend/src/SqlLab/components/TableExploreTree/index.tsx:23-80),
navigate with
Tab until you reach a schema-level tree node; `TreeNodeRenderer` is used as
the renderer
(TreeNodeRenderer.tsx:101-199) and, when `identifier === 'schema'`, creates
the
side-action container at lines 39-46.
3. Observe that the schema side-action container element
(TreeNodeRenderer.tsx:40-44) is a
`div` with `className="side-action-container"`, `role="menu"`,
`tabIndex={0}`, and
`onClick={e => e.stopPropagation()}`; its children are the actual
`ActionButton` controls
for refresh and pin operations (lines 48-99).
4. With keyboard focus on this container, press Enter or Space: no refresh
or pin action
is triggered because the container has no keyboard event handlers and its
only click
handler merely stops propagation, so the focus stop appears inert despite
being tabbable
and labeled as a menu.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e0c54177a21a4511a37747e3e08588c1&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=e0c54177a21a4511a37747e3e08588c1&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/SqlLab/components/TableExploreTree/TreeNodeRenderer.tsx
**Line:** 243:243
**Comment:**
*Incomplete Implementation: Adding tab focus to this `role="menu"`
container introduces a keyboard stop on an element that has no keyboard
behavior and no focus-state-driven action visibility, so users can land on an
apparently inert control. Either remove it from tab order or implement proper
menu keyboard semantics/focus handling.
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%2F42006&comment_hash=a64e5b22dffb9ef1d79b1a80d6dfbdd743d258f547122f63b2dc81854b8b96a6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=a64e5b22dffb9ef1d79b1a80d6dfbdd743d258f547122f63b2dc81854b8b96a6&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/TableExploreTree/TreeNodeRenderer.tsx:
##########
@@ -307,6 +308,7 @@ const TreeNodeRenderer: React.FC<TreeNodeRendererProps> = ({
<div
className="side-action-container"
role="menu"
+ tabIndex={0}
Review Comment:
**Suggestion:** This second `role="menu"` side-action container has the same
issue: it is now tabbable without corresponding keyboard interaction logic,
creating a non-functional focus target in each table row. Add keyboard
handlers/menu semantics or avoid making the container tabbable. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Inert tab stop for each table row actions.
- ⚠️ Keyboard navigation through actions becomes more cumbersome.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Using the same SQL Lab left sidebar (`SqlEditorLeftBar`,
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:200-239),
tab into the
table tree rendered by `TableExploreTree`.
2. In `TreeNodeRenderer`
(superset-frontend/src/SqlLab/components/TableExploreTree/TreeNodeRenderer.tsx:101-199),
when `identifier === 'table'`, a second `div` with
`className="side-action-container"` is
rendered at lines 109-114 with `role="menu"`, `tabIndex={0}`, and
`onClick={e =>
e.stopPropagation()}`.
3. This container wraps the per-table `ActionButton` controls for
copy-select, sort
columns, refresh schema, pin/unpin, and expand/collapse
(TreeNodeRenderer.tsx:117-190),
but the container itself has no keyboard handlers other than the
propagation-stopping
click.
4. When keyboard focus lands on this tabbable menu container for a table row
and the user
presses Enter or Space, none of the underlying actions are invoked; the user
must tab
again into each `ActionButton`, making the container an inert intermediate
focus target in
every row.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b43352da911e4ca7ba79d323fcc4063b&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=b43352da911e4ca7ba79d323fcc4063b&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/SqlLab/components/TableExploreTree/TreeNodeRenderer.tsx
**Line:** 311:311
**Comment:**
*Incomplete Implementation: This second `role="menu"` side-action
container has the same issue: it is now tabbable without corresponding keyboard
interaction logic, creating a non-functional focus target in each table row.
Add keyboard handlers/menu semantics or avoid making the container tabbable.
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%2F42006&comment_hash=e56665b9cf5bbbea438702d3a21a161ca1c43b993f806f69e31eba6be1973e79&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=e56665b9cf5bbbea438702d3a21a161ca1c43b993f806f69e31eba6be1973e79&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx:
##########
@@ -142,6 +142,7 @@ export const ChartsScopingListPanel = ({
</AddButtonContainer>
<FilterTitle
role="button"
+ tabIndex={0}
Review Comment:
**Suggestion:** This title is now tabbable but still relies only on
`onClick` on a styled `div`, so keyboard users can focus it but cannot activate
it with Enter/Space. Add a key handler or use a semantic button element to
avoid a focus stop that does nothing. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Keyboard users cannot toggle global scoping in modal.
- ⚠️ Cross-filter scoping UI partially inaccessible to keyboard.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In a dashboard, open the cross-filter scoping UI by triggering the
`ScopingModal`
component
(superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx:43-69),
typically via the native filter bar configuration controls.
2. Inside `ScopingModal`, the left panel renders `ChartsScopingListPanel`
(superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx:50-24)
with props including `setCurrentChartId` and `activeChartId`.
3. In `ChartsScopingListPanel` (ChartsScopingListPanel.tsx:32-51), the "All
charts/global
scoping" title is rendered as `FilterTitle` with `role="button"`,
`tabIndex={0}`, and
`onClick={() => setCurrentChartId(undefined)}`, and
`className={activeChartId ===
undefined ? 'active' : ''}`.
4. The `FilterTitle` component itself is defined as a styled `div`
(superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx:38-26)
with cursor and hover styles but no keyboard event handling; thus, when a
keyboard user
tabs to the "All charts/global scoping" element and presses Enter or Space,
the click
handler is not triggered and global scoping cannot be activated via
keyboard, despite the
element being focusable and presented as a button.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=97ffcb77bf4c4d3ea6a308b7ab9b033b&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=97ffcb77bf4c4d3ea6a308b7ab9b033b&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/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx
**Line:** 145:145
**Comment:**
*Incomplete Implementation: This title is now tabbable but still relies
only on `onClick` on a styled `div`, so keyboard users can focus it but cannot
activate it with Enter/Space. Add a key handler or use a semantic button
element to avoid a focus stop that does nothing.
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%2F42006&comment_hash=e830337b374b7e14db93abdf2228926f1904da7f1aaf08fc297ea663716c34d2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=e830337b374b7e14db93abdf2228926f1904da7f1aaf08fc297ea663716c34d2&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]
