Skip to content

Improves further on Blazor reconnection experience. #13152

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

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

NTaylorMullen
Copy link

  • Expanded ReconnectDisplay to have a refused method on it. This is the method that indicates we will never be able to reconnect to the server. By default we provide a nice little message letting users know that reconnection is no longer possible and that a refresh must take place.
  • Added a logger to the DefaultReconnectionDisplay since part of its job is handling Retry clicks which indirectly call reconnect(). Therefore, it needed the ability to log information to the console to inform users why certain reconnects were not possible.
  • Updated the UserSpecifiedDisplay to have a refused understanding. Added a new CSS class to represent the refused state as well.
  • Updated existing tests to abide by the new ReconnectDisplay structure
  • Added a new test to validate that the refused``ReconnectDisplay method results in proper behavior.

image

#12442

@@ -37,13 +37,8 @@ async function boot(userOptions?: Partial<BlazorOptions>): Promise<void> {
}

const reconnection = existingConnection || await initializeConnection(options, logger);
if (reconnection.state !== signalR.HubConnectionState.Connected) {
Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

So do we need to do something here to throw? Who's actually doing the throwing in this case?

Copy link
Author

Choose a reason for hiding this comment

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

SignalR's throwing, I considered throwing a custom exception but I felt the need to back this out after the throwing from SignalR seemed to absolutely be intentional.

Copy link
Author

@NTaylorMullen NTaylorMullen Aug 15, 2019

Choose a reason for hiding this comment

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

You sound extra surprised though. Would you rather me throw a custom exception?

@Eilon Eilon added the area-blazor Includes: Blazor, Razor Components label Aug 15, 2019
@NTaylorMullen NTaylorMullen added the tell-mode Indicates a PR which is being merged during tell-mode label Aug 15, 2019
- Expanded `ReconnectDisplay` to have a `rejected` method on it. This is the method that indicates we will never be able to reconnect to the server. By default we provide a nice little message letting users know that reconnection is no longer possible and that a refresh must take place.
- Added a logger to the `DefaultReconnectionDisplay` since part of its job is handling `Retry` clicks which indirectly call `reconnect()`. Therefore, it needed the ability to log information to the console to inform users why certain reconnects were not possible.
- Updated the `UserSpecifiedDisplay` to have a `refused` understanding. Added a new CSS class to represent the `refused` state as well.
- Updated existing tests to abide by the new `ReconnectDisplay` structure
- Added a new test to validate that the `refused``ReconnectDisplay` method results in proper behavior.

#12442
@NTaylorMullen NTaylorMullen force-pushed the nimullen/12442.reload branch from ea2a047 to 7648018 Compare August 15, 2019 17:32
@aspnet-hello
Copy link

This comment was made automatically. If there is a problem contact [email protected].

I've triaged the above build. I've created/commented on the following issue(s)
https://github.com/aspnet/AspNetCore-Internal/issues/2854

@NTaylorMullen
Copy link
Author

🆙 📅 'd to address the refused -> rejected rename. Let me see that ✔️!

@NTaylorMullen NTaylorMullen merged commit f890c91 into release/3.0 Aug 15, 2019
@ghost ghost deleted the nimullen/12442.reload branch August 15, 2019 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants