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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blazor layout sections #28660

Merged
merged 10 commits into from Apr 26, 2023
Merged

Blazor layout sections #28660

merged 10 commits into from Apr 26, 2023

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Mar 13, 2023

Fixes #28617

Surayya ... Note that I don't get access to the bits until the preview releases, so I can't check/test anything locally at this time. I had to guess a little on a few of the finer points, but you'll likely spot anywhere I went wrong 馃檲 quickly.

UPDATE: I reacted to the latest design changes coming for Preview 3. Given that I did that here, it seems like we should hold this until the Preview 3 release. However, I think we can go ahead and get it reviewed and updated.


Internal previews

馃搫 File 馃敆 Preview link
aspnetcore/blazor/components/layouts.md ASP.NET Core Blazor layouts
aspnetcore/blazor/components/sections.md aspnetcore/blazor/components/sections

@guardrex guardrex marked this pull request as ready for review March 17, 2023 11:36
@guardrex
Copy link
Collaborator Author

@surayya-MS ... I'm holding off on publishing this until Preview 3 is released, but do you want to review it now?

@mkArtakMSFT
Copy link
Member

@surayya-MS can you please review this when you get time? Thanks!

aspnetcore/blazor/components/layouts.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/layouts.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/layouts.md Outdated Show resolved Hide resolved
@guardrex
Copy link
Collaborator Author

guardrex commented Apr 7, 2023

Surayya ... The revisions are in place, except for one aspect ...

explain that this could be useful for creating a library

Could you flesh that bit out further for the topic? What you do want to tell devs about the approach?

@guardrex guardrex requested a review from surayya-MS April 7, 2023 10:20
Copy link
Member

@surayya-MS surayya-MS left a comment

Choose a reason for hiding this comment

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

Thanks @guardrex !

aspnetcore/blazor/components/layouts.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/layouts.md Outdated Show resolved Hide resolved
aspnetcore/blazor/components/layouts.md Outdated Show resolved Hide resolved
@surayya-MS
Copy link
Member

Surayya ... The revisions are in place, except for one aspect ...

explain that this could be useful for creating a library

Could you flesh that bit out further for the topic? What you do want to tell devs about the approach?

Yes, I described here #28660 (comment)

@guardrex guardrex requested a review from danroth27 April 12, 2023 09:52
@danroth27
Copy link
Member

danroth27 commented Apr 17, 2023

@surayya-MS What happens if you define a SectionOutlet but it isn't filled by a SectionContent? What happens if you define SectionContent but there isn't a matching SectionOutlet?

Also, is there ever a case where you'd use a SectionOutlet outside of a layout component? For example, can you use SectionOutlet in a parent component and then filling it from a child component?

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 18, 2023

That works great, so shall we make this it's own topic under the Components node and call it "ASP.NET Core Blazor sections" and not refer to "layouts," just call this feature "sections" throughout?

I have a follow-up question on casing: I know the dev can do whatever they want on casing, but what should we show for the argument format? Usually, we go with camel (topBar) or Pascal (TopBar).

btw -- Being a silly 馃, I don't know anything about framework design, but it seems like "outlet" isn't necessary or particularly helpful. When I first saw this, I wondered why not simplify to <Section ...> and <SectionContent ...>? It just seems like it's instantly easy to remember that way. No need to respond on that ... just a passing thought.

Example of non-layout use:

Shared/Child.razor:

<h3>Child</h3>

<p>Hello from the Child!</p>

<SectionContent SectionName="parentOutlet">
    Content provided by Child for 'parentOutlet' in Parent.
</SectionContent>

Pages/Parent.razor:

@page "/parent"

<h3>Parent</h3>

<p>Hello from the Parent!</p>

<div class="border border-info rounded-pill p-2">
    <SectionOutlet SectionName="parentOutlet" />
</div>

<Child />

Output:

image

@surayya-MS
Copy link
Member

What happens if you define a SectionOutlet but it isn't filled by a SectionContent? What happens if you define SectionContent but there isn't a matching SectionOutlet?

Nothing happens.

@surayya-MS
Copy link
Member

Also, is there ever a case where you'd use a SectionOutlet outside of a layout component? For example, can you use SectionOutlet in a parent component and then filling it from a child component?

Yes, there is. Thanks @guardrex for providing an example for this case.

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 18, 2023

I think @danroth27 will say we should have a topic for this in the Components node to avoid extending the already too long overview topic, but I'll wait to confirm it before setting this up that way.

Also, should we show camel or Pascal case as the example for the name? I know devs can do whatever they want, but I think we should pitch our example(s) one way.

... and just asking out of curiosity @surayya-MS, was just <Section /> going to cause a naming conflict 馃挜 within the framework or perhaps dev confusion with other things called "section" (e.g., @section), so you had to add something ("Outlet") to avoid a problem?

@surayya-MS
Copy link
Member

