fix(dashboard): restore arrow key navigation in modal inputs opened from dropdown#37978
fix(dashboard): restore arrow key navigation in modal inputs opened from dropdown#37978Ujjwaljain16 wants to merge 2 commits into
Conversation
…input keyboard navigation
Code Review Agent Run #e6b531Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
|
/review |
Code Review Agent Run #50bd80Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Code Review Agent Run #4d3257Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
|
I tested it in versions 6.0 and 6.1, but the key still doesn't work. Could I have applied something incorrectly? However, it seems to function properly after changing it to the code below. superset-frontend/src/dashboard/components/SaveModal.tsx This only makes the dashboard name input field work; it does not resolve the issues with the other input fields. |
EnxDev
left a comment
There was a problem hiding this comment.
Thanks for the writeup. A few concerns before this lands:
-
The new tests don't exercise the bug. Both render
ModalTriggerin isolation, but the issue only manifests when it's nested insideDropdown > Menu > Menu.Item. Neither test would catch a regression. The first one also pins implementation details (stopPropagationcalled,preventDefaultnot) rather than behavior. Could you replace them with a test that renders the actual dropdown-menu structure and asserts the dropdown closes + keys reach the input? -
@chkang-idk2's comment is worth addressing. They tested on 6.0/6.1 and report the keys still don't work with this approach. Could you confirm via DevTools that after the click, the dropdown's
aria-expandedflips tofalseand focus moves into the modal input? If it doesn't, the root cause may be focus management rather than event propagation, and the fix is incomplete. -
ModalTriggeris generic and widely used. Worth agit blameon the originalpreventDefault()to see why it was added — iftriggerNodeever contains an anchor or sits in a form, dropping it changes behavior for callers outside this PR's scope.
SUMMARY
Fixes an issue where Arrow (Left/Right), Home, and End keys did not work inside input fields in modals opened from the dashboard header dropdown (e.g., “Save as” dashboard modal and “Theme & CSS” modal).
Root Cause
ModalTrigger.open()callede.preventDefault(), which interfered with Ant Design’sMenuclick handling. As a result, the parentDropdowndid not properly close when the modal opened.Since the dropdown remained mounted in the DOM,
rc-menu’s internal keyboard navigation handler continued intercepting navigation keys (Arrow, Home, End) for ARIA menu navigation. This prevented normal cursor movement inside modal input fields.Alphanumeric typing was unaffected because
rc-menuonly intercepts navigation keys.Fix
Replaced
e.preventDefault()withe.stopPropagation()inModalTrigger.open().This ensures:
This is a minimal and safe change limited to click handling within
ModalTrigger.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable (keyboard navigation behavior change).
TESTING INSTRUCTIONS
Manual verification:
Open any dashboard (View or Edit mode).
Click the triple-dot menu in the top-right.
Select “Save as”.
In the “Dashboard name” input:
Close the modal.
Open Settings → Theme & CSS.
Repeat arrow/Home/End key checks in the Theme name input.
Re-open the dashboard dropdown and verify:
Automated tests:
Existing
ModalTriggertests pass.Added regression tests verifying:
stopPropagation()is used instead ofpreventDefault().ADDITIONAL INFORMATION