-
Notifications
You must be signed in to change notification settings - Fork 491
fix: Consistent keydown handling for EditableText and TagsInput escape key #8204
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
Conversation
📝 WalkthroughWalkthroughSwaps keyboard event bindings from Changes
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/21/2026, 07:23:03 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests: ✅ PassedResults: 507 passed, 0 failed, 0 flaky, 8 skipped (Total: 515) 📊 Browser Reports
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.4 kB (baseline 22.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.02 MB (baseline 1.02 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 430 kB (baseline 430 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 8 added / 8 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.8 kB (baseline 2.8 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 32.8 kB (baseline 32.8 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 3.05 MB (baseline 3.05 MB) • 🟢 -24 BStores, services, APIs, and repositories
Status: 7 added / 7 removed Utilities & Hooks — 18.8 kB (baseline 18.8 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 4 added / 4 removed Vendor & Third-Party — 10.4 MB (baseline 10.4 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 6.28 MB (baseline 6.28 MB) • ⚪ 0 BBundles that do not match a named category
Status: 25 added / 25 removed |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/extensions/vueNodes/components/NodeHeader.test.ts (1)
166-179: Inconsistent: Escape test still useskeyupinstead ofkeydown.Line 172 uses
keyup.escapebutEditableText.vuenow binds to@keydown.escape. This test should be updated tokeydown.escapefor consistency with the component changes.Proposed fix
- await input.trigger('keyup.escape') + await input.trigger('keydown.escape')
|
1 similar comment
|
|
…e key (#8204) ## Summary This PR improves keyboard event handling consistency and fixes an issue where pressing Escape in nested input components would unintentionally close parent modals/dialogs. ## Changes ### Keyboard Event Fixes **TagsInput Escape Key Handling** - Added `@keydown.escape.stop` handler to `TagsInputInput.vue` to prevent Escape from bubbling up and closing parent modals - The handler blurs the input and exits editing mode without propagating the event **EditableText keyup → keydown Migration** - Changed `@keyup.enter` to `@keydown.enter` and `@keyup.escape` to `@keydown.escape` - Using `keydown` is more consistent with how other UI frameworks handle these events and provides more responsive feedback - Updated corresponding unit tests to use `keydown` triggers ### Why keydown over keyup? - `keydown` fires immediately when the key is pressed, providing faster perceived response - Better consistency with browser/OS conventions for action triggers - Prevents default behaviors (like form submission) more reliably when needed - Aligns with other keyboard handlers in the codebase ## Testing - Updated `EditableText.test.ts` to use `keydown` events - Updated `NodeHeader.test.ts` to use `keydown.enter` - Manual testing: Escape in TagsInput no longer closes parent modal ## Checklist - [x] Unit tests updated - [x] Keyboard event handlers consistent - [x] No breaking changes to component API ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8204-fix-Consistent-keydown-handling-for-EditableText-and-TagsInput-escape-key-2ef6d73d365081f0ac6bed8bcae57657) by [Unito](https://www.unito.io)
…xt and TagsInput escape key (#8238) Backport of #8204 to `cloud/1.37`. Cherry-picked merge commit `7b701ad07b1c34d121448e21d6f8b5c13ef07d73`. ## Original PR Summary This PR improves keyboard event handling consistency and fixes an issue where pressing Escape in nested input components would unintentionally close parent modals/dialogs. ### Changes - **EditableText keyup → keydown Migration**: Changed `@keyup.enter` to `@keydown.enter` and `@keyup.escape` to `@keydown.escape` for more consistent and responsive feedback - Updated corresponding unit tests to use `keydown` triggers > **Note**: The TagsInput escape key handling changes from the original PR are not included in this backport because the TagsInput component (#8066) was added after the cloud/1.37 branch was created. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8238-backport-cloud-1-37-fix-Consistent-keydown-handling-for-EditableText-and-TagsInput-esc-2f06d73d365081288e5ed0c656d78412) by [Unito](https://www.unito.io)
…e key (Comfy-Org#8204) ## Summary This PR improves keyboard event handling consistency and fixes an issue where pressing Escape in nested input components would unintentionally close parent modals/dialogs. ## Changes ### Keyboard Event Fixes **TagsInput Escape Key Handling** - Added `@keydown.escape.stop` handler to `TagsInputInput.vue` to prevent Escape from bubbling up and closing parent modals - The handler blurs the input and exits editing mode without propagating the event **EditableText keyup → keydown Migration** - Changed `@keyup.enter` to `@keydown.enter` and `@keyup.escape` to `@keydown.escape` - Using `keydown` is more consistent with how other UI frameworks handle these events and provides more responsive feedback - Updated corresponding unit tests to use `keydown` triggers ### Why keydown over keyup? - `keydown` fires immediately when the key is pressed, providing faster perceived response - Better consistency with browser/OS conventions for action triggers - Prevents default behaviors (like form submission) more reliably when needed - Aligns with other keyboard handlers in the codebase ## Testing - Updated `EditableText.test.ts` to use `keydown` events - Updated `NodeHeader.test.ts` to use `keydown.enter` - Manual testing: Escape in TagsInput no longer closes parent modal ## Checklist - [x] Unit tests updated - [x] Keyboard event handlers consistent - [x] No breaking changes to component API ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8204-fix-Consistent-keydown-handling-for-EditableText-and-TagsInput-escape-key-2ef6d73d365081f0ac6bed8bcae57657) by [Unito](https://www.unito.io)
Summary
This PR improves keyboard event handling consistency and fixes an issue where pressing Escape in nested input components would unintentionally close parent modals/dialogs.
Changes
Keyboard Event Fixes
TagsInput Escape Key Handling
@keydown.escape.stophandler toTagsInputInput.vueto prevent Escape from bubbling up and closing parent modalsEditableText keyup → keydown Migration
@keyup.enterto@keydown.enterand@keyup.escapeto@keydown.escapekeydownis more consistent with how other UI frameworks handle these events and provides more responsive feedbackkeydowntriggersWhy keydown over keyup?
keydownfires immediately when the key is pressed, providing faster perceived responseTesting
EditableText.test.tsto usekeydowneventsNodeHeader.test.tsto usekeydown.enterChecklist
┆Issue is synchronized with this Notion page by Unito