Fix Tab key not confirming highlighted dropdown suggestion (#27112) - #27155
Fix Tab key not confirming highlighted dropdown suggestion (#27112)#27155nitishagar wants to merge 1 commit into
Conversation
I am not clear what you mean when you say "Recommended manual verification". Did you perform the verification yourself? If not, please do so. Including screenshots or a screen recording is one way to show that you've performed the expected interactive testing. This is the second pull request that you've opened. It appears you may not have read the contributing guidelines. Please read and follow them. Until I closed your previous pull request, you had more open pull requests than are allowed for a new contributor. That means you didn't follow the instructions in the must read issue. Please read and follow the instructions in the must read issue: |
MarkEWaite
left a comment
There was a problem hiding this comment.
Requesting changes to indicate that there are questions in my earlier review.
There was a problem hiding this comment.
Pull request overview
Restores Tab-to-confirm behavior for highlighted dropdown suggestions by reintroducing the DOM-property wiring (element.onkeypress) that the existing keyboard navigation/router code path invokes.
Changes:
- Adds
tryOnKeyPressEvent()to wiredropdownItem.onKeyPressonto the rendered menu item aselement.onkeypress. - Calls
tryOnKeyPressEvent()frommenuItem()alongside existing click and other special-case wiring.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@MarkEWaite Thank you for taking the time. Apologies for missing the contribution guidelines. I'll keep only this pull request open until it is accepted or rejected before opening any others. |
PR jenkinsci#11204 (commit 611294d) rewrote menuItem() in templates.js and dropped the assignment of options.onKeyPress onto the rendered element's onkeypress DOM property, while keeping the equivalent onClick wiring (tryOnClickEvent). The keyboard router in utils.js consumes the suggestion via that DOM property: default: if (selectedItem.onkeypress) { selectedItem.onkeypress(evt); } Because menuItem no longer sets item.onkeypress, it is always undefined, so the Tab-to-confirm handler defined by both producers (combo-box.js, autocomplete.js) never runs. Enter and mouse-click still work because they route through onClick / selectedItem.click(), which was preserved. Restore the wiring by adding a tryOnKeyPressEvent helper alongside tryOnClickEvent, mirroring the existing try* convention. It assigns element.onkeypress = opt.onKeyPress as a DOM property (not a listener) so the existing utils.js consumer can invoke it. It early-returns when onKeyPress is absent, so plain menu items (hetero-list, context menus, jumplists, overflow) and the submenu/split-button branch (which returns before the try* block) are unaffected. Fixes jenkinsci#27112
8c48dd5 to
85512d5
Compare

Fixes #27112
Root cause
PR #11204 (commit
611294d365) rewrotemenuItem()intemplates.jsand dropped the assignment ofoptions.onKeyPressonto the rendered element'sonkeypressDOM property, while keeping the equivalentonClickwiring (tryOnClickEvent).The keyboard router in
utils.jsconsumes the suggestion via that DOM property:Because
menuItemno longer setsitem.onkeypress, it is alwaysundefined, so the Tab-to-confirm handler defined by both producers (combo-box.js,autocomplete.js) never runs.Enterand mouse-click still work because they route throughonClick/selectedItem.click(), which was preserved.Testing done
yarn lint→ passes (eslint . && prettier --check .+stylelint src/main/scss, all green, exit 0)yarn build→ passes (webpack production build compiles, exit 0)element.onkeypress = opt.onKeyPressas a DOM property, which is exactly what the existing consumer atutils.js:167reads and invokes (selectedItem.onkeypress(evt)). The producer handlers (combo-box.js:14-20,autocomplete.js:24-30) runconfirm()+e.dropdown.hide()+evt.preventDefault()onevt.key === "Tab", so Tab will now confirm a highlighted suggestion identically to Enter.Enterand mouse-click are unaffected: their path (tryOnClickEventclick listener +keyboard.jsselectedItem.click()) is untouched.ComboBoxTest.javais HtmlUnit-based and cannot synthesize a realkeydownevent that traversesmakeKeyboardNavigable. The fix is verified by the surgical correctness of restoring the exact pre-Introduce experimental API for adding actions to experimental Run UI #11204 wiring (property assignment) that the existing consumer depends on.<f:combobox>//project-relationshipfield (combobox), type to show suggestions, arrow-down to highlight one, and press Tab — the value should be filled and the dropdown hidden, identical to pressing Enter.Screenshots (UI changes only)
Not a visual change; behavior-only fix.
Before
After
Proposed changelog entries
Proposed changelog category
/label regression-fix
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention