fix: reset touch state when iPad Safari loses focus #7134
Conversation
dosubot
bot
added
the
size:S
📝 WalkthroughWalkthroughAdds a touch-state reset helper and subscribes to document Changes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (11)**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,vue}📄 CodeRabbit inference engine (.cursorrules)
Files:
src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-11-24T19:47:34.324ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
🧬 Code graph analysis (1)src/extensions/core/simpleTouchSupport.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (3)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/04/2025, 01:24:56 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/04/2025, 01:33:14 AM 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) • 🔴 +444 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 972 kB (baseline 972 kB) • ⚪ 0 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 — 169 kB (baseline 169 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/extensions/core/simpleTouchSupport.ts (1)
85-107: Good fix; consider centralizing touch reset logic and a small robustness tweakThe new
visibilitychangeandtouchcancelhandlers correctly address the stucktouchCount/touchZoomingproblem and match the PR goal. To keep this maintainable and avoid the two blocks drifting if touch state evolves, you can DRY the reset into a local helper:- // Reset touch state when page loses visibility (e.g., switching apps on iPad) - // This prevents touchCount from getting stuck when touchend events are missed - document.addEventListener('visibilitychange', () => { - if (document.hidden) { - touchCount = 0 - touchZooming = false - touchTime = null - lastTouch = null - lastScale = null - touchDist = null - } - }) - - // Also handle touchcancel which fires when touch is interrupted - app.canvasEl.parentElement?.addEventListener('touchcancel', () => { - touchCount = 0 - touchZooming = false - touchTime = null - lastTouch = null - lastScale = null - touchDist = null - }) + const resetTouchState = () => { + touchCount = 0 + touchZooming = false + touchTime = null + lastTouch = null + lastScale = null + touchDist = null + } + + // Reset touch state when page loses visibility (e.g., switching apps on iPad) + // This prevents touchCount from getting stuck when touchend events are missed + document.addEventListener('visibilitychange', () => { + if (document.hidden) { + resetTouchState() + } + }) + + // Also handle touchcancel which fires when touch is interrupted + app.canvasEl.parentElement?.addEventListener('touchcancel', resetTouchState)If you want an extra bit of defensive robustness (in case any browser ever delivers a stray
touchendafter a manual reset), you could also clamptouchCountin thetouchendhandler:// Line 58 replacement touchCount = Math.max(0, touchCount - e.changedTouches.length)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.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/extensions/core/simpleTouchSupport.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/simpleTouchSupport.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
| touchCount = 0 | ||
| touchZooming = false | ||
| touchTime = null | ||
| lastTouch = null | ||
| lastScale = null | ||
| touchDist = null |
There was a problem hiding this comment.
Since we require the same cleanup, can we extract a method to make sure they stay in sync?
Also, not for this PR, but we might want to find a way to make sure we're cleaning up all the listeners we're adding. It's been a source of memory leaks.
Summary
When the browser loses focus (e.g., switching apps, showing the dock), touchend events may not fire, causing touchCount to remain non-zero. This blocks all subsequent single-finger interactions since processMouseDown returns early when touchCount > 0.
Added visibilitychange and touchcancel event listeners to reset touch state when the page becomes hidden or touch is interrupted by the system.
fix #6721
┆Issue is synchronized with this Notion page by Unito