test(DropdownMenu): pointer and keyboard hybrid interaction (#1814)#2091
test(DropdownMenu): pointer and keyboard hybrid interaction (#1814)#2091kotAPI wants to merge 3 commits into
Conversation
|
📝 WalkthroughWalkthroughA new test file ChangesDropdownMenu Hybrid Interaction Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. Comment |
Code reviewReviewed and pushed follow-up fixes addressing handler composition, test patterns ( CI was green before fixes; please re-run checks on latest commit. |
CoverageThis report compares the PR with the base branch. "Δ" shows how the PR affects each metric.
Coverage improved or stayed the same. Great job! Run |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/components/ui/DropdownMenu/tests/DropdownMenu.hybridInteraction.test.tsx`:
- Around line 20-29: The pointer-to-keyboard flow test currently only verifies
the menu is gone after selection, so it can miss a broken open state. Update the
test in DropdownMenu.hybridInteraction.test.tsx, around the pointer open and
keyboard navigation sequence in pointer open then keyboard navigation selects
item, to assert the menu is actually open immediately after
user.click(screen.getByText('Open')) and before sending keyboard input. Use the
existing screen queries and the menu item text 'One' to check the positive
opened state, then keep the selection and focus assertions as they are.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c946e37-b51e-47bd-9be9-40500f19cdbd
📒 Files selected for processing (1)
src/components/ui/DropdownMenu/tests/DropdownMenu.hybridInteraction.test.tsx
| test('pointer open then keyboard navigation selects item', async() => { | ||
| const user = userEvent.setup(); | ||
| render(menu()); | ||
|
|
||
| await user.click(screen.getByText('Open')); | ||
| await user.keyboard('{ArrowDown}{ArrowDown}{Enter}'); | ||
|
|
||
| await waitFor(() => expect(screen.queryByText('One')).not.toBeInTheDocument()); | ||
| await waitFor(() => expect(screen.getByText('Open')).toHaveFocus()); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a positive “menu opened” assertion in the pointer→keyboard flow
This test can pass even if opening breaks: queryByText('One') is only checked after selection, so it already passes when the menu never appears. Assert open state before keyboard selection.
Suggested patch
test('pointer open then keyboard navigation selects item', async() => {
const user = userEvent.setup();
render(menu());
await user.click(screen.getByText('Open'));
+ expect(await screen.findByText('One')).toBeInTheDocument();
await user.keyboard('{ArrowDown}{ArrowDown}{Enter}');
await waitFor(() => expect(screen.queryByText('One')).not.toBeInTheDocument());
await waitFor(() => expect(screen.getByText('Open')).toHaveFocus());
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('pointer open then keyboard navigation selects item', async() => { | |
| const user = userEvent.setup(); | |
| render(menu()); | |
| await user.click(screen.getByText('Open')); | |
| await user.keyboard('{ArrowDown}{ArrowDown}{Enter}'); | |
| await waitFor(() => expect(screen.queryByText('One')).not.toBeInTheDocument()); | |
| await waitFor(() => expect(screen.getByText('Open')).toHaveFocus()); | |
| }); | |
| test('pointer open then keyboard navigation selects item', async() => { | |
| const user = userEvent.setup(); | |
| render(menu()); | |
| await user.click(screen.getByText('Open')); | |
| expect(await screen.findByText('One')).toBeInTheDocument(); | |
| await user.keyboard('{ArrowDown}{ArrowDown}{Enter}'); | |
| await waitFor(() => expect(screen.queryByText('One')).not.toBeInTheDocument()); | |
| await waitFor(() => expect(screen.getByText('Open')).toHaveFocus()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/DropdownMenu/tests/DropdownMenu.hybridInteraction.test.tsx`
around lines 20 - 29, The pointer-to-keyboard flow test currently only verifies
the menu is gone after selection, so it can miss a broken open state. Update the
test in DropdownMenu.hybridInteraction.test.tsx, around the pointer open and
keyboard navigation sequence in pointer open then keyboard navigation selects
item, to assert the menu is actually open immediately after
user.click(screen.getByText('Open')) and before sending keyboard input. Use the
existing screen queries and the menu item text 'One' to check the positive
opened state, then keep the selection and focus assertions as they are.
2 participants
Summary
cover mixed pointer and keyboard selection paths
Test plan
Related to #1814
Summary by CodeRabbit