That works great, so shall we make this it's own topic under the Components node and call it "ASP.NET Core Blazor sections" and not refer to "layouts," just call this feature "sections" throughout?

I think it is a good idea! We should also put a link to sections in layouts docs as the main use case for sections was to change layouts.

@danroth27
Copy link
Member

danroth27 commented Apr 18, 2023

What happens if you define a SectionOutlet but it isn't filled by a SectionContent? What happens if you define SectionContent but there isn't a matching SectionOutlet?

Nothing happens.

@surayya-MS I'm seeing some errors when I try this out with .NET 8 Preview 3. If I create a SectionOutlet named IndexSection in Index.razor in a default Blazor Server app without a SectionContent to fill it, I get no error on the initial app load but if I click on the Counter tab and then back to the Home tab I get this:

System.InvalidOperationException: There is already a subscriber to the content with the given section ID 'IndexSection'.

@surayya-MS
Copy link
Member

Also, should we show camel or Pascal case as the example for the name? I know devs can do whatever they want, but I think we should pitch our example(s) one way.

For SectionName camel case. For SectionId both camel case and Pascal are good depending on the conventions for public fields. In the example for SectionId I prefer Pascal case for public static field.

@danroth27
Copy link
Member

danroth27 commented Apr 18, 2023

I think @danroth27 will say we should have a topic for this in the Components node to avoid extending the already too long overview topic, but I'll wait to confirm it before setting this up that way.

Sounds reasonable to me. I think we should still also mention sections in the layouts topic because that still is a common use case.

Also, should we show camel or Pascal case as the example for the name?

We could also use kebab-casing. That's what's often used for HTML IDs. @surayya-MS Thoughts?

@surayya-MS
Copy link
Member

We could also use kebab-casing. That's what's often used for HTML IDs.

Yes, I think kebab-case is better. @guardrex please ignore my previous comment where I said:

For SectionName camel case

Let's use kebab-case for SectionName.

@guardrex
Copy link
Collaborator Author

guardrex commented Apr 18, 2023

Latest commits ...

Add the following lines ...

Sections can be used in both [layouts](xref:blazor/components/layouts) and across nested parent-child components.

Although the argument passed to `SectionName` can use any type of casing, the documentation adopts kebab casing (for example, `top-bar`), which is a common casing choice for HTML element IDs. `SectionId` receives a static `object` field, and we always recommend Pascal casing for C# field names (for example, `TopbarSection`).

Moves the content to a new article ...

Title: ASP.NET Core Blazor sections
Physical location: blazor/components/sections.md
UID: blazor/components/sections
ToC location: Immediately under the Layouts article
Versioning: >= aspnetcore-8.0

Has a versioned Sections section in the layout article linking to the new article ...

## Sections

To control the content in a layout from a child Razor component, see <xref:blazor/components/sections>.

@surayya-MS
Copy link
Member

surayya-MS commented Apr 18, 2023

I'm seeing some errors when I try this out with .NET 8 Preview 3. If I create a SectionOutlet named IndexSection in Index.razor in a default Blazor Server app without a SectionContent to fill it, I get no error on the initial app load but if I click on the Counter tab and then back to the Home tab I get this:
System.InvalidOperationException: There is already a subscriber to the content with the given section ID 'IndexSection'.

@danroth27 this is strange behavior. I just tried it in the Samples/BlazorServerApp and there were no errors.

What is happening probably is that you have SectionOutlet with the same name in Index.razor and Counter.razor. But the exception should have appeared when you click on Counter.
This exception message "There is already a subscriber to the content with the given section ID" appears when there are many SectionOutlets with the same SectionName or SectionId

Could you please confirm if this is true?

@surayya-MS
Copy link
Member

... and just asking out of curiosity @surayya-MS, was just

going to cause a naming conflict 馃挜 within the framework or perhaps dev confusion with other things called "section" (e.g., @section), so you had to add something ("Outlet") to avoid a problem?

SectionOutlet and SectionContent already existed in the repo as internal classes for PageTitle component. So I cannot tell you because I did not name them. The naming looked good to me so I did not try to rename them. Thanks for bringing that up. I'll bring that up to the team and see what they think about it.

@guardrex
Copy link
Collaborator Author

@danroth27 @surayya-MS ... You're still discussing the ..... er .... 馃懡 strange behavior 馃浉馃槃 ... but is the PR ready to merge?

@surayya-MS
Copy link
Member

@danroth27 @surayya-MS ... You're still discussing the ..... er .... 馃懡 strange behavior 馃浉馃槃 ... but is the PR ready to merge?

I think it is. @danroth27 ?

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.

:shipit:

@guardrex guardrex merged commit 3497619 into main Apr 26, 2023
3 checks passed
@guardrex guardrex deleted the guardrex/blazor-layouts-sections branch April 26, 2023 15:06
Donciavas pushed a commit to Donciavas/AspNetCore.Docs that referenced this pull request Feb 7, 2024
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.

Blazor sections
4 participants