codeant-ai-for-open-source[bot] commented on code in PR #38791:
URL: https://github.com/apache/superset/pull/38791#discussion_r2981206476
##########
superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx:
##########
@@ -243,28 +245,17 @@ test('Arrow key navigation switches focus between
indicators', () => {
);
// Query the indicators
- const firstIndicator = screen.getByRole('button', {
- name: 'search Clinical Stage',
- });
- const secondIndicator = screen.getByRole('button', {
- name: 'search Age Group',
- });
+ const firstMenuItem = screen.queryByText('Clinical Stage')?.closest('li')!;
+ const secondMenuItem = screen.queryByText('Age Group')?.closest('li')!;
- // Focus the first indicator
- firstIndicator.focus();
- expect(firstIndicator).toHaveFocus();
+ expect(firstMenuItem).toBeInTheDocument();
+ expect(secondMenuItem).toBeInTheDocument();
- // Simulate ArrowDown key press
- fireEvent.keyDown(document.activeElement as Element, {
- key: 'ArrowDown',
- code: 'ArrowDown',
- });
- expect(secondIndicator).toHaveFocus();
+ // Focus the first indicator
+ firstMenuItem.focus();
+ expect(firstMenuItem).toHaveFocus();
- // Simulate ArrowUp key press
- fireEvent.keyDown(document.activeElement as Element, {
- key: 'ArrowUp',
- code: 'ArrowUp',
- });
- expect(firstIndicator).toHaveFocus();
+ // Focus the second indicator
+ secondMenuItem.focus();
Review Comment:
**Suggestion:** This test claims to validate arrow-key navigation, but it
only moves focus programmatically with `.focus()`. As written, it will still
pass even if keyboard navigation is broken, so regressions in actual
ArrowUp/ArrowDown handling won't be detected. Trigger keyboard events and
assert that focus changes as a result. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Keyboard navigation regressions may pass CI unnoticed.
- ⚠️ DetailsPanel a11y behavior is not truly validated.
```
</details>
```suggestion
// Navigate to the second indicator with ArrowDown
fireEvent.keyDown(firstMenuItem, { key: 'ArrowDown', code: 'ArrowDown' });
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run test `Arrow key navigation switches focus between indicators` in
`superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx:221`.
2. The test renders `DetailsPanel` and queries two menu `<li>` elements at
`DetailsPanel.test.tsx:248-249`.
3. It then directly calls `firstMenuItem.focus()` and
`secondMenuItem.focus()`
(`DetailsPanel.test.tsx:255,259`) instead of dispatching ArrowUp/ArrowDown
keyboard
events.
4. In component code, `DetailsPanel` delegates keyboard behavior to AntD
`Menu`
(`superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx:106-110`)
and contains no explicit arrow-key assertions in the test path.
5. Therefore, this test passes even though no arrow-key interaction is
exercised, so real
keyboard-navigation regressions would not be detected by this test.
```
</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/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx
**Line:** 259:259
**Comment:**
*Logic Error: This test claims to validate arrow-key navigation, but it
only moves focus programmatically with `.focus()`. As written, it will still
pass even if keyboard navigation is broken, so regressions in actual
ArrowUp/ArrowDown handling won't be detected. Trigger keyboard events and
assert that focus changes as a result.
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%2F38791&comment_hash=e04fc21e7713b8a1841cc21cb80eb33dba3d984b48e8b82af44cd53391e663fa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38791&comment_hash=e04fc21e7713b8a1841cc21cb80eb33dba3d984b48e8b82af44cd53391e663fa&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]
