-
Notifications
You must be signed in to change notification settings - Fork 61
Allow users to view workflow editor when disconnected from server #4012
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4012 +/- ##
==========================================
- Coverage 88.84% 88.83% -0.02%
==========================================
Files 422 422
Lines 19118 19118
==========================================
- Hits 16985 16983 -2
- Misses 2133 2135 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Hey @midigofrank , quick Qs before a full review:
- Why must the workflow be read only? Isn't the idea of this yDoc/collaborative editor that I can make changes and have those changes pushed to the web once my connection is restored?
- I still get the loading screen with low/intermittent connectivity on the IDE. Can that be handled in the same way?
|
Hey @taylordowns2000 , Re 1: Yes it's possible, it just requires a little more work. The current setup overwrites the changes in the store with the server changes. Maybe this should be the next step Re 2: Great catch! Let me fix that |
@stuartc says that they should merge and not overwrite. Let me do a diagnosis on what it would take to make that possible |
bdcb847 to
dca56c4
Compare
When the WebSocket disconnects, users previously saw an infinite loading spinner. Now the workflow remains visible in read-only mode with cached data until the connection is restored. Changes: - LoadingBoundary: Allow rendering with cached workflow data when disconnected - useWorkflowReadOnly: Add connection state check to enable read-only mode - SessionProvider: Track reconnection state and reinitialize session properly - createSessionStore: Clean up existing provider before reinitializing - Add SessionContext to useWorkflowReadOnly tests for connection state The implementation preserves Y.Doc state across reconnections while ensuring SessionStore event handlers are properly attached to track connection changes. All edit controls automatically disable when disconnected via the existing read-only system.
Extract provider lifecycle and Y.Doc persistence logic into dedicated hooks to improve code organization and reusability. Add ConnectionStatusContext to centralize connection state management and make it accessible throughout the component tree. Changes: - Extract useProviderLifecycle hook to manage provider creation/reconnection - Add useYDocPersistence hook for Y.Doc lifecycle management - Create ConnectionStatusContext for centralized connection status - Update SessionProvider to use new hooks and wrap with ConnectionStatusProvider - Track sync state and last sync time for better connection status visibility - Fix test error messages to match actual guard function behavior - Resolve merge conflict in ReadOnlyWarning test This refactoring maintains the same functionality while improving code maintainability and making connection state more accessible to components.
552c9a6 to
834965d
Compare
|
Hey @taylordowns2000 would you mind testing this out again. @stuartc by the way, the |
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.
does what it says on the tin. please until @stuartc gives a 👍 to merge.
@midigofrank , have you already created an issue for merging? (my review question 1) if not, please do so
|
@taylordowns2000 the latest commit merges the changes |
stuartc
left a comment
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.
A couple of things, I've thrown over a bunch of notes. There is some weird behaviour when switching to latest from a version, more easily noticed when you have two browsers open. It's like you're both on latest and also not :/
Description
Fixes the "loading workflow screen of death" issue where users experienced infinite loading spinners when the WebSocket disconnected. This PR enables graceful degradation by preserving offline workflow state and allowing offline editing with automatic sync on reconnection.
Key Changes
1. Graceful Disconnection Handling (commit 219a065)
2. Offline Editing Support (commit d601b42)
3. Architecture Improvements (commit dca56c4)
useProviderLifecyclehook for provider creation/reconnection managementuseYDocPersistencehook for Y.Doc lifecycle management (created once, only destroyed on unmount)ConnectionStatusContextto expose connection state to componentsHow It Works
The implementation uses Y.js CRDTs (Conflict-free Replicated Data Types) which automatically handle offline editing:
Closes #3972
Validation steps
Test 1: Offline Editing and Sync
Ctrl+Cin your server terminaliex -S mix phx.serverTest 2: Concurrent Offline Editing (Conflict Resolution)
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)