Skip to content
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

fix(sanity): preserve form (as readOnly) when reconnecting #5884

Merged
merged 1 commit into from Mar 4, 2024

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Mar 1, 2024

Description

Currently, if the connection drops while you're editing a document, the whole pane reloads and the react tree inside it will unmount/remount, loosing all its state. Since we're introducing more transient state in these components (comments, etc), we're running a bigger risk of losing people's stuff when this happens. Short disconnects happens quite often, so it can be pretty disruptive to the end-user. This PR makes sure that instead of taking over the pane when the listener disconnects (technically, is in a "reconnecting" state), we retain the form component instance, but set it as readonly to prevent the editor for making edits without being connected.

What to review

  • I've changed a few places to use connectionState !== 'reconnecting' instead of connectionState === 'connected'.
  • Is there anything I'm missing here?

Note: this is not a "perfect solution", and I noticed during testing that any open comments dialog gets closed when the document flips to read-only, which is still not great. However, this PR should make things a little bit less disruptive.

Testing

Manual testing is a bit tricky, because you will have to force a listener disconnect. However, I added a few lines to simulate the behavior during disconnect

Repro current behavior during disconnect

Open this deployment, which simulates connection drop 5 seconds after you stop editing a document (try making some edits to e.g. this document and wait for 5 seconds)

Test new behavior during disconnect

Open this deployment, which simulates connection drop 5 seconds after you stop editing, but also includes the changes in this PR (try making some edits to the same document on this deployment and wait for 5 seconds)

Notes for release

  • Improved how the editor handles intermittent disconnects. Instead of replacing the current form with a loading screen, the form will now instead be set "read only"

Copy link

vercel bot commented Mar 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview Mar 1, 2024 5:04pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 1, 2024 5:04pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Mar 1, 2024 5:04pm

Copy link
Member Author

bjoerge commented Mar 1, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bjoerge and the rest of your teammates on Graphite Graphite

@bjoerge bjoerge force-pushed the fix/sdx-532-fix-connection-drop-reload branch from 1e6579e to 57a635e Compare March 1, 2024 16:59
Copy link
Contributor

github-actions bot commented Mar 1, 2024

No changes to documentation

Copy link
Contributor

github-actions bot commented Mar 1, 2024

Component Testing Report Updated Mar 1, 2024 5:06 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 31s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 12s 3 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 12s 4 2 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 12s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 31s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 0s 15 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 0s 18 0 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 11s 6 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 19s 9 0 0

@bjoerge bjoerge requested review from a team and sjelfull and removed request for a team March 1, 2024 17:32
@bjoerge bjoerge marked this pull request as ready for review March 1, 2024 17:32
@bjoerge bjoerge requested a review from a team as a code owner March 1, 2024 17:32
@bjoerge bjoerge requested review from ricokahler and robinpyon and removed request for a team March 1, 2024 17:32
Copy link
Member

@sjelfull sjelfull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@bjoerge bjoerge added this pull request to the merge queue Mar 4, 2024
Merged via the queue into next with commit ed87e2a Mar 4, 2024
70 of 71 checks passed
@bjoerge bjoerge deleted the fix/sdx-532-fix-connection-drop-reload branch March 4, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants