Skip to content

Add SectionName to SectionOutlet and SectionContent #47221

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 17, 2023
Merged

Add SectionName to SectionOutlet and SectionContent #47221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bdff1b5
Added SectionName to SectionOutlet and SectionContent
surayya-MS Mar 15, 2023
29a4172
change error message; make identifier a local variable; change descri…
surayya-MS Mar 16, 2023
ab486e0
in ParameterView
surayya-MS Mar 16, 2023
bb41125
make SectionName and SectionId nullable
surayya-MS Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/Components/Components/src/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ Microsoft.AspNetCore.Components.Sections.SectionContent.ChildContent.get -> Micr
Microsoft.AspNetCore.Components.Sections.SectionContent.ChildContent.set -> void
Microsoft.AspNetCore.Components.Sections.SectionContent.Dispose() -> void
Microsoft.AspNetCore.Components.Sections.SectionContent.SectionContent() -> void
Microsoft.AspNetCore.Components.Sections.SectionContent.SectionId.get -> object!
Microsoft.AspNetCore.Components.Sections.SectionContent.SectionId.get -> object?
Microsoft.AspNetCore.Components.Sections.SectionContent.SectionId.set -> void
Microsoft.AspNetCore.Components.Sections.SectionContent.SectionName.get -> string?
Microsoft.AspNetCore.Components.Sections.SectionContent.SectionName.set -> void
Microsoft.AspNetCore.Components.Sections.SectionOutlet
Microsoft.AspNetCore.Components.Sections.SectionOutlet.Dispose() -> void
Microsoft.AspNetCore.Components.Sections.SectionOutlet.SectionId.get -> object!
Microsoft.AspNetCore.Components.Sections.SectionOutlet.SectionId.get -> object?
Microsoft.AspNetCore.Components.Sections.SectionOutlet.SectionId.set -> void
Microsoft.AspNetCore.Components.Sections.SectionOutlet.SectionName.get -> string?
Microsoft.AspNetCore.Components.Sections.SectionOutlet.SectionName.set -> void
Microsoft.AspNetCore.Components.Sections.SectionOutlet.SectionOutlet() -> void
Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder.AddComponentParameter(int sequence, string! name, object? value) -> void
override Microsoft.AspNetCore.Components.EventCallback.GetHashCode() -> int
Expand Down
82 changes: 56 additions & 26 deletions src/Components/Components/src/Sections/SectionContent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,21 @@ namespace Microsoft.AspNetCore.Components.Sections;
/// </summary>
public sealed class SectionContent : ISectionContentProvider, IComponent, IDisposable
{
private object? _registeredSectionId;
private object? _registeredIdentifier;
private bool? _registeredIsDefaultContent;
private SectionRegistry _registry = default!;

/// <summary>
/// Gets or sets the ID that determines which <see cref="SectionOutlet"/> instance will render
/// Gets or sets the <see cref="string"/> ID that determines which <see cref="SectionOutlet"/> instance will render
/// the content of this instance.
/// </summary>
[Parameter, EditorRequired] public object SectionId { get; set; } = default!;
[Parameter] public string? SectionName { get; set; }

/// <summary>
/// Gets or sets the <see cref="object"/> ID that determines which <see cref="SectionOutlet"/> instance will render
/// the content of this instance.
/// </summary>
[Parameter] public object? SectionId { get; set; }

/// <summary>
/// Gets or sets whether this component should provide the default content for the target
Expand All @@ -37,11 +43,56 @@ void IComponent.Attach(RenderHandle renderHandle)
}

Task IComponent.SetParametersAsync(ParameterView parameters)
{
// We are not using parameters.SetParameterProperties(this)
// because IsDefaultContent is internal property and not a parameter
SetParameterValues(parameters);

object? identifier;

if (SectionName is not null && SectionId is not null)
{
throw new InvalidOperationException($"{nameof(SectionContent)} requires that '{nameof(SectionName)}' and '{nameof(SectionId)}' cannot both have non-null values.");
}
else if (SectionName is not null)
{
identifier = SectionName;
}
else if (SectionId is not null)
{
identifier = SectionId;
}
else
{
throw new InvalidOperationException($"{nameof(SectionContent)} requires a non-null value either for '{nameof(SectionName)}' or '{nameof(SectionId)}'.");
}

if (!object.Equals(identifier, _registeredIdentifier) || IsDefaultContent != _registeredIsDefaultContent)
{
if (_registeredIdentifier is not null)
{
_registry.RemoveProvider(_registeredIdentifier, this);
}

_registry.AddProvider(identifier, this, IsDefaultContent);
_registeredIdentifier = SectionId;
Copy link

Choose a reason for hiding this comment

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

Now that you also can use SectionName to identify a section _registeredIdentifier should be set to identifier instead of SectionId. Otherwise when using the SectionName it would be null, and RemoveProvider isn't called when it should.

The same issue exists in SectionOutlet.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Flachdachs this is a closed PR, so could you log an issue with details around your concern?

Copy link

Choose a reason for hiding this comment

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

Of course. Actually my intention was to open an issue. I visited the page with the changes anonymously. It offered a "New Issue" button. But as I opened the same URL as logged in user the button was replaced with a Create Review button.

_registeredIsDefaultContent = IsDefaultContent;
}

_registry.NotifyContentChanged(identifier, this);

return Task.CompletedTask;
}

