codeant-ai-for-open-source[bot] commented on code in PR #42006:
URL: https://github.com/apache/superset/pull/42006#discussion_r3572850313
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx:
##########
@@ -139,6 +139,7 @@ const ItemTitleContainer = forwardRef<HTMLDivElement,
Props>(
<ItemTitle
Review Comment:
**Suggestion:** Setting `tabIndex={0}` on this custom `role="tab"` element
without keyboard interaction logic creates a focusable tab that cannot be
activated via keyboard input. Add keyboard handling (Enter/Space and tablist
navigation behavior) or use semantic tab controls to keep tab interaction
functional. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Filter config tabs not activatable via keyboard input.
- ⚠️ Keyboard users cannot switch filter being configured.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the native filters configuration modal, which renders
`ItemTitlePane` and, inside
it, `ItemTitleContainer` as shown in `ItemTitlePane.tsx:13-29` (the
component passes
`items`, `currentItemId`, and `onChange` into `ItemTitleContainer`).
2. In `ItemTitleContainer.tsx:127-188`, `renderComponent(id)` returns
`ItemTitle` with
`role="tab"`, `tabIndex={0}`, `onClick={() => onChange(id)}`, and
`aria-selected={isActive}`, making each filter title behave visually like a
tab and
clickable with the mouse.
3. Using keyboard-only navigation in the Filters Config modal, tab through
the list of
filter titles until focus lands on a non-active `ItemTitle` element (the
focusable tab is
the styled `div` defined at `ItemTitleContainer.tsx:32-57`).
4. Press Enter or Space while the tab has focus; because `ItemTitle` only
wires `onClick`
and there are no `onKeyDown`/`onKeyUp` handlers on `ItemTitle` or its
container, the key
press does not call `onChange(id)`, so the tab selection does not change and
keyboard
users cannot activate or switch tabs.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1a6549db1517412d905aee3aa90e9bb5&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=1a6549db1517412d905aee3aa90e9bb5&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/FiltersConfigModal/ItemTitleContainer.tsx
**Line:** 139:146
**Comment:**
*Incomplete Implementation: Setting `tabIndex={0}` on this custom
`role="tab"` element without keyboard interaction logic creates a focusable tab
that cannot be activated via keyboard input. Add keyboard handling (Enter/Space
and tablist navigation behavior) or use semantic tab controls to keep tab
interaction functional.
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=87d50a2a6c62f8fe974feebdd5453240b9639e886f9ea6a32f97a4570f664141&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=87d50a2a6c62f8fe974feebdd5453240b9639e886f9ea6a32f97a4570f664141&reaction=dislike'>👎</a>
##########
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx:
##########
@@ -115,6 +115,7 @@ const AdhocMetricEditPopoverTitle:
FC<AdhocMetricEditPopoverTitleProps> = ({
data-test="AdhocMetricEditTitle#trigger"
onMouseOver={handleMouseOver}
onMouseOut={handleMouseOut}
+ onFocus={handleMouseOver}
Review Comment:
**Suggestion:** Adding focus to the hover handler sets the hovered state for
keyboard users, but this component’s blur path does not clear that hover state,
so the edit icon can remain visually “hovered” after focus moves away. Pair the
focus behavior with a blur handler that resets the hover flag. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Metric label edit icon stays hovered after blur.
⚠️ Keyboard users see misleading hover/focus feedback.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render `AdhocMetricEditPopoverTitle` from
`superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx`
using the existing test harness in
`AdhocMetricEditPopoverTitle.test.tsx:36-48`, which
calls `render(<AdhocMetricEditPopoverTitle ... />)`.
2. Observe the non-edit view span at
`AdhocMetricEditPopoverTitle.tsx:113-121` with
`data-test="AdhocMetricEditTitle#trigger"`, which wires
`onMouseOver={handleMouseOver}`,
`onMouseOut={handleMouseOut}`, `onFocus={handleMouseOver}` (line 118), and
`onBlur={handleBlur}`.
3. Note that `handleMouseOver` and `handleMouseOut` only toggle the hover
flag (`const
[isHovered, setIsHovered] = useState(false);` and `const handleMouseOver =
useCallback(()
=> setIsHovered(true), []); const handleMouseOut = useCallback(() =>
setIsHovered(false),
[]);` at lines 58-65), while `handleBlur` only updates edit mode (`const
handleBlur =
useCallback(() => setIsEditMode(false), []);` at line 67) and does not reset
`isHovered`.
4. Using keyboard navigation (e.g., `userEvent.tab()` or `fireEvent.focus`
then
`fireEvent.blur` on `AdhocMetricEditTitle#trigger` in
`AdhocMetricEditPopoverTitle.test.tsx`), focus the span to trigger
`onFocus={handleMouseOver}` and set `isHovered=true`, then blur it; because
`onBlur` calls
`handleBlur` which does not clear `isHovered`, the icon color logic at
`AdhocMetricEditPopoverTitle.tsx:48` (`iconColor={isHovered ?
theme.colorPrimary :
theme.colorIcon}`) continues to render the hovered visual state even after
focus has moved
away.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c1d2a1454ad14bf09670598e97b96a10&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=c1d2a1454ad14bf09670598e97b96a10&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/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx
**Line:** 118:118
**Comment:**
*Logic Error: Adding focus to the hover handler sets the hovered state
for keyboard users, but this component’s blur path does not clear that hover
state, so the edit icon can remain visually “hovered” after focus moves away.
Pair the focus behavior with a blur handler that resets the hover flag.
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=bcc252b76b9644cd1f5e5444dd2bd9d01825fe3c2d189b260d864ea6f35377ce&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=bcc252b76b9644cd1f5e5444dd2bd9d01825fe3c2d189b260d864ea6f35377ce&reaction=dislike'>👎</a>
##########
superset-frontend/src/explore/components/controls/OptionControls/index.tsx:
##########
@@ -350,6 +350,7 @@ export const OptionControlLabel = ({
role="button"
data-test="remove-control-button"
onClick={onRemove}
+ tabIndex={0}
Review Comment:
**Suggestion:** This element was made focusable but still has only a mouse
click handler, so keyboard users can tab to it but cannot activate removal with
Enter/Space. Add keyboard activation handling (or use a native button) so
focusable behavior matches click behavior. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Option removal control unreachable via keyboard activation.
⚠️ Explore option controls expose non-functional keyboard tab stop.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render `OptionControlLabel` from
`superset-frontend/src/explore/components/controls/OptionControls/index.tsx`
as done in
`OptionControls.test.tsx:45`, which mounts `<OptionControlLabel
{...defaultProps} />`.
2. Inspect the close/remove UI in `OptionControls/index.tsx:21-35`, where
`getOptionControlContent()` returns a `CloseContainer` element with
`role="button"`,
`data-test="remove-control-button"`, `onClick={onRemove}`, and the newly
added
`tabIndex={0}` on line 353.
3. Note that `CloseContainer` is a styled container component (defined
earlier in the same
file) and there are no `onKeyDown`, `onKeyUp`, or `onKeyPress` handlers
attached to
`CloseContainer` to mirror the click behavior for keyboard activation; only
`onClick={onRemove}` is present.
4. In a keyboard-only interaction (for example, in a test using
`userEvent.tab()` to focus
`screen.getByTestId('remove-control-button')` and then
`userEvent.keyboard('{Enter}')`),
focus moves to the `CloseContainer` because of `tabIndex={0}`, but pressing
Enter or Space
does not fire `onClick` on this non-native button element, so `onRemove` is
never called
and the control cannot be activated via keyboard.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f52f784a88a54e6c96f7cfddbef52364&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=f52f784a88a54e6c96f7cfddbef52364&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/explore/components/controls/OptionControls/index.tsx
**Line:** 353:353
**Comment:**
*Incomplete Implementation: This element was made focusable but still
has only a mouse click handler, so keyboard users can tab to it but cannot
activate removal with Enter/Space. Add keyboard activation handling (or use a
native button) so focusable behavior matches 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%2F42006&comment_hash=9e54c5de03f432755b2c860f2ea6bc71a5f2d41186b7d81768264c89b68aa120&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=9e54c5de03f432755b2c860f2ea6bc71a5f2d41186b7d81768264c89b68aa120&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/queries/QueryPreviewModal.tsx:
##########
@@ -148,6 +148,7 @@ function QueryPreviewModal({
<QueryViewToggle>
<TabButton
role="button"
+ tabIndex={0}
Review Comment:
**Suggestion:** The tab toggles are now focusable but still rely only on
`onClick`, so focused tabs do not switch via Enter/Space on keyboard
interaction. Add key handling (or switch to native button semantics) so the
newly focusable controls are operable. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Query preview tabs not operable via keyboard input.
⚠️ QueryHistory preview modal fails accessibility keyboard expectations.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Query History page implemented in
`superset-frontend/src/pages/QueryHistoryList/index.tsx`; when
`queryCurrentlyPreviewing`
is truthy, the page renders `<QueryPreviewModal ... />` at lines 21-29 with
props
including `query`, `queries`, `fetchData`, `openInSqlLab`, and `show`.
2. Inside `QueryPreviewModal`
(`superset-frontend/src/features/queries/QueryPreviewModal.tsx:27-57`),
observe the tab
toggle UI: `QueryViewToggle` wraps two `TabButton` elements, each with
`role="button"`,
`tabIndex={0}`, and `onClick={() => setCurrentTab('user')}` or `onClick={()
=>
setCurrentTab('executed')}` respectively (lines 30-45).
3. Confirm that neither `TabButton` receives any keyboard event handlers
(`onKeyDown`,
`onKeyUp`, `onKeyPress`) to translate Enter/Space key presses into calls to
`setCurrentTab`; activation is solely wired through `onClick`, assuming
pointer
interaction.
4. In actual usage (or a test harness for `QueryPreviewModal`), open the
modal via the
Query History page, use `userEvent.tab()` to move keyboard focus to the
"User query" or
"Executed query" tab (made focusable via `tabIndex={0}` on line 151 and its
counterpart at
line 160), then press Enter or Space; because `TabButton` is not a native
`button` element
and has no key handlers, no click is fired and `setCurrentTab` is not
invoked, leaving the
current tab unchanged and making the focusable tab controls inoperable from
the keyboard.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dbd34600b1db470ea160a83cfcf14bad&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=dbd34600b1db470ea160a83cfcf14bad&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/features/queries/QueryPreviewModal.tsx
**Line:** 151:151
**Comment:**
*Incomplete Implementation: The tab toggles are now focusable but still
rely only on `onClick`, so focused tabs do not switch via Enter/Space on
keyboard interaction. Add key handling (or switch to native button semantics)
so the newly focusable controls are operable.
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=89fb53e2c7aa1a07d1f2b5fdba2630043e17e531f8cb6c16c512390f8ba83b4a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42006&comment_hash=89fb53e2c7aa1a07d1f2b5fdba2630043e17e531f8cb6c16c512390f8ba83b4a&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]
