Implement keyboard navigation improvements in HtmlEditor#34335
Implement keyboard navigation improvements in HtmlEditor#34335Raushen wants to merge 10 commits into
Conversation
…HTMLEditor_KBN
There was a problem hiding this comment.
Pull request overview
This PR improves DevExtreme HtmlEditor keyboard accessibility by adding a “focus escape” shortcut (Ctrl+Shift+Up/Down) to move focus out of the editing area (and into the toolbar when available), and by localizing the editor content aria-label to include usage instructions.
Changes:
- Added Ctrl+Shift+Up/Down handling in HtmlEditor to move focus into the toolbar (Up) or to the nearest focusable element outside the editor (Up/Down).
- Introduced reusable focus utilities (
getFirstFocusableElement,getNextFocusableElement,getPreviousFocusableElement) and centralized the “all focusable elements” selector inm_selectors. - Updated QUnit, Jest, and TestCafe coverage plus TestCafe models to validate focus behavior and toolbar-item focus state.
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/testcafe-models/toolbar/toolbarItem.ts | Adds a TestCafe model wrapper to expose toolbar item focus state. |
| packages/testcafe-models/htmlEditor/rootToolbar.ts | Updates HtmlEditor toolbar model to return ToolbarItem for focus assertions and element access. |
| packages/devextreme/testing/tests/DevExpress.ui.widgets.htmlEditor/htmlEditorParts/markup.tests.js | Updates aria-label expectation to use localized message keys. |
| packages/devextreme/js/localization/messages/ar.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/bg.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/ca.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/cs.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/da.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/de.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/el.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/en.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/es.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/fa.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/fi.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/fr.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/hi.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/hu.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/it.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/ja.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/ko.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/lt.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/lv.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/nb.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/nl.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/pl.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/pt.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/pt-BR.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/pt-PT.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/ro.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/ru.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/sl.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/sv.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/tr.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/uk.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/vi.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/zh.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/zh-CN.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/zh-Hans.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/localization/messages/zh-tw.json | Adds HtmlEditor aria-label message keys. |
| packages/devextreme/js/__internal/ui/toolbar/toolbar.ts | Adds focusFirstItem() to focus the first focusable toolbar element. |
| packages/devextreme/js/__internal/ui/popover/popover.ts | Adds a TS suppression comment for overlay stack typing. |
| packages/devextreme/js/__internal/ui/html_editor/themes/m_base.ts | Sets HtmlEditor content aria-label via localized message keys (label + escape instructions). |
| packages/devextreme/js/__internal/ui/html_editor/modules/m_toolbar.ts | Switches toolbar import and exposes focusFirstItem() on the HtmlEditor toolbar module. |
| packages/devextreme/js/__internal/ui/html_editor/html_editor.ts | Implements Ctrl+Shift+Up/Down focus escape logic and makes toolbar method calls return values. |
| packages/devextreme/js/__internal/ui/html_editor/tests/focus_escape.test.ts | Adds Jest coverage for focus escape behavior. |
| packages/devextreme/js/__internal/ui/tests/mock/model/html_editor.ts | Adds a DOM-based HtmlEditor test model for keyboard simulation. |
| packages/devextreme/js/__internal/grids/new/grid_core/keyboard_navigation/const.ts | Re-exports focusable selector constant from m_selectors to avoid duplication. |
| packages/devextreme/js/__internal/core/utils/m_selectors.ts | Centralizes the focusable selector and exports isElementVisible. |
| packages/devextreme/js/__internal/core/utils/focus.ts | Adds shared focus-navigation helpers for first/next/previous focusable elements. |
| packages/devextreme/js/__internal/core/utils/focus.test.ts | Adds Jest tests for the new focus helpers. |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/keyboard.ts | Adds E2E coverage for Ctrl+Shift+Up/Down focus escape in HtmlEditor. |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/dialogs/aiDialog/common.ts | Updates click target to the new toolbar-item model (.element). |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/dialogs/addImage/common.ts | Updates toolbar item clicks to use .element from the new model. |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/dialogs/addImage/addImageUrl.ts | Updates toolbar item clicks to use .element from the new model. |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/dialogs/addImage/addImageFromDevice.ts | Updates toolbar item clicks to use .element from the new model. |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/common.ts | Updates AI toolbar interaction to click the new model’s .element. |
| e2e/testcafe-devextreme/tests/accessibility/htmlEditor.ts | Updates AI toolbar interaction to click the new model’s .element. |
| e2e/testcafe-devextreme/helpers/domUtils.ts | Adds helper to insert a focusable element after a target (used by new E2E tests). |
| @@ -0,0 +1,102 @@ | |||
| import { | |||
There was a problem hiding this comment.
Why a new jest tests file? we have all htmleditor related tests here in QUnit:
packages/devextreme/testing/tests/DevExpress.ui.widgets.htmlEditor/htmlEditorParts
There was a problem hiding this comment.
It is Jest test, not qUnit. Jest - it is modern testing platform. It allow us use Typescript and create fast, stable unit tests.
There was a problem hiding this comment.
I know, but all our tests are written using QUnit and starting writing new tests in Jest might lead to a mess, I guess this should be resolved separately: either we move all our tests to Jest or we continue writing QUnits.
There was a problem hiding this comment.
Actually no, we already have a lot of Jest tests and there is no any mess. Jest tests have their own structure, qUnit - its own, TestCafe - its own. For example we do not need move all TestCafe tests to qUnit or vice versa. We just choose more convenient way to write tests.
There was a problem hiding this comment.
You're right that we don't need to move TestCafe tests to QUnit, but I think that's a bit different because they solve different problems. TestCafe is for E2E tests, while QUnit and Jest are both unit test frameworks.
I actually agree that Jest is a more modern option. My concern is consistency. All existing HTMLEditor unit tests are in the htmlEditorParts folder, and they're all (as well as other components in Components area) written in QUnit. If we start adding new unit tests in Jest alongside QUnit for the same component, it becomes less obvious where to look for tests and which framework should be used for new ones.
That's why I see two reasonable options:
- decide that we're migrating HTMLEditor (and other component's) unit tests to Jest (or even Vitest, since it's even better and more modern test framework and we have plans to move to Vite as far as I know), and gradually move existing tests using boyscout rule as well as for other components;
- or continue writing HTMLEditor unit tests in QUnit until we make that migration decision.
Without such a decision, I think mixing two unit testing frameworks within the same area of the codebase will make the test suite less consistent over time, even if both frameworks coexist in the repository.
Although I think we should consider this approach but it's up to you :)
No description provided.