codeant-ai-for-open-source[bot] commented on code in PR #38112:
URL: https://github.com/apache/superset/pull/38112#discussion_r2831712709
##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -2512,23 +2508,23 @@ const AlertReportModal:
FunctionComponent<AlertReportModalProps> = ({
<div className="filters-add-container">
{filterNativeFilterOptions().length > 0 && (
// eslint-disable-next-line
jsx-a11y/anchor-is-valid
- <a
+ <Button
className="filters-add-btn"
- role="button"
+ buttonStyle="link"
tabIndex={0}
onClick={() => {
handleAddFilterField();
add();
}}
- onKeyDown={e => {
+ onKeyDown={(e: any) => {
if (e.key === 'Enter' || e.key === ' ') {
handleAddFilterField();
add();
}
}}
>
Review Comment:
**Suggestion:** The new `Button` already handles keyboard activation
(Enter/Space) by firing its `onClick`, so keeping the custom `onKeyDown`
handler that also calls the add logic will cause keyboard users pressing Enter
or Space to add the same filter twice; removing the redundant `onKeyDown`
avoids this double-trigger while preserving accessibility. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboard alert/report filters duplicate when using keyboard activation.
- ⚠️ Keyboard-only users see confusing duplicate filter entries.
- ⚠️ Validation and UX around native filters become inconsistent.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable the ALERT_REPORTS_FILTER feature flag so that filters are
available in the
Alert/Report modal (see `filtersEnabled =
isFeatureEnabled(FeatureFlag.AlertReportsFilter);` in
`superset-frontend/src/features/alerts/AlertReportModal.tsx` around lines
593–598).
2. In the Superset UI, open the Alerts & Reports workflow and create or edit
a
report/alert so that `AlertReportModal` (same file, exported at the bottom)
is rendered.
3. In the modal, under the "Contents" panel, select "Dashboard" as the
content type and
choose a dashboard that has native filters so that
`filterNativeFilterOptions().length >
0` evaluates true (this controls rendering of the add button in the filters
section around
lines 2459–2520).
4. Tab keyboard focus to the "+ Apply another dashboard filter" button
rendered from the
`<Button>` block at lines 2512–2528, then press Space or Enter: the `Button`
component
(from `@superset-ui/core/components`, imported at line 37) will fire its
`onClick` handler
once due to keyboard activation, and the custom `onKeyDown` handler will
also call
`handleAddFilterField(); add();`, resulting in two new filter rows being
added instead of
one.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/alerts/AlertReportModal.tsx
**Line:** 2520:2525
**Comment:**
*Logic Error: The new `Button` already handles keyboard activation
(Enter/Space) by firing its `onClick`, so keeping the custom `onKeyDown`
handler that also calls the add logic will cause keyboard users pressing Enter
or Space to add the same filter twice; removing the redundant `onKeyDown`
avoids this double-trigger while preserving accessibility.
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%2F38112&comment_hash=47cb3b5c4d09c6ae8c5a9c28c5d58480ad6b723fc7694df6c1d98aa364049b9f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38112&comment_hash=47cb3b5c4d09c6ae8c5a9c28c5d58480ad6b723fc7694df6c1d98aa364049b9f&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]