private void SetParameterValues(in ParameterView parameters)
{
foreach (var param in parameters)
{
switch (param.Name)
{
case nameof(SectionContent.SectionName):
SectionName = (string)param.Value;
break;
case nameof(SectionContent.SectionId):
SectionId = param.Value;
break;
Expand All @@ -55,35 +106,14 @@ Task IComponent.SetParametersAsync(ParameterView parameters)
throw new ArgumentException($"Unknown parameter '{param.Name}'");
}
}

if (SectionId is null)
{
throw new InvalidOperationException($"{nameof(SectionContent)} requires a non-null value for the parameter '{nameof(SectionId)}'.");
}

if (!object.Equals(SectionId, _registeredSectionId) || IsDefaultContent != _registeredIsDefaultContent)
{
if (_registeredSectionId is not null)
{
_registry.RemoveProvider(_registeredSectionId, this);
}

_registry.AddProvider(SectionId, this, IsDefaultContent);
_registeredSectionId = SectionId;
_registeredIsDefaultContent = IsDefaultContent;
}

_registry.NotifyContentChanged(SectionId, this);

return Task.CompletedTask;
}

/// <inheritdoc/>
public void Dispose()
{
if (_registeredSectionId is not null)
if (_registeredIdentifier is not null)
{
_registry.RemoveProvider(_registeredSectionId, this);
_registry.RemoveProvider(_registeredIdentifier, this);
}
}
}
44 changes: 32 additions & 12 deletions src/Components/Components/src/Sections/SectionOutlet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,22 @@ public sealed class SectionOutlet : ISectionContentSubscriber, IComponent, IDisp
{
private static readonly RenderFragment _emptyRenderFragment = _ => { };

private object? _subscribedSectionId;
private object? _subscribedIdentifier;
private RenderHandle _renderHandle;
private SectionRegistry _registry = default!;
private RenderFragment? _content;

/// <summary>
/// Gets or sets the ID that determines which <see cref="SectionContent"/> instances will provide
/// Gets or sets the <see cref="string"/> ID that determines which <see cref="SectionContent"/> instances will provide
/// content to this instance.
/// </summary>
[Parameter, EditorRequired] public object SectionId { get; set; } = default!;
[Parameter] public string? SectionName { get; set; }

/// <summary>
/// Gets or sets the <see cref="object"/> ID that determines which <see cref="SectionContent"/> instances will provide
/// content to this instance.
/// </summary>
[Parameter] public object? SectionId { get; set; }

void IComponent.Attach(RenderHandle renderHandle)
{
Expand All @@ -31,20 +37,34 @@ Task IComponent.SetParametersAsync(ParameterView parameters)
{
parameters.SetParameterProperties(this);

if (SectionId is null)
object? identifier;

if (SectionName is not null && SectionId is not null)
{
throw new InvalidOperationException($"{nameof(SectionOutlet)} requires that '{nameof(SectionName)}' and '{nameof(SectionId)}' cannot both have non-null values.");
}
else if (SectionName is not null)
{
identifier = SectionName;
}
else if (SectionId is not null)
{
identifier = SectionId;
}
else
{
throw new InvalidOperationException($"{nameof(SectionOutlet)} requires a non-null value for the parameter '{nameof(SectionId)}'.");
throw new InvalidOperationException($"{nameof(SectionOutlet)} requires a non-null value either for '{nameof(SectionName)}' or '{nameof(SectionId)}'.");
}

if (!object.Equals(SectionId, _subscribedSectionId))
if (!object.Equals(identifier, _subscribedIdentifier))
{
if (_subscribedSectionId is not null)
if (_subscribedIdentifier is not null)
{
_registry.Unsubscribe(_subscribedSectionId);
_registry.Unsubscribe(_subscribedIdentifier);
}

_registry.Subscribe(SectionId, this);
_subscribedSectionId = SectionId;
_registry.Subscribe(identifier, this);
_subscribedIdentifier = SectionId;
}

RenderContent();
Expand Down Expand Up @@ -74,9 +94,9 @@ private void RenderContent()
/// <inheritdoc/>
public void Dispose()
{
if (_subscribedSectionId is not null)
if (_subscribedIdentifier is not null)
{
_registry.Unsubscribe(_subscribedSectionId);
_registry.Unsubscribe(_subscribedIdentifier);
}
}
}
52 changes: 26 additions & 26 deletions src/Components/Components/src/Sections/SectionRegistry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ namespace Microsoft.AspNetCore.Components.Sections;

internal sealed class SectionRegistry
{
private readonly Dictionary<object, ISectionContentSubscriber> _subscribersBySectionId = new();
private readonly Dictionary<object, List<ISectionContentProvider>> _providersBySectionId = new();
private readonly Dictionary<object, ISectionContentSubscriber> _subscribersByIdentifier = new();
private readonly Dictionary<object, List<ISectionContentProvider>> _providersByIdentifier = new();

public void AddProvider(object sectionId, ISectionContentProvider provider, bool isDefaultProvider)
public void AddProvider(object identifier, ISectionContentProvider provider, bool isDefaultProvider)
{
if (!_providersBySectionId.TryGetValue(sectionId, out var providers))
if (!_providersByIdentifier.TryGetValue(identifier, out var providers))
{
providers = new();
_providersBySectionId.Add(sectionId, providers);
_providersByIdentifier.Add(identifier, providers);
}

if (isDefaultProvider)
Expand All @@ -26,18 +26,18 @@ public void AddProvider(object sectionId, ISectionContentProvider provider, bool
}
}

public void RemoveProvider(object sectionId, ISectionContentProvider provider)
public void RemoveProvider(object identifier, ISectionContentProvider provider)
{
if (!_providersBySectionId.TryGetValue(sectionId, out var providers))
if (!_providersByIdentifier.TryGetValue(identifier, out var providers))
{
throw new InvalidOperationException($"There are no content providers with the given section ID '{sectionId}'.");
throw new InvalidOperationException($"There are no content providers with the given section ID '{identifier}'.");
}

var index = providers.LastIndexOf(provider);

if (index < 0)
{
throw new InvalidOperationException($"The provider was not found in the providers list of the given section ID '{sectionId}'.");
throw new InvalidOperationException($"The provider was not found in the providers list of the given section ID '{identifier}'.");
}

providers.RemoveAt(index);
Expand All @@ -47,44 +47,44 @@ public void RemoveProvider(object sectionId, ISectionContentProvider provider)
// We just removed the most recently added provider, meaning we need to change
// the current content to that of second most recently added provider.
var content = GetCurrentProviderContentOrDefault(providers);
NotifyContentChangedForSubscriber(sectionId, content);
NotifyContentChangedForSubscriber(identifier, content);
}
}

public void Subscribe(object sectionId, ISectionContentSubscriber subscriber)
public void Subscribe(object identifier, ISectionContentSubscriber subscriber)
{
if (_subscribersBySectionId.ContainsKey(sectionId))
if (_subscribersByIdentifier.ContainsKey(identifier))
{
throw new InvalidOperationException($"There is already a subscriber to the content with the given section ID '{sectionId}'.");
throw new InvalidOperationException($"There is already a subscriber to the content with the given section ID '{identifier}'.");
}

// Notify the new subscriber with any existing content.
var content = GetCurrentProviderContentOrDefault(sectionId);
var content = GetCurrentProviderContentOrDefault(identifier);
subscriber.ContentChanged(content);

_subscribersBySectionId.Add(sectionId, subscriber);
_subscribersByIdentifier.Add(identifier, subscriber);
}

public void Unsubscribe(object sectionId)
public void Unsubscribe(object identifier)
{
if (!_subscribersBySectionId.Remove(sectionId))
if (!_subscribersByIdentifier.Remove(identifier))
{
throw new InvalidOperationException($"The subscriber with the given section ID '{sectionId}' is already unsubscribed.");
throw new InvalidOperationException($"The subscriber with the given section ID '{identifier}' is already unsubscribed.");
}
}

public void NotifyContentChanged(object sectionId, ISectionContentProvider provider)
public void NotifyContentChanged(object identifier, ISectionContentProvider provider)
{
if (!_providersBySectionId.TryGetValue(sectionId, out var providers))
if (!_providersByIdentifier.TryGetValue(identifier, out var providers))
{
throw new InvalidOperationException($"There are no content providers with the given section ID '{sectionId}'.");
throw new InvalidOperationException($"There are no content providers with the given section ID '{identifier}'.");
}

// We only notify content changed for subscribers when the content of the
// most recently added provider changes.
if (providers.Count != 0 && providers[^1] == provider)
{
NotifyContentChangedForSubscriber(sectionId, provider.Content);
NotifyContentChangedForSubscriber(identifier, provider.Content);
}
}

Expand All @@ -93,14 +93,14 @@ public void NotifyContentChanged(object sectionId, ISectionContentProvider provi
? providers[^1].Content
: null;

private RenderFragment? GetCurrentProviderContentOrDefault(object sectionId)
=> _providersBySectionId.TryGetValue(sectionId, out var existingList)
private RenderFragment? GetCurrentProviderContentOrDefault(object identifier)
=> _providersByIdentifier.TryGetValue(identifier, out var existingList)
? GetCurrentProviderContentOrDefault(existingList)
: null;

private void NotifyContentChangedForSubscriber(object sectionId, RenderFragment? content)
private void NotifyContentChangedForSubscriber(object identifier, RenderFragment? content)
{
if (_subscribersBySectionId.TryGetValue(sectionId, out var subscriber))
if (_subscribersByIdentifier.TryGetValue(identifier, out var subscriber))
{
subscriber.ContentChanged(content);
}
Expand Down
Loading