-
Notifications
You must be signed in to change notification settings - Fork 523
fix(kit): escape key propagation in preview dialog to prevent closing parent dialog #11419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Tests are running 🚀Wait for workflow run with tests to finish ☕ |
projects/kit/components/preview/dialog/test/preview-dialog.escape.spec.ts
Outdated
Show resolved
Hide resolved
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this 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 fixes a bug where pressing the Escape key in a preview dialog opened within a regular dialog would close both dialogs simultaneously instead of closing only the preview dialog first.
- Modified the
TuiPreviewDialog
component to prevent escape key event propagation by addingpreventDefault()
andstopPropagation()
calls - Added comprehensive Cypress tests to verify the layered escape key behavior works correctly
- Ensures proper dialog nesting behavior where escape keys are handled by the topmost dialog first
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
projects/kit/components/preview/dialog/preview-dialog.component.ts |
Added onEscape method to handle escape key events with proper event propagation prevention |
projects/demo-cypress/src/tests/preview-dialog.cy.ts |
Added new test file to verify escape key behavior in nested dialog scenarios |
projects/kit/components/preview/dialog/preview-dialog.component.ts
Outdated
Show resolved
Hide resolved
View your CI Pipeline Execution ↗ for commit 05a8869
☁️ Nx Cloud last updated this comment at |
Visit the preview URL for this PR (updated for commit 05a8869): https://taiga-previews-demo--pr11419-copilot-fix-11401-demo-6xdnd5gg.web.app (expires Sun, 27 Jul 2025 17:21:49 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 73dddc3c665194f3e11f18c16aeb71af4c289c37 |
BundleMonFiles updated (1)
Unchanged files (4)
Total files change +26B 0% Groups updated (1)
Final result: ✅ View report in BundleMon website ➡️ |
Co-authored-by: splincode <12021443+splincode@users.noreply.github.com>
Co-authored-by: splincode <12021443+splincode@users.noreply.github.com>
Co-authored-by: splincode <12021443+splincode@users.noreply.github.com>
Co-authored-by: splincode <12021443+splincode@users.noreply.github.com>
Co-authored-by: splincode <12021443+splincode@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
9132bb2
to
9a37711
Compare
Playwright test resultsDetails
Flaky testschromium › tests/kit/dropdown-hover/dropdown-hover.pw.spec.ts › DropdownHover › Examples › With DropdownMobile › Opens mobile version of dropdown on the 2nd time click Skipped testschromium › tests/addon-mobile/mobile-dropdown/mobile-dropdown-with-textfield.pw.spec.ts › DropdownMobile for textfields › with legacy select |
projects/kit/components/preview/dialog/preview-dialog.component.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: splincode <12021443+splincode@users.noreply.github.com>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Problem
When a preview dialog is opened inside a regular dialog, pressing Escape closes both components simultaneously instead of closing only the preview dialog first.
Reproduction:
Root Cause
Both
TuiPreviewDialog
andTuiDialogCloseService
listen for escape key events on the document:(document:keydown.esc)
host binding!event.defaultPrevented
checkThe preview dialog handles the escape key but doesn't prevent event propagation, so the parent dialog also receives the same event and closes simultaneously.
Solution
Modified
TuiPreviewDialog
to prevent escape key event propagation:Before:
After:
Technical Details
!event.defaultPrevented
before handling escape keyspreventDefault()
in the preview dialog, we ensure the dialog service ignores the already-handled escape eventExpected Behavior After Fix
Testing
Fixes #11401.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
cloud.nx.app
node /home/REDACTED/work/taiga-ui/taiga-ui/node_modules/.bin/nx serve demo
(dns block)/opt/hostedtoolcache/node/20.19.4/x64/bin/node /home/REDACTED/work/taiga-ui/taiga-ui/.nx/cache/cloud/2507.21.8/lib/heartbeat/heartbeat-process.js
(dns block)node /home/REDACTED/work/taiga-ui/taiga-ui/node_modules/.bin/nx build demo
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.