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

Blazor Sections API Proposal #46937

Closed
surayya-MS opened this issue Feb 28, 2023 · 14 comments · Fixed by #47221
Closed

Blazor Sections API Proposal #46937

surayya-MS opened this issue Feb 28, 2023 · 14 comments · Fixed by #47221
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one
Milestone

Comments

@surayya-MS
Copy link
Member

surayya-MS commented Feb 28, 2023

Background and Motivation

Related to #28182.
There is a need for a feature that will allow customers to change a part of main layout depending on which page they are on. A solution to achieve that exists and was described in #28182 (comment). Similar solution was implemented as part of #10450. https://github.com/dotnet/aspnetcore/tree/main/src/Components/Components/src/Sections these classes are internal.
We want to make them a part of public API (and change a few things for other potential use cases).

Proposed API

SectionOutlet component that renders content provided by SectionContent with the same SectionId

public sealed class SectionOutlet : IComponent, IDisposable
{
    [Parameter, EditorRequired] public object SectionId { get; set; }
}
public sealed class SectionContent : IComponent, IDisposable
{
    [Parameter, EditorRequired] public object SectionId { get; set; }
    [Parameter] public RenderFragment? ChildContent { get; set; }
{

Usage Examples

Common use case would be to use SectionOutlet component in MainLayout.razor and provide the desired content via SectionContent component in other pages.
This is not the only case. You could use SectionOutlet SectionContent to modify a part of parent component depending on which child components are rendered.

Example:

  • Add this code into the default template's MainLayout.razor:
@code{
    internal static object TopbarSection = new();
}
  • Add SectionOutlet into the default template's MainLayout.razor:
<div class="top-row px-4">
    <SectionOutlet SectionId="TopbarSection" />
    <a href="https://docs.microsoft.com/aspnet/" target="_blank">About</a>
</div>
  • Add SectionContent into Counter.razor:
<SectionContent SectionId="MainLayout.TopbarSection">
    <button class="btn btn-primary" @onclick="IncrementCount">Click me</button>
</SectionContent>

You'll get this:

image

If you don't want to use a static field inMainlayout.razor you could also use a string as SectionId:

<SectionOutlet SectionId="@("topbar")" />

In Counter.razor:

<SectionContent SectionId="@("topbar")">
    <button class="btn btn-primary" @onclick="IncrementCount">Click me</button>
</SectionContent>

Alternative Designs

The original design with string Name as an identifier instead of object SectionId.

public sealed class SectionOutlet : IComponent, IDisposable
{
    [Parameter, EditorRequired] public string Name { get; set; }
}
public sealed class SectionContent : IComponent, IDisposable
{
    [Parameter, EditorRequired] public string Name { get; set; }
    [Parameter] public RenderFragment? ChildContent { get; set; }
{

With this design you cannot hide your SectionOutlet. This is useful when you are developing a library and you don't want any accidental content in the user's app to be rendered in library's component.

Risks

Case when SectionId is IEquatable<T> and Equals(object), GetHashCode() not overriden.

@surayya-MS surayya-MS added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-blazor Includes: Blazor, Razor Components api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Feb 28, 2023
@ghost
Copy link

ghost commented Feb 28, 2023

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@mkArtakMSFT mkArtakMSFT added this to the 8.0-preview3 milestone Feb 28, 2023
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Feb 28, 2023
@surayya-MS surayya-MS mentioned this issue Feb 28, 2023
4 tasks
@BrennanConroy
Copy link
Member

API Review Notes:

  • How easy is it to pass objects around in Blazor?
    • You can have a parameter in a child component or inject via a service/using static or Cascading Value
  • What's wrong with string Name?
    • Easy to conflict with someone else, e.g. I use "head" and another library uses "head", we'll step on eachother.
    • Can't avoid conflicts with an error since it's not necessarily the users fault and wouldn't be fixable if multiple libraries conflicted. Plus you might want multiple SectionOutlets with the same "name" in a single app
  • Namespace? Is .Sections fine instead of the "main" Components namespace?
    • Yes, we think it'll be less commonly used than the "core" components, and other components do use custom namespaces as well, .Web, .Authorization, e.g.

API approved!

namespace Microsoft.AspNetCore.Components.Sections;

+ public sealed class SectionOutlet : IComponent, IDisposable
+ {
+     [Parameter, EditorRequired] public object SectionId { get; set; }

+     // <inheritdoc/>
+     void Attach(RenderHandle renderHandle);

+     // <inheritdoc/>
+     Task SetParametersAsync(ParameterView parameters);

+     // <inheritdoc/>
+     public void Dispose();
+ }

+ public sealed class SectionContent : IComponent, IDisposable
+ {
+     [Parameter, EditorRequired] public object SectionId { get; set; }
+     [Parameter] public RenderFragment? ChildContent { get; set; }

+     // <inheritdoc/>
+     void Attach(RenderHandle renderHandle);

+     // <inheritdoc/>
+     Task SetParametersAsync(ParameterView parameters);

+     // <inheritdoc/>
+     public void Dispose();
+ }

@BrennanConroy BrennanConroy added api-approved API was approved in API review, it can be implemented and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Mar 6, 2023
@mkArtakMSFT
Copy link
Member

@guardrex can you add docs for this feature based on the description from @surayya-MS ?

@guardrex
Copy link
Contributor

guardrex commented Mar 8, 2023

Yes. I'll ping on the (upcoming) docs issue if I get stuck.

@msynk
Copy link

msynk commented Mar 9, 2023

great idea👍
I suggest using Slot instead of the more general name Section.
The Slot is also used in a lot of other frameworks/libraries out there.

@eaton-sam
Copy link

How does this work with cascading parameters? Are they supported?
If a component used in a SectionContent has a CascadingParameter property, should that parameter be provided by the component hierarchy of the SectionContent or SectionOutlet?

For example, should this CounterTopBar component output "Hello from MainLayout" or "Hello from Counter":

MainLayout.razor

<div class="top-row px-4">
  <CascadingValue Value="@("Hello from MainLayout")">
      <SectionOutlet SectionId="TopbarSection" />
      <a href="https://docs.microsoft.com/aspnet/" target="_blank">About</a>
  </CascadingValue>
</div>

CounterTopBar.razor

<div>Here's some content with a cascading value: @CascadedValue</div>

@code {
   [CascadingParameter] public string CascadedValue {get;set;}
}

Counter.razor

<CascadingValue Value="@("Hello from Counter")">
  <SectionContent SectionId="MainLayout.TopbarSection">
     <CounterTopBar />
  </SectionContent>
</CascadingValue>

I think that currently this parameter would be provided by the MainLayout cascading value and would render "Hello from MainLayout". This makes sense, but I'm not sure that it's obvious for users.

A real world example would be an app where a page has a state object, which it cascades to all components on the page. The MainLayout could have a SectionOutlet in the header for rendering quick actions. Any components placed in a SectionContent for this outlet would have a null state value in their cascading parameter.

@yv989c
Copy link

yv989c commented Mar 9, 2023

great idea👍
I suggest using Slot instead of the more general name Section.
The Slot is also used in a lot of other frameworks/libraries out there.

@msynk I also like the idea of using some form of Slot in the naming, but more than that I would like to have it implemented closer to the concept of slots as in the Web Components standard. This way its usage will be more natural to those familiar with them. Right now what have been proposed is not aligned with the patterns used by this standard.

https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_templates_and_slots#adding_flexibility_with_slots

@ElderJames
Copy link
Contributor

I think it's better to allow passing parameters to the SectionOutlet as the context of the SectionContent child content.

@surayya-MS surayya-MS reopened this Mar 13, 2023
@guardrex
Copy link
Contributor

guardrex commented Mar 13, 2023

@surayya-MS ... Are additional changes coming? I just put the PR up at ...

dotnet/AspNetCore.Docs#28660

... which I can hold off on pinging u for review if additional changes are coming.

@surayya-MS
Copy link
Member Author

@guardrex , yes. I'll update the issue description tomorrow.

@guardrex
Copy link
Contributor

I've marked the PR as a draft for now. I'll keep an 👁️ on this issue and associated PRs here. I'll ping u later when the docs PR is ready for review after your next round of updates.

@surayya-MS
Copy link
Member Author

New Design

There is a problem with the current design.

If you want to use a string for SectionId you should do this <SectionOutlet SectionId="@("topbar")" /> which is not a very good user experience.

You cannot just do this <SectionOutlet SectionId="topbar" /> because there is no variable with name 'topbar'. Compiler error: CS0103 "The name 'topbar' does not exist in current context".
Btw, this would work: <SectionOutlet SectionId="123" />. SectionId here is an integer 123.

Since the case where SectionId is a string will be used a lot I propose new design.

public sealed class SectionOutlet : IComponent, IDisposable
{
+   [Parameter] public string SectionName { get; set; }
-   [Parameter, EditorRequired] public object SectionId { get; set; }
+   [Parameter] public object SectionId { get; set; }
}
public sealed class SectionContent : IComponent, IDisposable
{
+   [Parameter] public string SectionName { get; set; }
-   [Parameter, EditorRequired] public object SectionId { get; set; }
+   [Parameter] public object SectionId { get; set; }
    [Parameter] public RenderFragment? ChildContent { get; set; }
}

With SectionName you can set an ID of type string for SectionOutlet and SectionContent.
You now can do this: <SectionOutlet SectionName="topbar" />

You can set an ID either using SectionName or SectionId. That is why I removed EditorRequired attribute from SectionId. If both of them are not null an exception will be thrown.

Updated Usage Examples

Common use case would be to use SectionOutlet component in MainLayout.razor and provide the desired content via SectionContent component in other pages.
This is not the only case. You could use SectionOutlet SectionContent to modify a part of parent component depending on which child components are rendered.

Example:

  • Add SectionOutlet into the default template's MainLayout.razor:
<div class="top-row px-4">
    <SectionOutlet SectionId="topbar" />
    <a href="https://docs.microsoft.com/aspnet/" target="_blank">About</a>
</div>
  • Add SectionContent into Counter.razor:
<SectionContent SectionId="topbar">
    <button class="btn btn-primary" @onclick="IncrementCount">Click me</button>
</SectionContent>

You'll get this:

image

If you want to "hide" your SectionOutlet from accidental rendering of content from other SectionContents with same SectionName you can use SectionId to implicitly set an ID of type object. (This could be useful for creating a library).

Previous example could be achieved by using SectionId instead of SectionName:

  • Add this code into the default template's MainLayout.razor:
@code{
    internal static object TopbarSection = new();
}
  • Add SectionOutlet into the default template's MainLayout.razor:
<div class="top-row px-4">
    <SectionOutlet SectionId="TopbarSection" />
    <a href="https://docs.microsoft.com/aspnet/" target="_blank">About</a>
</div>
  • Add SectionContent into Counter.razor:
<SectionContent SectionId="MainLayout.TopbarSection">
    <button class="btn btn-primary" @onclick="IncrementCount">Click me</button>
</SectionContent>

@surayya-MS surayya-MS added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-approved API was approved in API review, it can be implemented labels Mar 15, 2023
@ghost
Copy link

ghost commented Mar 15, 2023

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@halter73
Copy link
Member

Thanks for the API update. I agree that SectionId="@("topbar")" is ugly.

I marked both SectionName and SectionId as nullable. I imagine if the SectionId is unset, it can return the SectionName from the getter, but the same isn't true in reverse. And there's the transient state before either is set when both could be null. Let me know if this doesn't make sense.

I'm not sure if it's obvious that SectionName and SectionId are essentially equivalent, but no one has proposed better names. In the interest of unblocking the work for preview3, I'll mark this api-approved again ahead of our normal API review meeting.

API Approved!

namespace Microsoft.AspNetCore.Components.Sections;

+ public sealed class SectionOutlet : IComponent, IDisposable
+ {
+     [Parameter] public string? SectionName { get; set; }
+     [Parameter] public object? SectionId { get; set; }

+     // <inheritdoc/>
+     void Attach(RenderHandle renderHandle);

+     // <inheritdoc/>
+     Task SetParametersAsync(ParameterView parameters);

+     // <inheritdoc/>
+     public void Dispose();
+ }

+ public sealed class SectionContent : IComponent, IDisposable
+ {
+     [Parameter] public string? SectionName { get; set; }
+     [Parameter] public object? SectionId { get; set; }

+     [Parameter] public RenderFragment? ChildContent { get; set; }

+     // <inheritdoc/>
+     void Attach(RenderHandle renderHandle);

+     // <inheritdoc/>
+     Task SetParametersAsync(ParameterView parameters);

+     // <inheritdoc/>
+     public void Dispose();
+ }

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Mar 16, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants