Skip to content

Popup: Close popup on Escape key press when focus is inside a child editor#32957

Merged
marker-dao merged 8 commits intoDevExpress:26_1from
r-farkhutdinov:26_1_popup_esc_handle
Mar 20, 2026
Merged

Popup: Close popup on Escape key press when focus is inside a child editor#32957
marker-dao merged 8 commits intoDevExpress:26_1from
r-farkhutdinov:26_1_popup_esc_handle

Conversation

@r-farkhutdinov
Copy link
Contributor

No description provided.

@r-farkhutdinov r-farkhutdinov self-assigned this Mar 18, 2026
@r-farkhutdinov r-farkhutdinov marked this pull request as ready for review March 18, 2026 15:27
@r-farkhutdinov r-farkhutdinov requested a review from a team as a code owner March 18, 2026 15:27
Copilot AI review requested due to automatic review settings March 18, 2026 15:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Popup keyboard behavior so pressing Escape closes the popup even when focus is inside a child element/editor (where ignoreChildEvents would otherwise prevent the Overlay-level Escape handler from running).

Changes:

  • Add a Popup._keyboardHandler override to close on Escape when the keydown originates from a child element inside the popup content.
  • Add a QUnit regression test covering Escape pressed on a child <input> element.
  • Add a React Storybook scenario demonstrating Escape behavior with editors inside a Popup.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js Adds a regression test ensuring Escape closes the popup when focus is on a child element.
packages/devextreme/js/__internal/ui/popup/m_popup.ts Implements Escape handling for keydown events originating from child elements within the popup content.
apps/react-storybook/stories/popup/data.ts Provides sample data for the new Popup Storybook story.
apps/react-storybook/stories/popup/Popup.stories.tsx Adds a Storybook example demonstrating Escape-to-close when focus is inside embedded editors.

You can also share your feedback on Copilot code review. Take the survey.

Copilot AI review requested due to automatic review settings March 18, 2026 15:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts dxPopup keyboard handling so that pressing Escape closes the popup even when focus is inside a child editor/element (instead of only when the popup content element itself is the event target). It also adds regression tests and a React Storybook example to demonstrate the behavior.

Changes:

  • Add Popup._keyboardHandler override to close on Escape when the keydown originates from a child element inside popup content (unless preventDefault() was called).
  • Add QUnit coverage for Escape behavior when focus is on a child element, including a preventDefault() scenario.
  • Add a React Storybook story demonstrating Escape handling with editors inside a popup.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/devextreme/js/__internal/ui/popup/m_popup.ts Overrides keyboard handling to close popup on Escape from child elements, with a preventDefault() escape hatch.
packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js Adds QUnit tests validating Escape closes popup from a child element and respects preventDefault().
apps/react-storybook/stories/popup/data.ts Provides sample data for the new popup story.
apps/react-storybook/stories/popup/Popup.stories.tsx Adds a Storybook scenario demonstrating Escape handling with TextBox/SelectBox/DateBox inside Popup.

You can also share your feedback on Copilot code review. Take the survey.

@r-farkhutdinov r-farkhutdinov marked this pull request as draft March 18, 2026 15:58
@r-farkhutdinov r-farkhutdinov force-pushed the 26_1_popup_esc_handle branch 2 times, most recently from 17a7efb to 66f643f Compare March 19, 2026 12:09
@r-farkhutdinov r-farkhutdinov marked this pull request as ready for review March 19, 2026 13:22
@r-farkhutdinov r-farkhutdinov requested review from a team as code owners March 19, 2026 13:22
Copilot AI review requested due to automatic review settings March 19, 2026 13:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates DevExtreme’s internal Popup keyboard handling so that pressing Escape closes the popup even when focus is inside a child editor/element (not just on the popup content root), while adding an internal escape-ignore flag for specific popups that must preserve existing nested-escape behavior.

Changes:

  • Add Popup._keyboardHandler override to close on Escape when the event target is a child element.
  • Introduce internal _ignoreCloseOnChildEscape option and enable it for toolbar overflow menu and grid column chooser popups.
  • Add QUnit coverage and a React Storybook scenario for Escape handling inside editors.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js Adds regression tests for Escape behavior when focus is on a child element, including preventDefault and ignore-flag scenarios.
packages/devextreme/js/__internal/ui/toolbar/internal/toolbar.menu.ts Sets _ignoreCloseOnChildEscape: true to preserve “Escape on nested items should NOT close popup” behavior in the toolbar overflow menu.
packages/devextreme/js/__internal/ui/popup/m_popup.ts Implements new Escape-from-child close behavior and introduces _ignoreCloseOnChildEscape.
packages/devextreme/js/__internal/grids/grid_core/column_chooser/m_column_chooser.ts Sets _ignoreCloseOnChildEscape: true for the column chooser popup to preserve prior Escape behavior for nested focus targets.
apps/react-storybook/stories/popup/data.ts Adds demo data for the new Popup Storybook story.
apps/react-storybook/stories/popup/Popup.stories.tsx Adds a Storybook example demonstrating Escape closing the popup from inside editors.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the core Popup keyboard handling so pressing Escape closes the popup even when focus is inside a child element (e.g., an embedded editor), with opt-outs for specific internal popup usages.

Changes:

  • Add Popup-level handling for Escape keydown events originating from child elements, with a private opt-out flag.
  • Add QUnit coverage for closing / preventDefault behavior / opt-out behavior.
  • Add a React Storybook example demonstrating Escape behavior from editors inside a Popup.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js Adds keyboard navigation tests covering Escape behavior when focus is inside child elements.
packages/devextreme/js/__internal/ui/toolbar/internal/toolbar.menu.ts Sets _ignoreCloseOnChildEscape: true for the toolbar dropdown menu popup to preserve its Escape behavior.
packages/devextreme/js/__internal/ui/popup/m_popup.ts Overrides _keyboardHandler to close on child-originated Escape and introduces _ignoreCloseOnChildEscape.
packages/devextreme/js/__internal/grids/grid_core/column_chooser/m_column_chooser.ts Sets _ignoreCloseOnChildEscape: true for the column chooser popup.
apps/react-storybook/stories/popup/data.ts Adds sample data for the new Popup Storybook story.
apps/react-storybook/stories/popup/Popup.stories.tsx Adds a Storybook story demonstrating Escape closing behavior from child editors.

You can also share your feedback on Copilot code review. Take the survey.

anna-shakhova
anna-shakhova previously approved these changes Mar 19, 2026

useFlatToolbarButtons?: boolean;

_ignoreCloseOnChildEscape?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should set a default value for this property as well, just as we do for all the others.

Copy link
Contributor

@marker-dao marker-dao Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved 002c48d

@marker-dao marker-dao self-assigned this Mar 20, 2026
Copilot AI review requested due to automatic review settings March 20, 2026 12:10
marker-dao
marker-dao previously approved these changes Mar 20, 2026
pharret31
pharret31 previously approved these changes Mar 20, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the internal dxPopup keyboard handling so that pressing Escape closes the popup even when focus is inside a nested child element/editor, while providing an internal opt-out for components that have custom Escape behavior.

Changes:

  • Added Popup._keyboardHandler logic to close on Escape when the event target is a child element (unless prevented or explicitly ignored).
  • Introduced a private option _ignoreCloseOnChildEscape (default false) and enabled it for internal popups that should preserve existing Escape behavior (e.g., toolbar overflow menu, column chooser).
  • Added QUnit coverage for child-focus Escape closing, preventDefault() behavior, and the opt-out flag; added a React Storybook example to manually verify editor-focused Escape behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/devextreme/testing/tests/DevExpress.ui.widgets/popup.tests.js Adds regression tests for Escape handling when focus is inside child elements, including preventDefault and opt-out behavior.
packages/devextreme/js/__internal/ui/popup/m_popup.ts Implements “Escape from child closes popup” behavior and introduces _ignoreCloseOnChildEscape option.
packages/devextreme/js/__internal/ui/toolbar/internal/toolbar.menu.ts Sets _ignoreCloseOnChildEscape: true to preserve toolbar menu’s custom Escape behavior.
packages/devextreme/js/__internal/grids/grid_core/column_chooser/m_column_chooser.ts Sets _ignoreCloseOnChildEscape: true for the column chooser popup to avoid changing Escape behavior there.
apps/react-storybook/stories/popup/data.ts Adds sample data used by the new Popup story.
apps/react-storybook/stories/popup/Popup.stories.tsx Adds a Storybook scenario demonstrating Escape behavior when focus is inside embedded editors.

anna-shakhova
anna-shakhova previously approved these changes Mar 20, 2026
@marker-dao marker-dao dismissed stale reviews from anna-shakhova, pharret31, and themself via c6649b5 March 20, 2026 13:04
@marker-dao marker-dao merged commit 3da65f3 into DevExpress:26_1 Mar 20, 2026
129 of 130 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants