From bdff1b547c2c4944cd83a92a2bf4e9a65609e3a7 Mon Sep 17 00:00:00 2001 From: Surayya Huseyn Zada Date: Wed, 15 Mar 2023 16:48:11 +0100 Subject: [PATCH 1/4] Added SectionName to SectionOutlet and SectionContent --- .../Components/src/PublicAPI.Unshipped.txt | 4 + .../Components/src/Sections/SectionContent.cs | 81 ++++++---- .../Components/src/Sections/SectionOutlet.cs | 43 ++++-- .../src/Sections/SectionRegistry.cs | 52 +++---- .../test/E2ETest/Tests/SectionsTest.cs | 121 ++++++++++++++- .../ParentComponentWithTwoChildren.razor | 141 ++++++++++++++++-- 6 files changed, 360 insertions(+), 82 deletions(-) diff --git a/src/Components/Components/src/PublicAPI.Unshipped.txt b/src/Components/Components/src/PublicAPI.Unshipped.txt index 2709f48969f1..d94d5c7494b9 100644 --- a/src/Components/Components/src/PublicAPI.Unshipped.txt +++ b/src/Components/Components/src/PublicAPI.Unshipped.txt @@ -11,10 +11,14 @@ 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.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.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 diff --git a/src/Components/Components/src/Sections/SectionContent.cs b/src/Components/Components/src/Sections/SectionContent.cs index b19cf3525a5f..8d2461b022fc 100644 --- a/src/Components/Components/src/Sections/SectionContent.cs +++ b/src/Components/Components/src/Sections/SectionContent.cs @@ -8,15 +8,22 @@ namespace Microsoft.AspNetCore.Components.Sections; /// public sealed class SectionContent : ISectionContentProvider, IComponent, IDisposable { - private object? _registeredSectionId; + private object? _identifier; + private object? _registeredIdentifier; private bool? _registeredIsDefaultContent; private SectionRegistry _registry = default!; /// - /// Gets or sets the ID that determines which instance will render + /// Gets or sets the ID of type that determines which instance will render /// the content of this instance. /// - [Parameter, EditorRequired] public object SectionId { get; set; } = default!; + [Parameter] public string SectionName { get; set; } = default!; + + /// + /// Gets or sets the ID of type that determines which instance will render + /// the content of this instance. + /// + [Parameter] public object SectionId { get; set; } = default!; /// /// Gets or sets whether this component should provide the default content for the target @@ -37,11 +44,54 @@ 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 + SetCustomParameters(parameters); + + if (SectionName is not null && SectionId is not null) + { + throw new InvalidOperationException($"Both '{nameof(SectionName)}' and '{nameof(SectionId)}' cannot have 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; + _registeredIsDefaultContent = IsDefaultContent; + } + + _registry.NotifyContentChanged(_identifier, this); + + return Task.CompletedTask; + } + + private void SetCustomParameters(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; @@ -55,35 +105,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; } /// public void Dispose() { - if (_registeredSectionId is not null) + if (_registeredIdentifier is not null) { - _registry.RemoveProvider(_registeredSectionId, this); + _registry.RemoveProvider(_registeredIdentifier, this); } } } diff --git a/src/Components/Components/src/Sections/SectionOutlet.cs b/src/Components/Components/src/Sections/SectionOutlet.cs index e7f481052014..b971f314f27d 100644 --- a/src/Components/Components/src/Sections/SectionOutlet.cs +++ b/src/Components/Components/src/Sections/SectionOutlet.cs @@ -10,16 +10,23 @@ public sealed class SectionOutlet : ISectionContentSubscriber, IComponent, IDisp { private static readonly RenderFragment _emptyRenderFragment = _ => { }; - private object? _subscribedSectionId; + private object? _identifier; + private object? _subscribedIdentifier; private RenderHandle _renderHandle; private SectionRegistry _registry = default!; private RenderFragment? _content; /// - /// Gets or sets the ID that determines which instances will provide + /// Gets or sets the ID of type that determines which instances will provide /// content to this instance. /// - [Parameter, EditorRequired] public object SectionId { get; set; } = default!; + [Parameter] public string SectionName { get; set; } = default!; + + /// + /// Gets or sets the ID of type that determines which instances will provide + /// content to this instance. + /// + [Parameter] public object SectionId { get; set; } = default!; void IComponent.Attach(RenderHandle renderHandle) { @@ -31,20 +38,32 @@ Task IComponent.SetParametersAsync(ParameterView parameters) { parameters.SetParameterProperties(this); - if (SectionId is null) + if (SectionName is not null && SectionId is not null) + { + throw new InvalidOperationException($"Both '{nameof(SectionName)}' and '{nameof(SectionId)}' cannot have 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(); @@ -74,9 +93,9 @@ private void RenderContent() /// public void Dispose() { - if (_subscribedSectionId is not null) + if (_subscribedIdentifier is not null) { - _registry.Unsubscribe(_subscribedSectionId); + _registry.Unsubscribe(_subscribedIdentifier); } } } diff --git a/src/Components/Components/src/Sections/SectionRegistry.cs b/src/Components/Components/src/Sections/SectionRegistry.cs index 0976deccc466..3e5a0908c5d1 100644 --- a/src/Components/Components/src/Sections/SectionRegistry.cs +++ b/src/Components/Components/src/Sections/SectionRegistry.cs @@ -5,15 +5,15 @@ namespace Microsoft.AspNetCore.Components.Sections; internal sealed class SectionRegistry { - private readonly Dictionary _subscribersBySectionId = new(); - private readonly Dictionary> _providersBySectionId = new(); + private readonly Dictionary _subscribersByIdentifier = new(); + private readonly Dictionary> _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) @@ -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); @@ -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); } } @@ -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); } diff --git a/src/Components/test/E2ETest/Tests/SectionsTest.cs b/src/Components/test/E2ETest/Tests/SectionsTest.cs index 4e798996daa2..c82666c2f706 100644 --- a/src/Components/test/E2ETest/Tests/SectionsTest.cs +++ b/src/Components/test/E2ETest/Tests/SectionsTest.cs @@ -8,6 +8,7 @@ using Xunit.Abstractions; using Microsoft.AspNetCore.Components.E2ETest; using OpenQA.Selenium; +using Microsoft.AspNetCore.Components.Sections; namespace Microsoft.AspNetCore.Components.E2ETests.Tests; @@ -29,6 +30,90 @@ protected override void InitializeAsyncCore() _appElement = Browser.MountTestComponent(); } + [Fact] + public void RenderTwoSectionOutletsWithSameSectionId_TrowsException() + { + _appElement.FindElement(By.Id("section-outlet-same-id")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains("There is already a subscriber to the content with the given section ID 'System.Object'", logs[0].Message); + } + + [Fact] + public void RenderTwoSectionOutletsWithSameSectionName_TrowsException() + { + _appElement.FindElement(By.Id("section-outlet-same-name")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains("There is already a subscriber to the content with the given section ID 'test1'", logs[0].Message); + } + + [Fact] + public void RenderTwoSectionOutletsWithEqualSectionNameToSectionId_TrowsException() + { + _appElement.FindElement(By.Id("section-outlet-equal-name-id")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains("There is already a subscriber to the content with the given section ID 'test2'", logs[0].Message); + } + + [Fact] + public void RenderSectionOutletWithSectionNameAndSectionId_TrowsException() + { + _appElement.FindElement(By.Id("section-outlet-with-name-id")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains($"Both '{nameof(SectionOutlet.SectionName)}' and '{nameof(SectionOutlet.SectionId)}' cannot have values.", logs[0].Message); + } + + [Fact] + public void RenderSectionOutletWithoutSectionNameAndSectionId_TrowsException() + { + _appElement.FindElement(By.Id("section-outlet-without-name-id")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains($"{nameof(SectionOutlet)} requires a non-null value either for '{nameof(SectionOutlet.SectionName)}' or '{nameof(SectionOutlet.SectionId)}'.", logs[0].Message); + } + + [Fact] + public void RenderSectionContentWithSectionNameAndSectionId_ThrowsException() + { + _appElement.FindElement(By.Id("section-content-with-name-id")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains($"Both '{nameof(SectionContent.SectionName)}' and '{nameof(SectionContent.SectionId)}' cannot have values.", logs[0].Message); + } + + [Fact] + public void RenderSectionContentWithoutSectionNameAndSectionId_ThrowsException() + { + _appElement.FindElement(By.Id("section-content-without-name-id")).Click(); + + var logs = Browser.GetBrowserLogs(LogLevel.Severe); + + Assert.True(logs.Count > 0); + + Assert.Contains($"{nameof(SectionContent)} requires a non-null value either for '{nameof(SectionContent.SectionName)}' or '{nameof(SectionContent.SectionId)}'.", logs[0].Message); + } + [Fact] public void NoExistingSectionContents_SectionOutletsRenderNothing() { @@ -38,7 +123,7 @@ public void NoExistingSectionContents_SectionOutletsRenderNothing() } [Fact] - public void RenderOneSectionContent_MatchingSectionOutletRendersContentSuccessfully() + public void RenderSectionContentWithSectionId_MatchingSectionOutletRendersContentSuccessfully() { _appElement.FindElement(By.Id("counter-render-section-content")).Click(); @@ -51,7 +136,15 @@ public void RenderOneSectionContent_MatchingSectionOutletRendersContentSuccessfu } [Fact] - public void RenderTwoSectionContentsWithSameId_LastRenderedOverridesSectionOutletContent() + public void RenderSectionContentWithSectionName_MatchingSectionOutletRendersContentSuccessfully() + { + _appElement.FindElement(By.Id("section-content-with-name")).Click(); + + Browser.Exists(By.Id("test6")); + } + + [Fact] + public void RenderTwoSectionContentsWithSameSectionId_LastRenderedOverridesSectionOutletContent() { _appElement.FindElement(By.Id("counter-render-section-content")).Click(); @@ -90,14 +183,14 @@ public void BothSectionContentsGetDisposed_SectionOutletsRenderNothing() } [Fact] - public void SectionContentIdChanges_MatchingSectionOutletRendersContent() + public void SectionContentSectionIdChanges_MatchingSectionOutletWithSectionNameRendersContent() { // Render Counter and TextComponent SectionContents with same Name // TextComponent SectionContent overrides Counter SectionContent _appElement.FindElement(By.Id("counter-render-section-content")).Click(); _appElement.FindElement(By.Id("text-render-section-content")).Click(); - _appElement.FindElement(By.Id("counter-change-section-content-name")).Click(); + _appElement.FindElement(By.Id("counter-change-section-content-id")).Click(); Browser.Exists(By.Id("counter")); } @@ -110,11 +203,29 @@ public void SectionContentIdChangesToNonExisting_NoMatchingSectionOutletResultin _appElement.FindElement(By.Id("counter-render-section-content")).Click(); _appElement.FindElement(By.Id("text-render-section-content")).Click(); - _appElement.FindElement(By.Id("counter-change-section-content-name-nonexisting")).Click(); + _appElement.FindElement(By.Id("counter-change-section-content-id-nonexisting")).Click(); Browser.DoesNotExist(By.Id("counter")); } + [Fact] + public void SectionContentSectionNameChanges_MatchingSectionOutletWithSectionIdRendersContent() + { + _appElement.FindElement(By.Id("section-content-with-name")).Click(); + + _appElement.FindElement(By.Id("counter-render-section-content")).Click(); + _appElement.FindElement(By.Id("counter-change-section-content-id")).Click(); + + // Counter Component Content overrides second SectionContent + Browser.DoesNotExist(By.Id("test6")); + Browser.Exists(By.Id("counter")); + + _appElement.FindElement(By.Id("section-content-with-name-change-name")).Click(); + + Browser.Exists(By.Id("test6")); + Browser.Exists(By.Id("counter")); + } + [Fact] public void SectionOutletGetsDisposed_NoContentsRendered() { diff --git a/src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor b/src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor index 4834b62abcd2..1d48ec85a868 100644 --- a/src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor +++ b/src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor @@ -2,20 +2,118 @@

Parent Component

-@if (SectionOutletExists) +@if (FirstSectionOutletExists) { }

Text between two section outlets

- + + +

Text between two section outlets

+ + + +@if (SectionOutletWithSameSectionIdExists) +{ + +} + +
+ +@if (SectionOutletWithSameSectionNameExists) +{ + + +} + +
+ +@if (SectionOutletWithEqualSectionNameToSectionIdExist) +{ + + +} + +
+ +@if (SectionOutletWithSectionNameAndSectionIdExists) +{ + +} + +
+ +@if (SectionOutletWithoutSectionNameAndSectionIdExists) +{ + +} + +
+ +@if (SectionContentWithSectionNameAndSectionIdExists) +{ + +

Test4

+
+} + +
+ +@if (SectionContentWithoutSectionNameAndSectionIdExists) +{ + +

Test5

+
+} + +
+ +@if (SectionContentWithSectionNameExists) +{ + +

Test6

+
+} + +
+ +
+ @@ -33,16 +131,22 @@ Dispose Counter SectionContent
-
-
+ +
+ @@ -59,14 +163,25 @@
@code { + private bool FirstSectionOutletExists = true; + private bool SectionOutletWithSameSectionIdExists = false; + private bool SectionOutletWithSameSectionNameExists = false; + private bool SectionOutletWithEqualSectionNameToSectionIdExist = false; + private bool SectionOutletWithSectionNameAndSectionIdExists = false; + private bool SectionOutletWithoutSectionNameAndSectionIdExists = false; + + private bool SectionContentWithSectionNameAndSectionIdExists = false; + private bool SectionContentWithoutSectionNameAndSectionIdExists = false; + private bool SectionContentWithSectionNameExists = false; + private static object FirstSectionId = new(); - private static int SecondSectionId = 123; + private static string SecondSectionName = "second-section"; + private static string ThirdSectionName = "thrird-section"; private bool CounterSectionContentExists = false; private bool TextComponentSectionContentExists = false; private object CounterSectionContentId = FirstSectionId; private object TextComponentSectionContentId = FirstSectionId; - - private bool SectionOutletExists = true; + private string TestSectionContentName = SecondSectionName; } From 29a41728c8903b2ce0ad59e550a857d96f0207f4 Mon Sep 17 00:00:00 2001 From: Surayya Huseyn Zada Date: Thu, 16 Mar 2023 18:46:44 +0100 Subject: [PATCH 2/4] change error message; make identifier a local variable; change description of SectionName and SectionId --- .../Components/src/Sections/SectionContent.cs | 23 ++++++++++--------- .../Components/src/Sections/SectionOutlet.cs | 17 +++++++------- .../test/E2ETest/Tests/SectionsTest.cs | 4 ++-- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/Components/Components/src/Sections/SectionContent.cs b/src/Components/Components/src/Sections/SectionContent.cs index 8d2461b022fc..7a563dc05c8e 100644 --- a/src/Components/Components/src/Sections/SectionContent.cs +++ b/src/Components/Components/src/Sections/SectionContent.cs @@ -8,19 +8,18 @@ namespace Microsoft.AspNetCore.Components.Sections; ///
public sealed class SectionContent : ISectionContentProvider, IComponent, IDisposable { - private object? _identifier; private object? _registeredIdentifier; private bool? _registeredIsDefaultContent; private SectionRegistry _registry = default!; /// - /// Gets or sets the ID of type that determines which instance will render + /// Gets or sets the ID that determines which instance will render /// the content of this instance. /// [Parameter] public string SectionName { get; set; } = default!; /// - /// Gets or sets the ID of type that determines which instance will render + /// Gets or sets the ID that determines which instance will render /// the content of this instance. /// [Parameter] public object SectionId { get; set; } = default!; @@ -47,43 +46,45 @@ Task IComponent.SetParametersAsync(ParameterView parameters) { // We are not using parameters.SetParameterProperties(this) // because IsDefaultContent is internal property and not a parameter - SetCustomParameters(parameters); + SetParameterValues(parameters); + + object? identifier; if (SectionName is not null && SectionId is not null) { - throw new InvalidOperationException($"Both '{nameof(SectionName)}' and '{nameof(SectionId)}' cannot have values."); + 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; + identifier = SectionName; } else if (SectionId is not null) { - _identifier = SectionId; + 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 (!object.Equals(identifier, _registeredIdentifier) || IsDefaultContent != _registeredIsDefaultContent) { if (_registeredIdentifier is not null) { _registry.RemoveProvider(_registeredIdentifier, this); } - _registry.AddProvider(_identifier, this, IsDefaultContent); + _registry.AddProvider(identifier, this, IsDefaultContent); _registeredIdentifier = SectionId; _registeredIsDefaultContent = IsDefaultContent; } - _registry.NotifyContentChanged(_identifier, this); + _registry.NotifyContentChanged(identifier, this); return Task.CompletedTask; } - private void SetCustomParameters(ParameterView parameters) + private void SetParameterValues(ParameterView parameters) { foreach (var param in parameters) { diff --git a/src/Components/Components/src/Sections/SectionOutlet.cs b/src/Components/Components/src/Sections/SectionOutlet.cs index b971f314f27d..cfa92a3bae37 100644 --- a/src/Components/Components/src/Sections/SectionOutlet.cs +++ b/src/Components/Components/src/Sections/SectionOutlet.cs @@ -10,20 +10,19 @@ public sealed class SectionOutlet : ISectionContentSubscriber, IComponent, IDisp { private static readonly RenderFragment _emptyRenderFragment = _ => { }; - private object? _identifier; private object? _subscribedIdentifier; private RenderHandle _renderHandle; private SectionRegistry _registry = default!; private RenderFragment? _content; /// - /// Gets or sets the ID of type that determines which instances will provide + /// Gets or sets the ID that determines which instances will provide /// content to this instance. /// [Parameter] public string SectionName { get; set; } = default!; /// - /// Gets or sets the ID of type that determines which instances will provide + /// Gets or sets the ID that determines which instances will provide /// content to this instance. /// [Parameter] public object SectionId { get; set; } = default!; @@ -38,31 +37,33 @@ Task IComponent.SetParametersAsync(ParameterView parameters) { parameters.SetParameterProperties(this); + object? identifier; + if (SectionName is not null && SectionId is not null) { - throw new InvalidOperationException($"Both '{nameof(SectionName)}' and '{nameof(SectionId)}' cannot have values."); + 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; + identifier = SectionName; } else if (SectionId is not null) { - _identifier = SectionId; + identifier = SectionId; } else { throw new InvalidOperationException($"{nameof(SectionOutlet)} requires a non-null value either for '{nameof(SectionName)}' or '{nameof(SectionId)}'."); } - if (!object.Equals(_identifier, _subscribedIdentifier)) + if (!object.Equals(identifier, _subscribedIdentifier)) { if (_subscribedIdentifier is not null) { _registry.Unsubscribe(_subscribedIdentifier); } - _registry.Subscribe(_identifier, this); + _registry.Subscribe(identifier, this); _subscribedIdentifier = SectionId; } diff --git a/src/Components/test/E2ETest/Tests/SectionsTest.cs b/src/Components/test/E2ETest/Tests/SectionsTest.cs index c82666c2f706..ab10f02da5ed 100644 --- a/src/Components/test/E2ETest/Tests/SectionsTest.cs +++ b/src/Components/test/E2ETest/Tests/SectionsTest.cs @@ -75,7 +75,7 @@ public void RenderSectionOutletWithSectionNameAndSectionId_TrowsException() Assert.True(logs.Count > 0); - Assert.Contains($"Both '{nameof(SectionOutlet.SectionName)}' and '{nameof(SectionOutlet.SectionId)}' cannot have values.", logs[0].Message); + Assert.Contains($"{nameof(SectionOutlet)} requires that '{nameof(SectionOutlet.SectionName)}' and '{nameof(SectionOutlet.SectionId)}' cannot both have non-null values.", logs[0].Message); } [Fact] @@ -99,7 +99,7 @@ public void RenderSectionContentWithSectionNameAndSectionId_ThrowsException() Assert.True(logs.Count > 0); - Assert.Contains($"Both '{nameof(SectionContent.SectionName)}' and '{nameof(SectionContent.SectionId)}' cannot have values.", logs[0].Message); + Assert.Contains($"{nameof(SectionContent)} requires that '{nameof(SectionContent.SectionName)}' and '{nameof(SectionContent.SectionId)}' cannot both have non-null values.", logs[0].Message); } [Fact] From ab486e09976b1e187490a07dc3ba0ef43c032bd9 Mon Sep 17 00:00:00 2001 From: Surayya Huseyn Zada Date: Thu, 16 Mar 2023 18:50:18 +0100 Subject: [PATCH 3/4] in ParameterView --- src/Components/Components/src/Sections/SectionContent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/Components/src/Sections/SectionContent.cs b/src/Components/Components/src/Sections/SectionContent.cs index 7a563dc05c8e..6c6205e94734 100644 --- a/src/Components/Components/src/Sections/SectionContent.cs +++ b/src/Components/Components/src/Sections/SectionContent.cs @@ -84,7 +84,7 @@ Task IComponent.SetParametersAsync(ParameterView parameters) return Task.CompletedTask; } - private void SetParameterValues(ParameterView parameters) + private void SetParameterValues(in ParameterView parameters) { foreach (var param in parameters) { From bb41125600795f41274621b25eceef0c038dac12 Mon Sep 17 00:00:00 2001 From: Surayya Huseyn Zada Date: Thu, 16 Mar 2023 19:00:28 +0100 Subject: [PATCH 4/4] make SectionName and SectionId nullable --- src/Components/Components/src/PublicAPI.Unshipped.txt | 8 ++++---- src/Components/Components/src/Sections/SectionContent.cs | 4 ++-- src/Components/Components/src/Sections/SectionOutlet.cs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Components/Components/src/PublicAPI.Unshipped.txt b/src/Components/Components/src/PublicAPI.Unshipped.txt index d94d5c7494b9..caba076f349a 100644 --- a/src/Components/Components/src/PublicAPI.Unshipped.txt +++ b/src/Components/Components/src/PublicAPI.Unshipped.txt @@ -9,15 +9,15 @@ 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.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.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 diff --git a/src/Components/Components/src/Sections/SectionContent.cs b/src/Components/Components/src/Sections/SectionContent.cs index 6c6205e94734..c2f722d9a566 100644 --- a/src/Components/Components/src/Sections/SectionContent.cs +++ b/src/Components/Components/src/Sections/SectionContent.cs @@ -16,13 +16,13 @@ public sealed class SectionContent : ISectionContentProvider, IComponent, IDispo /// Gets or sets the ID that determines which instance will render /// the content of this instance. /// - [Parameter] public string SectionName { get; set; } = default!; + [Parameter] public string? SectionName { get; set; } /// /// Gets or sets the ID that determines which instance will render /// the content of this instance. /// - [Parameter] public object SectionId { get; set; } = default!; + [Parameter] public object? SectionId { get; set; } /// /// Gets or sets whether this component should provide the default content for the target diff --git a/src/Components/Components/src/Sections/SectionOutlet.cs b/src/Components/Components/src/Sections/SectionOutlet.cs index cfa92a3bae37..1cfa836e7cc8 100644 --- a/src/Components/Components/src/Sections/SectionOutlet.cs +++ b/src/Components/Components/src/Sections/SectionOutlet.cs @@ -19,13 +19,13 @@ public sealed class SectionOutlet : ISectionContentSubscriber, IComponent, IDisp /// Gets or sets the ID that determines which instances will provide /// content to this instance. /// - [Parameter] public string SectionName { get; set; } = default!; + [Parameter] public string? SectionName { get; set; } /// /// Gets or sets the ID that determines which instances will provide /// content to this instance. /// - [Parameter] public object SectionId { get; set; } = default!; + [Parameter] public object? SectionId { get; set; } void IComponent.Attach(RenderHandle renderHandle) {