-
Notifications
You must be signed in to change notification settings - Fork 448
Fix copy not working when text is selected in dialogs #7166
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
The graph node copy handler was intercepting Ctrl+C/Cmd+C even when the user had text selected in dialogs (like error dialogs). This made it impossible to copy error messages or other text from modal dialogs. Add a hasTextSelection() check to shouldIgnoreCopyPaste() so that when the user has text selected anywhere in the document, the default browser copy behavior is used instead of the graph copy handler.
📝 WalkthroughWalkthroughAdded an internal hasTextSelection() helper to detect non-empty document text selections and updated shouldIgnoreCopyPaste to include that detection, so copy/paste events are handled when text is selected in addition to existing text-input and linearMode checks. Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/04/2025, 09:25:54 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/04/2025, 09:35:25 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • ⚪ 0 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 973 kB (baseline 972 kB) • 🔴 +249 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 173 kB (baseline 173 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/scripts/app.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/scripts/app.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/scripts/app.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/scripts/app.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/scripts/app.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/scripts/app.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/scripts/app.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/scripts/app.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/scripts/app.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/scripts/app.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/scripts/app.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/scripts/app.ts
🧠 Learnings (3)
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Implement proper error handling
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.{ts,tsx,js,vue} : Implement proper error handling in components and services
Applied to files:
src/scripts/app.ts
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
|
@Myestery Backport to Please manually cherry-pick commit Conflicting files
|
1 similar comment
|
@Myestery Backport to Please manually cherry-pick commit Conflicting files
|
|
Should we backport this one? |
Yes, this issue has been highlighted in many forums |
|
Okay, I will do it manually now |
- Allow default browser copy (Ctrl+C / Cmd+C) when text is selected anywhere in the document - Previously, the graph node copy handler intercepted copy events even in dialogs Users could not copy error messages from the PromptExecutionError dialog or other modal dialogs. When pressing Ctrl+C with text selected in a dialog, the graph copy handler would intercept the event and prevent the default browser copy behavior. Add a `hasTextSelection()` check to `shouldIgnoreCopyPaste()`. When the user has any text selected in the document, the function returns `true`, allowing the default browser copy to proceed. - [ ] Open an error dialog (trigger a workflow error) - [ ] Click "Show Report" to expand error details - [ ] Select some text in the dialog - [ ] Press Ctrl+C (or Cmd+C on Mac) - [ ] Paste elsewhere to verify the text was copied - [ ] Verify graph node copy still works when no text is selected https://github.com/user-attachments/assets/30a0c501-95ee-4148-b321-3d60339a41c5 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7166-Fix-copy-not-working-when-text-is-selected-in-dialogs-2bf6d73d36508182a240fd3153cb6969) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <[email protected]>
…alogs (#7269) ## Summary Backport of #7166 to core/1.33 branch. - Fixes copy not working when text is selected in dialogs - Also includes workflow priority fix (workflow checked before parameters) ## Conflicts Resolved - `src/scripts/app.ts`: Accepted incoming changes for workflow priority logic ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7269-backport-core-1-33-Fix-copy-not-working-when-text-is-selected-in-dialogs-2c46d73d365081b690bfc9dc1548618e) by [Unito](https://www.unito.io) Co-authored-by: Johnpaul Chiwetelu <[email protected]> Co-authored-by: GitHub Action <[email protected]>
## Summary - When backports fail due to merge conflicts, the comment now includes a copyable prompt for AI coding assistants - Styled similar to CodeRabbit's "Prompt for AI Agents" feature - Prompt includes all necessary context: PR URL, merge commit, target branch, conflict files, resolution guidelines ## Example Output When a backport fails due to conflicts, the workflow now posts a cleaner comment with an AI agent prompt: --- ###⚠️ Backport to `core/1.33` failed **Reason:** Merge conflicts detected during cherry-pick of `5233749` <details> <summary>📄 Conflicting files</summary> - src/scripts/app.ts </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Backport PR #7166 (#7166) to core/1.33. Cherry-pick merge commit 5233749 onto new branch backport-7166-to-core-1.33 from origin/core/1.33. Resolve conflicts in: src/scripts/app.ts. For test snapshots (browser_tests/**/*-snapshots/), accept PR version if changed in original PR, else keep target. For package.json versions, keep target branch. For pnpm-lock.yaml, regenerate with pnpm install. Ask user for non-obvious conflicts. Create PR titled "[backport core/1.33] <original title>" with label "backport". See .github/workflows/pr-backport.yaml for workflow details. ``` </details> --- The "Prompt for AI Agents" section can be copied directly into Claude Code, Cursor, or other AI coding assistants to resolve the conflicts automatically. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7367-ci-add-AI-agent-prompt-to-backport-conflict-comments-2c66d73d365081e1a8fbcfdc48ea8777) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
Summary
Problem
Users could not copy error messages from the PromptExecutionError dialog or other modal dialogs. When pressing Ctrl+C with text selected in a dialog, the graph copy handler would intercept the event and prevent the default browser copy behavior.
Solution
Add a
hasTextSelection()check toshouldIgnoreCopyPaste(). When the user has any text selected in the document, the function returnstrue, allowing the default browser copy to proceed.Test plan
Screen.Recording.2025-12-04.at.22.13.43.mov
┆Issue is synchronized with this Notion page by Unito