codeant-ai-for-open-source[bot] commented on code in PR #38342:
URL: https://github.com/apache/superset/pull/38342#discussion_r2986805379
##########
superset-frontend/src/dashboard/components/IconButton.tsx:
##########
@@ -22,6 +22,7 @@ import { styled } from '@apache-superset/core/ui';
interface IconButtonProps {
icon: JSX.Element;
label?: string;
+ 'aria-label'?: string;
onClick: MouseEventHandler<HTMLDivElement>;
}
Review Comment:
**Suggestion:** The new accessibility prop is optional even when the button
has no visible text, so icon-only usages can still render a `role="button"`
element with no accessible name. This breaks screen-reader interaction and WCAG
name requirements. Make the props a discriminated union so either `label` or
`aria-label` is always provided. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SQL Lab table pin button lacks accessible name.
- ⚠️ WCAG 4.1.2 non-compliance in tree action controls.
```
</details>
```suggestion
type IconButtonProps = {
icon: JSX.Element;
onClick: MouseEventHandler<HTMLDivElement>;
} & (
| {
label: string;
'aria-label'?: string;
}
| {
label?: string;
'aria-label': string;
}
);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open SQL Lab, which renders `TableExploreTree` in
`superset-frontend/src/SqlLab/components/TableExploreTree/index.tsx:110-238`
(`TableExploreTree` function).
2. `TableExploreTree` passes `renderNode` to `react-arborist` at
`index.tsx:216-226`,
which renders `TreeNodeRenderer` for each node.
3. For table nodes, `TreeNodeRenderer` renders `<IconButton ... onClick ...
/>` without
`label` or `aria-label` at
`superset-frontend/src/SqlLab/components/TableExploreTree/TreeNodeRenderer.tsx:226-236`.
4. In `IconButton`
(`superset-frontend/src/dashboard/components/IconButton.tsx:43-53`),
`aria-label` is set as `ariaLabel || label`; with both missing, rendered
`role="button"`
has no accessible name.
5. Result: screen-reader users encounter an unnamed pin action control in
SQL Lab table
tree; discriminated-union props would raise a TypeScript error at this
callsite and
prevent this regression.
```
</details>
<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/IconButton.tsx
**Line:** 22:27
**Comment:**
*Logic Error: The new accessibility prop is optional even when the
button has no visible text, so icon-only usages can still render a
`role="button"` element with no accessible name. This breaks screen-reader
interaction and WCAG name requirements. Make the props a discriminated union so
either `label` or `aria-label` is always provided.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38342&comment_hash=7dec66957e15a0e084cd8f673c966997032a67b44193a089c5ed05b46e3e8385&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38342&comment_hash=7dec66957e15a0e084cd8f673c966997032a67b44193a089c5ed05b46e3e8385&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]
