-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add page reload when resuming Blazor circuit fails #64290
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?
Changes from 3 commits
fce8418
04ea8f0
a864856
9cdfa5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -15,7 +15,7 @@ function handleReconnectStateChanged(event) { | |||||
| reconnectModal.close(); | ||||||
| } else if (event.detail.state === "failed") { | ||||||
| document.addEventListener("visibilitychange", retryWhenDocumentBecomesVisible); | ||||||
| } else if (event.detail.state === "rejected") { | ||||||
| } else if (event.detail.state === "rejected" || (event.detail.state === "resume-failed" && !!event.detail.graceful)) { | ||||||
|
||||||
| } else if (event.detail.state === "rejected" || (event.detail.state === "resume-failed" && !!event.detail.graceful)) { | |
| } else if (event.detail.state === "rejected" || (event.detail.state === "resume-failed" && event.detail.remote)) |
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.
@javiercn your earlier suggestion of just
} else if (event.detail.state === "rejected" || event.detail.state === "resume-failed") {
provides a better empty new circuit experience.
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.
Hmm, not sure what you mean. We don't want to reload the page unconditionally when the circuit fails to resume. We only want to do that when it happens as a result of the app disconnecting, but not when the app explicitly pauses the circuit.
For ungraceful disconnections the client is not in control, but for pausing and then resuming, the client is. We don't want to unconditionally reload the page by default because that precludes users from saving any data that they had on the page.
Example, you are filling in a long form field, you switch to a different tab. Your JS code "pauses" the circuit when the visibility changes. When the user comes back to the tab and you try to "resume" the circuit, if for some reason the resume operation fails, we don't want to result in a refresh.
In the case of reconnection, the client app is not in control, but in case of reconnection, the client triggered the "pause" operation in the first place and should have some code to handle things when the "resume" goes wrong.
I would argue that automatically reloading is also a bad policy to have for reconnection, as you run into a similar problem, but I think that ship has sailed.
We want to restore the behavior for failed reconnections + failed resumes by default, but we don't want to automatically reload the page in case of failed resumes when the app triggered the pause.
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.
Remote and graceful represent the same concept. remote resumes are always ungraceful and vice versa.
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.
I see, I thought those were orthogonal properties. Then it simplifies the change. I will update the PR.