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

New .NET MAUI BWA with shared UI article #32390

Merged
merged 12 commits into from May 2, 2024

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Apr 24, 2024

Fixes #29880

Notes

  • Sample app - It's over here ..............

    https://github.com/dotnet/blazor-samples/tree/main/8.0/MauiBlazorWeb

    A README that cross-links the live article will be added later after the article goes live.

    NOTE: When I fixed the folder naming of the sample app on the sample repo and pulled the new app down for a couple of test runs, the first time I built the solution it broke. There were some cryptic MAUI errors, and it reported that it couldn't find the RCL's assembly. When I then re-built the solution, it built fine ... and runs fine. I hope devs obtaining the app don't hit that problem. If that's going to be a regular thing, then the article should call it out.

  • Check on the metadata basics

    • Title OK?
    • Description OK?
    • UID OK?
    • Filename (URL) OK?
  • Sample app versus steps - It seems to me that we shouldn't give step-by-step instructions for creating the sample app if we're providing the sample app via the samples repo and cross-linking it. Devs can just download that. What I think we should have are migration guidance for converting a MAUI app into a MAUI+BWA+RCL/shared UI solution. That's the way the PR does it, so see if that makes sense. I can always change it if this doesn't seem like the right approach.

  • Controlling render modes - Now here, I'm a bit confused by your discussion with Eilon and Steve via ...

    BlazorWebView needs a way to enable overriding ResolveComponentForRenderModeย aspnetcore#51235 (comment)

    ... and ...

    https://github.com/BethMassi/HybridSharedUI/pull/1/files#diff-5aaf282c84c804227c5da8e4bcaa06b881ba7634013aa0db927ae16a70b5cd97R6

    Is that right? ๐Ÿ˜• ... because I think there's a simpler way to go unless I'm missing something (and I probably am! ๐Ÿ™ˆ). See the article in the DIFF here (the Use Blazor render modes section), which is a little bit different than the result of that convo and the commit that was linked to. What's on the PR seems to work and seems to be slightly simpler. However, I'm notorious for my ๐Ÿ™ˆ RexHaqsโ„ข ๐Ÿคฎ๐Ÿ˜†, so I won't be surprised to hear that I missed a concept that requires the approach taken by the convo/commit of you/Eilon/Steve โ˜๏ธ.

  • Still in draft status ... I need to walk all of the steps again in the migration section to make sure that they're exactly right. I also need to edit the text again. However, please feel free to provide a full review now if you want. I just want to hit this whole thing one more time on Thursday morning with fresh ๐Ÿ‘๏ธ๐Ÿ‘๏ธ.


Internal previews

๐Ÿ“„ File ๐Ÿ”— Preview link
aspnetcore/blazor/fundamentals/index.md ASP.NET Core Blazor fundamentals
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app

@guardrex guardrex self-assigned this Apr 24, 2024
@guardrex guardrex requested a review from BethMassi April 24, 2024 12:31
@BethMassi
Copy link
Contributor

NOTE: When I fixed the folder naming of the sample app on the sample repo and pulled the new app down for a couple of test runs, the first time I built the solution it broke. There were some cryptic MAUI errors, and it reported that it couldn't find the RCL's assembly. When I then re-built the solution, it built fine ... and runs fine. I hope devs obtaining the app don't hit that problem. If that's going to be a regular thing, then the article should call it out.

The sample built and ran fine for me so I think it's OK.

  • Check on the metadata basics

    • Title OK?
    • Description OK?
    • UID OK?
    • Filename (URL) OK?

These LGTM.

  • Sample app versus steps - It seems to me that we shouldn't give step-by-step instructions for creating the sample app if we're providing the sample app via the samples repo and cross-linking it. Devs can just download that. What I think we should have are migration guidance for converting a MAUI app into a MAUI+BWA+RCL/shared UI solution. That's the way the PR does it, so see if that makes sense. I can always change it if this doesn't seem like the right approach.

Sounds good.

The readme and sample are vetted and correct. We did this offline so the conversation is outdated. For .NET 9, we'll fix the framework so render modes are always ignored in MAUI BlazorWebView.

@BethMassi
Copy link
Contributor

Nice work @guardrex it's starting to really come together! Can we also make sure we link to this sample article from this conceptual article? https://learn.microsoft.com/aspnet/core/blazor/hybrid/reuse-razor-components

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 26, 2024

@BethMassi ... And now for something completely different. - Monty Python's Flying Circus ยฉ1969-1974 BBC

Thanks for the way that you laid that out. There's a very different presentation introduced this morning that breaks down along those lines.

Notes

  • I added the cross-link that you mentioned.
  • We probably can't use pivots or tabs for the specifications of these solutions because the tab/pivot names are awfully long. Therefore, I've used sections.
  • There's a little boilerplate for the interactive render settings class, but splitting that out into a separate section is worse than having the duplication IMO. Readers generally don't like to be thrown around if we can avoid it, and the length of the boilerplate is reasonable.
  • I'd be surprised if I have everything just right. I probably introduced some ๐Ÿ˜ˆ๐Ÿ˜ˆ๐Ÿ˜ˆ that you'll probably spot.
  • Based on what you provided, it now looks like the sample is incorrectly configured. I think we should probably go with InteractiveServer per-component render modes, which means that I'll move the render modes from the App component and add the render mode to the Counter component of the RCL. I haven't done that yet ... wanted to check with you on it and on the whole approach on content layout here first.

Copy link
Contributor

@BethMassi BethMassi left a comment

Choose a reason for hiding this comment

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

I like this structure a lot better as it runs though all the options more completely. Thank you! Left some comments and corrections.

aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
@guardrex
Copy link
Collaborator Author

guardrex commented Apr 27, 2024

@BethMassi ... Updates applied, but I may have broken something in the process. The added content might need additional information as well.

UPDATE: I'm back to the grindstone on Monday morning โ›๏ธ. I think unless I broke something that it's looking good ... complicated, yes ... a bit hard to explain, yes ... but good.

This is going to improve nicely for .NET 9. Unfortunately, it might introduce quite a bit of horrible versioning to maintain going forward. Do you want to keep the guidance for 8.0, or rewrite for .NET 9 and move the article version to .NET 9 or later? One idea for .NET 9 or later is to PDF the article for .NET 8 and keep that available, including leaving the 8.0 sample app on the sample repo, but drop the .NET 8 coverage in favor of a rewrite for .NET 9 or later.

Copy link
Contributor

@BethMassi BethMassi left a comment

Choose a reason for hiding this comment

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

Looking good! Added a few corrections.

aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/tutorials/maui-blazor-web-app.md Outdated Show resolved Hide resolved
@guardrex
Copy link
Collaborator Author

guardrex commented May 1, 2024

@BethMassi ... There were still inconsistencies in naming for the project references ๐Ÿ˜ฉ, so I now try using the project names. See if that works.

Copy link
Contributor

@BethMassi BethMassi left a comment

Choose a reason for hiding this comment

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

So close! I forgot one last step for the most complicated case...

@guardrex
Copy link
Collaborator Author

guardrex commented May 2, 2024

@BethMassi ... See if the last TWO THREE ๐Ÿ˜„ commits covers it.

I had "csharp" instead of "razor" as the code language, so it took an extra commit ๐Ÿ˜ˆ.

... and I missed a space ... another rotten gremlin ๐Ÿ˜ˆ๐Ÿ”ซ๐Ÿ’€ ... requiring a third commit.

Let me know if we're holding for anyone else to look (e.g., Dan, Artak). Otherwise, I'll merge and take this live when you sign off.

Copy link
Contributor

@BethMassi BethMassi left a comment

Choose a reason for hiding this comment

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

Ship it!

@guardrex guardrex merged commit bd1aa07 into main May 2, 2024
3 checks passed
@guardrex guardrex deleted the guardrex/blazor-maui-hybrid-bwa-shared-ui branch May 2, 2024 22:15
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.

Interfaces/proxies for web+native device scenarios sample
2 participants