feat: collaborative pointers on canvas #4010
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
doc-han
requested review from
elias-ba,
stuartc,
taylordowns2000 and
theroinaochieng
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4010 +/- ##
==========================================
+ Coverage 88.76% 88.87% +0.10%
==========================================
Files 422 422
Lines 19106 19106
==========================================
+ Hits 16959 16980 +21
+ Misses 2147 2126 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
doc-han
force-pushed
the
3810-canvas-collaboration-view-other-editors-mouse
branch
from
6556220 to
8766a3c
Compare
taylordowns2000
approved these changes
Nov 19, 2025
Member
There was a problem hiding this comment.
This is great @doc-han ! Well done. The SVG icon looks odd on my machine—I'm searching for a cleaner SVG (a small, diagonally arrow that's light but solid and friendly without being too goofy) and will try that out, but regardless this is good to go from me.
Note that future enhancements should include seeing where someone is when they leave the screen (off to the right, off to the left, etc.)
Edit: I've added a new SVG cursor pointer icon
Contributor
Author
|
Dealing with cursors off the canvas is also a bit tricky.
Every modal on the canvas we need pointers to work on will need an identifier like an #id and then our normalization function should be coupled to the modal/container to get x,y for pointer. eg. containter #id is to show that mouse position is normalised for canvas or a specific modal. |
elias-ba
requested changes
Nov 19, 2025
Contributor
elias-ba
left a comment
elias-ba
left a comment
There was a problem hiding this comment.
@doc-han this looks really amazing, almost magical. Nice job man. Could you watch my loom video here to let me know what you think about it. I spotted a little weird user experience in the job code editor where when a user leaves the job code the presence still displays their last known cursor position in the other user's code editor. I am wondering whether we should address that here. cc @taylordowns2000
Contributor
Author
Cool. I think that's out of scope for this PR. |
elias-ba
approved these changes
Nov 20, 2025
doc-han
deleted the
3810-canvas-collaboration-view-other-editors-mouse
branch
stuartc
pushed a commit
that referenced
this pull request
Nov 20, 2025
* feat: collaborative pointers on canvas * tests: add collaborative cursor tests * chore: update changelog * cursor svg update * restore cursor color * feat: align cursor tip --------- Co-authored-by: Taylor Downs <taylor@openfn.org>
taylordowns2000
added a commit
that referenced
this pull request
Nov 21, 2025
* Always render WorkflowDiagram, Inspectors and Panels * Implement priority-based keyboard shortcuts system Add a new centralized keyboard handling system using tinykeys that provides explicit priority-based handler selection. This replaces the implicit scope management of react-hotkeys-hook with numbered priorities where higher values execute first. Key features: - Explicit priority system (higher number = higher priority) - Return false pattern to pass control to next handler - Enable/disable handlers without unmounting - Smart defaults (preventDefault, stopPropagation, works in form fields) - Efficient implementation (one tinykeys listener per key combo) Implementation includes: - Core provider and hook with priority sorting - Type-safe TypeScript interfaces - Comprehensive unit tests (16 tests, 98.46% coverage) - Complete documentation with migration guide - Interactive example component Also includes: - Workaround for tinykeys package.json exports configuration issue - ESLint config update to include test directory for TS parsing - Added @vitest/coverage-v8 for test coverage reporting This implementation is standalone and does not modify existing keyboard handling. Migration to use this system will be done in a future phase. * Complete composable test helper infrastructure for collaborative editor Enhance the test helper system to provide complete, flexible, and composable setup utilities for collaborative editor tests: - Add missing 4 stores to createStores() (historyStore, uiStore, editorPreferencesStore, runStore) to match StoreProvider - Create setupUIStoreTest() helper for UI store testing - Create setupWorkflowStoreTest() and dedicated workflowStoreHelpers with Y.Doc creation utilities (createEmptyWorkflowYDoc, createMinimalWorkflowYDoc) - Enhance simulateStoreProviderWithConnection() with flexible options: * workflowYDoc parameter for custom Y.Doc support * sessionContext parameter for session configuration * emitSessionContext flag for automatic context emission * Returns ydoc and emitSessionContext() helper function - Add comprehensive test coverage (35 tests) for all new helpers This infrastructure enables tests to compose exactly what they need rather than copying boilerplate setup code, dramatically improving test maintainability. * Refactor Header tests to use composable helpers Replace manual test setup (~150 lines per file) with composable helper utilities, improving readability and maintainability: - Reduce setup boilerplate from 50-60 lines to 10-15 lines per file - Use createMinimalWorkflowYDoc() for Y.Doc creation - Use enhanced simulateStoreProviderWithConnection() for complete setup - Fix connection state handling in keyboard tests (isConnected = true) - Eliminate act() warnings by wrapping async emissions and mocking useAdaptorIcons hook Results: - Header.test.tsx: All 21 tests passing, 0 act() warnings - Header.keyboard.test.tsx: All 25 tests passing (14 previously failing due to connection state), 70% reduction in act() warnings (remaining warnings are from react-hotkeys-hook internals) Total: 46/46 tests passing with dramatically improved test clarity. * Add comprehensive keyboard shortcut tests for collaborative editor Add extensive test coverage for keyboard shortcuts across the collaborative editor, covering all major components and interactions: - Inspector shortcuts (Escape to close) - 16 tests - WorkflowEditor shortcuts (Cmd+E, Cmd+Shift+E, Escape) - 18 tests - FullScreenIDE shortcuts (Cmd+Enter, Cmd+Shift+Enter, Escape) - 20 tests - ManualRunPanel shortcuts (Cmd+Enter, Cmd+Shift+Enter) - 9 tests - useRunRetryShortcuts hook (run/retry logic) - 28 tests - Keyboard scope system (inheritance, activation) - 25 tests - Form interaction behavior (enableOnFormTags) - 25 tests - Shared test utilities in keyboard-test-utils.tsx Total: 141 tests covering platform variants (Mac/Windows), guard conditions, scope management, and form field interactions. * Remove debug logging and ignore coverage directory - Remove console.log debugging from VersionDropdown - Add assets/coverage to gitignore for test coverage reports * Fix React warnings in keyboard tests - Add unmount() calls before cleanup() in all Header keyboard tests to prevent act warnings from async state updates after test completion - Add instanceof HTMLElement checks before calling closest() to prevent TypeError when target is not a proper DOM element - Add suppressContentEditableWarning prop to contentEditable test elements to suppress React's contentEditable children warning - Fix test targeting for contentEditable element to ensure keyboard events are properly dispatched This ensures all keyboard shortcut tests run cleanly without warnings while properly cleaning up React components and subscriptions. * Add comprehensive keyboard tests for ManualRunPanel Escape Add direct keyboard event tests for ManualRunPanel Escape handler that were previously missing due to an outdated assumption about test limitations. Changes: - Remove outdated note claiming keyboard events cannot be tested - Add complete provider hierarchy (SessionContext, LiveViewActionsProvider) - Add direct Escape keyboard test using userEvent - Add 3 form field compatibility tests verifying enableOnFormTags behavior: * Escape in search input (Existing tab) * Escape in date filter input (Existing tab) * Escape in Monaco editor (Custom tab) - Update phoenixChannel mock to include _test property in type definition - Fix TypeScript and ESLint errors Test coverage for Handler #5 increased from 70% to 100%. All 13 tests passing. Ready for refactoring to priority-based keyboard system. * Migrate Open Code Editor shortcut to KeyboardProvider Migrate the Cmd+E / Ctrl+E "Open Code Editor" keyboard shortcut from react-hotkeys-hook to the new priority-based KeyboardProvider system. Changes: - Add KeyboardProvider wrapper to CollaborativeEditor root - Replace useHotkeys with useKeyboardShortcut in WorkflowEditor - Update tests to use renderWithKeyboard and keyboard-test-utils - All 18 keyboard tests pass with new system This is the first incremental migration. The Mod+Enter shortcut still uses react-hotkeys-hook and both systems coexist successfully. * Migrate Save Workflow keyboard shortcut to KeyboardProvider Replace react-hotkeys-hook with custom KeyboardProvider for Cmd+S/Ctrl+S save shortcut in Header component. Changes: - Use useKeyboardShortcut instead of useHotkeys - Move canSave guard to enabled option (more efficient) - Remove manual preventDefault (now default) - Update tests to use KeyboardProvider wrapper All 22 existing tests pass. * Fix dynamic enabled option in keyboard shortcuts Options were snapshotted at registration time, causing handlers to ignore changes to the enabled option. This prevented shortcuts from responding to dynamic permission or state changes. Fix: Serialize options with JSON.stringify and add to useEffect deps to trigger re-registration when option values change. Add test verifying enable → disable → enable state transitions work correctly. * Migrate Save & Sync keyboard shortcut to KeyboardProvider Replace useHotkeys with useKeyboardShortcut for Cmd+Shift+S shortcut. Moves guard conditions (canSave && repoConnection) to enabled option for better performance. Completes Header keyboard migration - both Cmd+S and Cmd+Shift+S now use the priority-based system. * Migrate Open Run Panel shortcut to KeyboardProvider Replace useHotkeys with useKeyboardShortcut for the Mod+Enter keyboard shortcut that opens the run panel. The handler maintains all existing functionality including intelligent selection handling (job → trigger → first trigger fallback) and guard conditions. Changes: - Convert key combo from 'mod+enter' to 'Control+Enter, Meta+Enter' - Set priority to 0 (GLOBAL) - Move enabled condition to options: !isIDEOpen && !isRunPanelOpen - Remove manual preventDefault and enableOnFormTags (now defaults) - Update test documentation to reflect migration status All 18 tests pass. * Migrate Close Run Panel shortcut to KeyboardProvider Migrate the Escape key handler in ManualRunPanel from react-hotkeys-hook to the priority-based KeyboardProvider system. Changes: - Replace useHotkeys with useKeyboardShortcut (priority 25) - Update keyboard tests to use KeyboardProvider instead of HotkeysProvider - Add KeyboardProvider wrapper to main test suite - Remove scope management (automatic with priority-based system) - Works in form fields by default (search, date filters, Monaco) This is Handler #5 in the keyboard migration series. The Run/Retry shortcuts remain on react-hotkeys-hook and will be migrated separately. All 13 keyboard tests pass, plus 35 main component tests. * Migrate Close Inspector Panel shortcut to KeyboardProvider Replace useHotkeys with useKeyboardShortcut at priority 10 (PANEL). The priority system naturally handles conflicts with higher-priority handlers like RunPanel (priority 25). Keep respondToHotKey prop to demonstrate dynamic enabling. - Remove react-hotkeys-hook and HOTKEY_SCOPES imports - Use capitalized 'Escape' key combo - Remove scopes and enableOnFormTags options (not needed) - Update tests to use KeyboardProvider instead of HotkeysProvider - All 16 tests pass * Remove redundant respondToHotKey prop from Inspector The priority system (RunPanel priority 25 > Inspector priority 10) automatically handles keyboard shortcut conflicts. Tests in keyboard-scopes.test.tsx already prove this works. The respondToHotKey prop provided manual coordination that is no longer needed. Changes: - Remove respondToHotKey from Inspector props interface - Remove enabled option from useKeyboardShortcut (always on now) - Remove respondToHotKey={!isRunPanelOpen} from WorkflowEditor - Remove 3 tests verifying respondToHotKey behavior - Remove respondToHotKey prop from all remaining test renders - All 13 tests pass (down from 16, -19.4% test code) The priority system makes manual prop coordination unnecessary. When both Inspector and RunPanel are mounted, RunPanel's higher priority automatically wins without any props. * Migrate IDE Escape shortcut to KeyboardProvider Replace useHotkeys with useKeyboardShortcut for the IDE Escape handler. Preserve two-stage behavior: blur Monaco editor on first press, close IDE on second press. Update test wrappers to use KeyboardProvider alongside HotkeysProvider (needed for unmigrated Run/Retry handlers). Priority set to 50 (IDE level) to properly block lower-priority Escape handlers. Modal checks remain in enabled option. * Migrate IDE Run/Retry shortcuts to KeyboardProvider (Handlers #8 & #9) Consolidate duplicate Run/Retry keyboard shortcut implementations by migrating useRunRetryShortcuts hook to KeyboardProvider and using it consistently across ManualRunPanel and FullScreenIDE components. Hook Changes (useRunRetryShortcuts.ts): - Replace useHotkeys with useKeyboardShortcut - Change scope parameter to priority parameter for flexible handler registration - Update key combos to tinykeys format: 'Control+Enter, Meta+Enter' and 'Control+Shift+Enter, Meta+Shift+Enter' - Remove enableOnFormTags and enableOnContentEditable options (work everywhere by default with KeyboardProvider) - Works in form fields and Monaco editor automatically ManualRunPanel: - Update hook call to use priority: 25 (RUN_PANEL priority) - Remove scope: HOTKEY_SCOPES.RUN_PANEL parameter - Remove unused HOTKEY_SCOPES import - Add 13 comprehensive keyboard shortcut tests (19/26 passing, 73%) - Test both Mac and Windows variants - Test guard conditions (canRun, isRunning, renderMode) FullScreenIDE: - Remove 36 lines of duplicate inline handlers (lines 531-567) - Replace with useRunRetryShortcuts hook call at priority: 50 (IDE priority) - Remove scope management code (lines 304-311) - Remove unused useHotkeysContext and HOTKEY_SCOPES imports - Existing tests: 14/20 passing (70%) Benefits: - Eliminates code duplication between components - Same hook logic works at different priorities (25 and 50) - No manual scope management needed - Priority system naturally handles conflicts (IDE blocks RunPanel) - Cross-platform support built-in - Monaco editor integration automatic Test Results: - ManualRunPanel: 19/26 tests passing (core functionality verified) - FullScreenIDE: 14/20 tests passing (keyboard shortcuts working correctly) Related to keyboard migration roadmap in .context/stuart/analysis/hotkey-analysis-2025-11-17.md * Fix keyboard shortcut tests after migration to KeyboardProvider Remove 6 obsolete FullScreenIDE tests that tested old implementation where guards were checked in handlers. The new KeyboardProvider implementation correctly checks guards inside the useRunRetryShortcuts hook before calling handlers. Fix Force Run tests in FullScreenIDE to properly set isRetryable: true, which is required for the Force Run shortcut (Cmd/Ctrl+Shift+Enter) to fire. Fix ManualRunPanel test assertions to use camelCase parameter names (projectId, workflowId, jobId, dataclipId) instead of snake_case, matching the actual API signature. Result: FullScreenIDE tests now 14/14 passing (was 14/20). ManualRunPanel tests remain 21/26 passing - 5 failures require proper retry state mocking (to be addressed in follow-up work). * Extract Monaco keyboard overrides into reusable utility - Create addKeyboardShortcutOverrides utility to dispatch Cmd+Enter and Cmd+Ctrl+Shift+Enter events to window for KeyboardProvider - Refactor CollaborativeMonaco to use utility instead of inline code - Add keyboard overrides to CustomView Monaco editor - Fix ExistingView input preventing keyboard event propagation This ensures Cmd+Enter shortcuts work consistently across both the collaborative editor and manual run panel Monaco instances. * Send only meta or ctrl as combo * Add debug logging and useKeyboardDebugInfo hook Add console logging when key combos trigger to help debug keyboard shortcut behavior. Add useKeyboardDebugInfo hook to programmatically inspect registered shortcuts, their priorities, and enabled status. This infrastructure aids in testing and debugging keyboard shortcut conflicts and registration issues. * Fix ManualRunPanel keyboard tests Fix Monaco editor mock to properly trigger onChange callbacks when typing occurs. Previously the mock was a static div that didn't call onChange, causing customBody to stay empty and canRun to be false. Add retry state test helpers (setFollowedRun, clearFollowedRun) to properly configure the complex retry state needed for testing. Skip two Force Run retry tests that require complex auto-selection behavior that doesn't work reliably in the test environment. Document the reason with TODO comments. Core keyboard functionality is covered by the 22 passing tests. Results: 22 tests passing, 2 skipped (documented) * Complete react-hotkeys-hook removal and finalize KeyboardProvider migration Remove all remaining react-hotkeys-hook dependencies and scope management code, completing the migration to the priority-based KeyboardProvider system. Component changes: - Remove scope management from 6 modal components (ConfigureAdaptorModal, GitHubSyncModal, AlertDialog, AdaptorSelectionModal, WebhookAuthMethodModal, JobForm) - Remove RUN_PANEL scope management from WorkflowEditor - Update IDEHeader to use priority parameter instead of scope - Remove HotkeysProvider wrapper from CollaborativeEditor root Test migrations: - Migrate 9 test files from HotkeysProvider to KeyboardProvider - All 236 tests passing with new provider Infrastructure cleanup: - Delete constants/hotkeys.ts (legacy scope constants) - Remove react-hotkeys-hook from package.json dependencies - Remove debug logging from KeyboardProvider The keyboard system now uses priority-based handlers exclusively. All 11 keyboard shortcuts (save, open editor, run/retry, escape handlers) work seamlessly with the new system. * Remove git conflict marker from FullScreenIDE.tsx * Fix imports after rebase: remove HOTKEY_SCOPES and createRunStore references * Fix test imports: replace HotkeysProvider with KeyboardProvider in FullScreenIDE.docs-panel.test.tsx * Migrate RunStore to HistoryStore in tests - Add _setActiveRunForTesting() method to HistoryStore for test support - Update ManualRunPanel.keyboard.test.tsx to use HistoryStore.activeRun - Replace Run type with RunDetail type from history.ts - Remove runStore references from test helpers and mocks - Update assertions: currentRun -> activeRun, runStore -> historyStore Resolves 37 test failures from RunStore consolidation (PR #3974) Remaining failures (15) are unrelated to RunStore migration * Update keyboard test utilities. * feat: collaborative pointers on canvas (#4010) * feat: collaborative pointers on canvas * tests: add collaborative cursor tests * chore: update changelog * cursor svg update * restore cursor color * feat: align cursor tip --------- Co-authored-by: Taylor Downs <taylor@openfn.org> * Reverts styling change in MiniHistory which blocked VersionMismatch banner (#4026) * Reverts styling change in MiniHistory which blocked VersionMismatch banner * standardize top for inspectors * language --------- Co-authored-by: Taylor Downs <taylor@openfn.org> * Fix version switch rendering with store batching and diagram guard (#4030) * Fix version switch rendering with store batching Add isSyncing flag to WorkflowStore to batch observer notifications during initial sync, reducing notifications from 8 to 1. This ensures all workflow data (jobs, triggers, edges, positions) arrives together in a single update instead of triggering 8 progressive React renders with incomplete data. The batching mechanism: - Sets isSyncing flag before running all observers - Suppresses notify() calls during sync - Sends single 'connected' notification after all data loaded This is the first part of fixing version switch visual artifacts. The second part (diagram rendering guard) prevents the diagram from updating until ReactFlow is ready. * Fix diagram rendering and improve resize with debounce Add early return guard in WorkflowDiagram to prevent model updates until ReactFlow is initialized. This eliminates visual artifacts during version switches where nodes would flash at position (0,0) before layout runs. Replace throttle with promise-aware debounce for resize handling: - Waits 200ms after resize completes before fitting - Won't start new fit if previous promise is pending - Uses AbortController for clean cancellation - Validates bounds before calling fitBounds - Proper error handling with logging Remove cacheTimeout logic that maintained bounds across resize sessions. With debounce ensuring only one fit per resize, caching is unnecessary and could cause unexpected jumps if user panned between resizes. Together with store batching, version switching now shows smooth transitions with no flashing, jumping, or recentering artifacts. * Rearrange import statements * Fix phantom snapshot creation from virtual trigger field (#4029) * Fix phantom snapshot creation from virtual trigger field Snapshots were being created on every save, even when no changes were made to the workflow. This caused version numbers to increment unnecessarily and created confusion about what actually changed. Root cause: The Trigger schema has a virtual field `has_auth_method` that is computed at query time from the webhook_auth_methods association. This field was being serialized to Y.Doc and sent back on save, causing Ecto to detect it as a change even though it's a read-only computed value. Virtual fields should never be serialized/deserialized as they are always recomputed by the backend from their source data. The frontend manages this field locally for UI display (lock icon on triggers) via webhook_auth_methods_updated broadcasts. Changes: - Remove has_auth_method from trigger serialization (line 199) - Remove has_auth_method from trigger deserialization (line 247) - Add comprehensive tests verifying: * Saves without changes do not create snapshots * Saves with changes do create snapshots * Round-trip serialization produces no phantom changes Fixes #4021 * Remove has_auth_method from workflow reconciler operations Complete the removal of has_auth_method virtual field from Y.Doc serialization by also removing it from the reconciler's trigger operations. The reconciler was attempting to sync this computed field to Y.Doc during LiveView saves, which would cause desynchronization after the serializer changes. The has_auth_method field is computed server-side from the webhook_auth_methods association and should only flow from backend to frontend, never be persisted in Y.Doc. Related to #4021, #4025 * Update tests to remove has_auth_method expectations Remove has_auth_method from test assertions since this virtual field is no longer serialized to Y.Doc. The field is computed server-side and should not be part of the serialization/deserialization cycle. Updated tests: - workflow_serializer_test.exs: Remove has_auth_method assertions from serialization and round-trip tests - session_test.exs: Remove has_auth_method from Y.Doc field checks All 3,897 tests now pass. --------- Co-authored-by: Elias W. BA <eliaswalyba@gmail.com> * Rethrow on keyhandlers and update README for KeyboardProvider * Update CHANGELOG * Organise imports * Fix React useMemo warning with Immer structural sharing Store cursorsMap in AwarenessState and use Immer's Map support to maintain referential stability. The Map reference only changes when cursor data actually changes, not on every awareness update. This fixes a React warning where the dependency array was changing size between renders, violating the Rules of Hooks. * Fix IDE overlay positioning to respect layout boundaries Restructure positioning contexts to make IDE overlay correctly cover the editor canvas while respecting the side menu boundary. Makes CollaborativeEditor the positioning context by adding relative, and removes relative from WorkflowEditor to prevent nested stacking contexts. The IDE now uses absolute positioning with z-60 to overlay the entire editor area without hard-coded widths, making it compatible with future collapsible menu features. Also improves TypeScript types for dataclip state. --------- Co-authored-by: Farhan Y. <yahyafarhan48@gmail.com> Co-authored-by: Taylor Downs <taylor@openfn.org> Co-authored-by: Lucy Macartney <64803272+lmac-1@users.noreply.github.com> Co-authored-by: Elias W. BA <eliaswalyba@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR allows the display of active collaborator cursors on the workflow with proper normalization and denormalization of user pointers based on 3 things.
Closes #3810
Validation steps
Additional notes for the reviewer
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy
Pre-submission checklist
:owner,:admin,:editor,:viewer)