Lookup: Fix keyboard navigation (research for T1327208)#33375
Open
marker-dao wants to merge 5 commits intoDevExpress:26_1from
Open
Lookup: Fix keyboard navigation (research for T1327208)#33375marker-dao wants to merge 5 commits intoDevExpress:26_1from
marker-dao wants to merge 5 commits intoDevExpress:26_1from
Conversation
marker-dao
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Updates DevExtreme’s internal dxLookup/DropDownList behavior to improve keyboard navigation and focus handling, and adjusts the jQuery playground page to reproduce/inspect the behavior.
Changes:
- Propagate
focusStateEnabledinto Lookup popup toolbar buttons and adjust focus cleanup on search box blur. - Make Lookup list
tabIndexdepend onsearchEnabled(intended to keep focus in the search box when search is enabled). - Propagate
focusStateEnabledto the underlying List without desktop-only gating inDropDownList.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/devextreme/playground/jquery.html | Switches the playground widget from dxButton to dxLookup for keyboard-navigation research. |
| packages/devextreme/js/__internal/ui/m_lookup.ts | Updates focus/keyboard behavior for the Lookup popup (toolbar button focusStateEnabled, list tabIndex logic, focusedElement reset on search blur). |
| packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_list.ts | Adjusts List configuration/option propagation for focusStateEnabled and includes formatting cleanups. |
marker-dao
force-pushed
the
26_1_T1327208_lookup
branch
from
2be5899 to
358e04b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Updates DevExtreme’s Lookup/DropDownList focus behavior to improve keyboard navigation and focus management (per T1327208 research), and adjusts QUnit coverage to validate the new focus/tabIndex rules.
Changes:
- Lookup: make list
tabIndexdepend onsearchEnabled(and update it on runtime changes); clear listfocusedElementwhen the search box loses focus. - Lookup: pass
focusStateEnabledthrough to popup toolbar buttons (done/clear/cancel) and add focused/tabIndex regression tests. - DropDownList: propagate
focusStateEnabledto the inner List on mobile (hover state remains desktop-only) and update related tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/lookup.tests.js | Adds focus/tabIndex and toolbar focus-state tests; updates ARIA expectations for tabindex when searchEnabled toggles. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.editors/dropDownList.tests.js | Updates focus-policy tests to reflect list focusStateEnabled propagation on mobile. |
| packages/devextreme/js/__internal/ui/m_lookup.ts | Implements list tabIndex switching based on searchEnabled, clears list focus on search focus-out, and passes focusStateEnabled to toolbar button configs. |
| packages/devextreme/js/__internal/ui/drop_down_editor/m_drop_down_list.ts | Removes desktop-only restriction for list focusStateEnabled propagation and updates option-changed handling accordingly. |
Comment on lines
118
to
133
| @@ -129,7 +129,7 @@ QUnit.module('focus policy', { | |||
| this.instance.option({ hoverStateEnabled: true, focusStateEnabled: true }); | |||
|
|
|||
| assert.notOk(list.option('hoverStateEnabled'), 'hover state should not be changed on mobiles'); | |||
| assert.notOk(list.option('focusStateEnabled'), 'focus state should not be changed on mobiles'); | |||
| assert.ok(list.option('focusStateEnabled'), 'focus state should be changed on mobiles'); | |||
| } | |||
Comment on lines
+768
to
776
| const { focusStateEnabled, applyButtonText: text } = this.option(); | ||
|
|
||
| return [ | ||
| { | ||
| shortcut: 'done', | ||
| options: { | ||
| text, | ||
| focusStateEnabled, | ||
| onClick: this._applyButtonHandler.bind(this), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.