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

Added sample demonstrating how to connect session with Blazor and WebForms #542

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

pockets3407
Copy link
Contributor

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

PR Title
Updated sample demonstrating how to connect session with Blazor and WebForms using a Blazor Web App

PR Description
Updated sample demonstrating how to connect session with Blazor and WebForms using a Blazor Web App

Addresses #382

@pockets3407
Copy link
Contributor Author

@twsouthwick I redid the branch because I screwed up the rebase yesterday. It should be all set.

@pockets3407 pockets3407 changed the title manually added sample because I screwed up rebase Added sample demonstrating how to connect session with Blazor and WebForms Sep 7, 2024
Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

LGTM, except let's break it out into its own sample so the blazor components in webforms one still works

* B: Block all requests from paths containing "/_blazor"
* TLDR: We need to ensure that SystemWeb.Adapters is not used with Blazor SignalR
*/
app.UseWhen(
Copy link
Member

Choose a reason for hiding this comment

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

Is there a tracking issue for this? Not sure if this is the best way, but we should have an out of the box way to handle this I think.

Copy link
Contributor Author

@pockets3407 pockets3407 Sep 9, 2024

Choose a reason for hiding this comment

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

Not that I know of. I mentioned the deadlock in my previous PR and ended up with this workaround to get my project (and this sample) working.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good for the sample for now. could you create an issue to track this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

app.MapBlazorHub();
app.MapBlazorPages("/_Host");
app.MapForwarder("/{**catch-all}", app.Configuration["ProxyTo"]!).WithOrder(int.MaxValue);
app.MapRazorComponents<App>()
Copy link
Member

Choose a reason for hiding this comment

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

This breaks the existing sample that is to show using blazor components in webforms. It seems like this sample you have should be a separate one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The existing sample still works. See the images below. The blazor web page is now the index and I have updated the nav bar to reference web form pages.

image
 
image

I can make a separate sample if needed. I was under the impression from my previous PR that I should combine them.

Copy link
Member

Choose a reason for hiding this comment

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

awesome! I did not see that initially. it's fine as-is

Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for rebasing and the work here.

@twsouthwick twsouthwick merged commit e8cd20e into dotnet:main Sep 9, 2024
5 checks passed
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.

2 participants