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

Update Blazor Hybrid template to match ASP.NET Core Blazor Web template #17946

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

Eilon
Copy link
Member

@Eilon Eilon commented Oct 11, 2023

Updated .NET MAUI Blazor Hybrid template to match latest ASP.NET Core Blazor Web template code from https://github.com/dotnet/aspnetcore/tree/release/8.0/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWeb-CSharp

  • Also updated MAUI BlazorWebView build targets to be aware of the Components project folder we now use. This should be an additive-only change, so it shouldn't realistically break any existing upgraded project. All it does is ensure that .razor files in that folder don't cause errors in certain iOS/MacCatalyst/Android build scenarios. If you didn't have such files before, still no problem. And if you did, you probably already had to do some workaround, and this shouldn't interfere with that.

Tested locally on Windows, Android, and MacCatalyst. My iOS Simulator is not working right now.

Differences/changes from the ASP.NET version to what's in this PR:

  1. All "sample content" is always included (there's no option for it)
  2. No typeof(Program) in MAUI so it uses typeof(MauiProgram) instead
  3. Removed anything identity/auth related
  4. There's no App.razor in MAUI so some bits of CSS/HTML were placed in different files (e.g. index.html and app.css)
  5. In MAUI there's already a type called Layout in a default imported namespace, so in Routes.razor the default layout is set to Components.Layout.MainLayout instead of just Layout.MainLayout
  6. Removed nullable annotations because the MAUI templates are not nullable-enabled (the weather stuff had a few ? in it originally)

Fixes #16797

Note for future humans or super-smart AI: This PR had some small bugs in it:

  1. The NavMenu name substitution was missing and the openInEditor action in the template.json file wasn't updated. Bug Default MAUI Blazor app always has title 'BlazorWeb-CSharp' and doesn't open default file #18382 was logged, and the fixes are in PR Fix project name substitution in Blazor template nav menu and "VS install" prompt #18388.
  2. Some files weren't in the right location, and the fix is in Move MAUI Blazor template files to match ASP.NET Blazor template #18581

@Eilon Eilon added area-blazor Blazor Hybrid / Desktop, BlazorWebView area-templates Project templates, Item Templates for Blazor and MAUI labels Oct 11, 2023
@Eilon Eilon requested a review from a team as a code owner October 11, 2023 00:15
@@ -1,7 +1,7 @@
{
"author": "Microsoft",
"name": ".NET MAUI Blazor App",
"description": "A project for creating a .NET MAUI application for iOS, Android, Mac Catalyst, WinUI, and Tizen using Blazor",
"name": ".NET MAUI Blazor Hybrid App",
Copy link
Member Author

Choose a reason for hiding this comment

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

This wasn't really part of this change, but the build auto-updates this file. Looks like it should have been already updated due to some earlier change, but somehow wasn't. Might as well get the change in here.

@Eilon Eilon requested a review from halter73 October 11, 2023 20:57
@@ -70,6 +70,7 @@
<!-- Create a list of all content that *is* in the wwwroot folder (to preserve ItemGroup metadata) -->
<_TemporaryHiddenContent Include="@(Content)" Exclude="@(_NonWWWRootContent)" />
<!-- Add Scoped CSS files in the app to the list of hidden items -->
<_TemporaryHiddenContent Include="Components\**\*.razor.css" />
Copy link
Member

Choose a reason for hiding this comment

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

Does mean you really can only have Blazor components with component specific styles (.razor.css) in these folders?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe so, at least, that's what it originally was for. But in the intervening years no one has ever complained about it even once...

Having said that, I'm not entirely sure it's necessary, but I feel it's quite safe to add (that is, it shouldn't cause any harm, and at most should help some scenarios).

Copy link
Member

Choose a reason for hiding this comment

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

Why does the same thing appear on lines 73 and 95? Is that an accident?

Copy link
Member Author

Choose a reason for hiding this comment

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

The top one is a target for not-hot-restart, the bottom one is hot-restart specifically. At one point we had only the not-hot-restart target and the fix was to duplicate everything into another target.

@@ -0,0 +1,6 @@
<Router AppAssembly="@typeof(MauiProgram).Assembly">
Copy link
Member

Choose a reason for hiding this comment

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

I think you still want the NotFound content here. It's not used in a Blazor Web App, but I think it can still show up in a Blazor Hybrid up if you mistype a link to a page.

Copy link
Member Author

Choose a reason for hiding this comment

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

With the changes in this PR, this is what you see when you navigate to a missing URL:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

I chatted with @danroth27 and we agreed it's OK to have this screen. Ultimately if anyone ever sees this then it's a bug in the app, so it doesn't matter much what it looks like. Unlike Blazor Web apps you don't get to type into an address bar to go wherever you want, so you should never see this.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed for now.

Note that this reasoning may cease to apply in the future if we implement dotnet/aspnetcore#45654, which would let people programmatically trigger the NotFound content, e.g., if you visited a link to /entries/{id} and then at runtime the DB/API backend says there is no such ID.

Copy link
Member

@danroth27 danroth27 left a comment

Choose a reason for hiding this comment

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

One small comment about the Routes component, but otherwise looks good to me.

@Eilon
Copy link
Member Author

Eilon commented Oct 12, 2023

@halter73 / @SteveSandersonMS could you give this a once-over so I can merge?

@samhouts samhouts added this to the .NET 8 GA milestone Oct 12, 2023
@mattleibow
Copy link
Member

For 6. Removed nullable annotations because the MAUI templates are not nullable-enabled (the weather stuff had a few ? in it originally)

Why?

Can/should we enable nullability? In our efforts to be modern and stay modern, maybe we should?

@Eilon
Copy link
Member Author

Eilon commented Oct 12, 2023

For 6. Removed nullable annotations because the MAUI templates are not nullable-enabled (the weather stuff had a few ? in it originally)

Why?

Can/should we enable nullability? In our efforts to be modern and stay modern, maybe we should?

From my investigation now, I think it's something we should strongly consider: #17987

But perhaps now is not the time, given the time we have left?

@Eilon Eilon merged commit e982732 into net8.0 Oct 13, 2023
@Eilon Eilon deleted the eilon/update-templates-aspnetcore-release80 branch October 13, 2023 16:40
@PureWeen
Copy link
Member

/backport to release/8.0.1xx-rc2.1

@github-actions
Copy link
Contributor

Started backporting to release/8.0.1xx-rc2.1: https://github.com/dotnet/maui/actions/runs/6561953145

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Blazor Hybrid / Desktop, BlazorWebView area-templates Project templates, Item Templates for Blazor and MAUI fixed-in-8.0.0-rc.2.9511 Look for this fix in 8.0.0-rc.2.9511
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants