codeant-ai-for-open-source[bot] commented on code in PR #36991:
URL: https://github.com/apache/superset/pull/36991#discussion_r2674350277
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/styles/index.tsx:
##########
@@ -287,14 +287,14 @@ export const StyledChartContainer = styled.div<{
.dt-is-filter {
cursor: pointer;
:hover {
- background-color: ${theme.colorPrimaryBgHover};
+ background-color: ${theme.colorBgSpotlight};
}
}
.dt-is-active-filter {
background: ${theme.colorPrimaryBg};
:hover {
Review Comment:
**Suggestion:** The new styles add only `:hover` rules which are not exposed
to keyboard users; this is an accessibility bug. Add `:focus-visible` (or
`:focus`) alongside `:hover` so keyboard focus shows the same visual state as
hover. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
:hover,
:focus-visible {
background-color: ${theme.colorBgSpotlight};
}
}
.dt-is-active-filter {
background: ${theme.colorPrimaryBg};
:hover,
:focus-visible {
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is an accessibility improvement — keyboard users won't see the
hover-only visual affordance.
Adding :focus-visible (or :focus) aligns keyboard focus visuals with hover,
which is a real UX/QA issue rather than a cosmetic preference.
The suggestion addresses a meaningful accessibility gap introduced by the
new hover rules.
</details>
<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/styles/index.tsx
**Line:** 289:296
**Comment:**
*Possible Bug: The new styles add only `:hover` rules which are not
exposed to keyboard users; this is an accessibility bug. Add `:focus-visible`
(or `:focus`) alongside `:hover` so keyboard focus shows the same visual state
as hover.
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>
##########
superset-frontend/plugins/plugin-chart-table/src/Styles.tsx:
##########
@@ -120,12 +120,12 @@ export default styled.div`
}
td.dt-is-filter:hover {
- background-color: ${theme.colorPrimaryBgHover};
+ background-color: ${theme.colorBgSpotlight};
}
Review Comment:
**Suggestion:** Accessibility / contrast risk: the hover rule for filterable
cells only sets a background color without ensuring the foreground text color
provides sufficient contrast on that new background; if
`theme.colorBgSpotlight` is darker/lighter than the default row background,
text may become unreadable. Add an explicit foreground color (and keep it
consistent across hover/focus) so contrast is preserved. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
color: ${theme.colorText};
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is a sensible accessibility hardening: the PR adds a new hover
background but does not guarantee the text color will maintain sufficient
contrast against that background. Adding an explicit color (e.g.
${theme.colorText}) is a low-risk, targeted improvement that prevents
regressions on themes where colorBgSpotlight would reduce readability.
</details>
<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-table/src/Styles.tsx
**Line:** 124:124
**Comment:**
*Possible Bug: Accessibility / contrast risk: the hover rule for
filterable cells only sets a background color without ensuring the foreground
text color provides sufficient contrast on that new background; if
`theme.colorBgSpotlight` is darker/lighter than the default row background,
text may become unreadable. Add an explicit foreground color (and keep it
consistent across hover/focus) so contrast is preserved.
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>
--
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]
