From 002cb51d7243c1f5bda89f64c61950b0ef0485ed Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 4 Jul 2019 22:37:23 -0700 Subject: [PATCH] Cleanup of components based on CaptureUnmatchedAttributes (#11580) Part of #10713 This change addresses the cleanup of the form components by removing the Id and Class parameters as explicit parameters. These components will now propagate the id and class attributes to the created HTML element without the need for explicit parameters. Note that we preserve the combining behaviour of class with FieldClass. * Update NavLink to use new features Simplifies the design of NavLink to use our new features for capturing unmatched attributes. I can't run the functional tests locally so I hope this works.... --- ...etCore.Components.netstandard2.0.Manual.cs | 8 +- .../src/Forms/InputComponents/InputBase.cs | 29 +++--- .../Forms/InputComponents/InputCheckbox.cs | 7 +- .../src/Forms/InputComponents/InputDate.cs | 7 +- .../src/Forms/InputComponents/InputNumber.cs | 13 ++- .../src/Forms/InputComponents/InputSelect.cs | 9 +- .../src/Forms/InputComponents/InputText.cs | 9 +- .../Forms/InputComponents/InputTextArea.cs | 9 +- .../Components/src/Routing/NavLink.cs | 92 ++++++++++--------- .../Components/test/Forms/InputBaseTest.cs | 39 ++------ 10 files changed, 99 insertions(+), 123 deletions(-) diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.Manual.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.Manual.cs index 61f8f87e49..4c25bec3fa 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.Manual.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.Manual.cs @@ -153,8 +153,6 @@ namespace Microsoft.AspNetCore.Components.Forms protected InputBase() { } [Parameter(CaptureUnmatchedValues = true)] public System.Collections.Generic.IReadOnlyDictionary AdditionalAttributes { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} - [Microsoft.AspNetCore.Components.ParameterAttribute] - public string Class { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} protected string CssClass { get { throw null; } } protected T CurrentValue { get { throw null; } set { } } protected string CurrentValueAsString { get { throw null; } set { } } @@ -162,8 +160,6 @@ namespace Microsoft.AspNetCore.Components.Forms protected string FieldClass { get { throw null; } } protected Microsoft.AspNetCore.Components.Forms.FieldIdentifier FieldIdentifier { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } [Microsoft.AspNetCore.Components.ParameterAttribute] - public string Id { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} - [Microsoft.AspNetCore.Components.ParameterAttribute] public T Value { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} [Microsoft.AspNetCore.Components.ParameterAttribute] public Microsoft.AspNetCore.Components.EventCallback ValueChanged { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} @@ -253,6 +249,10 @@ namespace Microsoft.AspNetCore.Components.Routing public NavLink() { } [Microsoft.AspNetCore.Components.ParameterAttribute] public string ActiveClass { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} + [Microsoft.AspNetCore.Components.ParameterAttribute(CaptureUnmatchedValues = true)] + public System.Collections.Generic.IReadOnlyDictionary AdditionalAttributes { get; private set; } + [Microsoft.AspNetCore.Components.ParameterAttribute] + public RenderFragment ChildContent { get; set; } [Microsoft.AspNetCore.Components.ParameterAttribute] public Microsoft.AspNetCore.Components.Routing.NavLinkMatch Match { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } private set { throw null; }} public void Configure(Microsoft.AspNetCore.Components.RenderHandle renderHandle) { } diff --git a/src/Components/Components/src/Forms/InputComponents/InputBase.cs b/src/Components/Components/src/Forms/InputComponents/InputBase.cs index 3d00474223..518876717f 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputBase.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputBase.cs @@ -26,16 +26,6 @@ namespace Microsoft.AspNetCore.Components.Forms /// [Parameter(CaptureUnmatchedValues = true)] public IReadOnlyDictionary AdditionalAttributes { get; private set; } - /// - /// Gets a value for the component's 'id' attribute. - /// - [Parameter] public string Id { get; private set; } - - /// - /// Gets a value for the component's 'class' attribute. - /// - [Parameter] public string Class { get; private set; } - /// /// Gets or sets the value of the input. This should be used with two-way binding. /// @@ -157,14 +147,25 @@ namespace Microsoft.AspNetCore.Components.Forms => EditContext.FieldClass(FieldIdentifier); /// - /// Gets a CSS class string that combines the and + /// Gets a CSS class string that combines the class attribute and /// properties. Derived components should typically use this value for the primary HTML element's /// 'class' attribute. /// protected string CssClass - => string.IsNullOrEmpty(Class) - ? FieldClass // Never null or empty - : $"{Class} {FieldClass}"; + { + get + { + if (AdditionalAttributes != null && + AdditionalAttributes.TryGetValue("class", out var @class) && + !string.IsNullOrEmpty(Convert.ToString(@class))) + { + return $"{@class} {FieldClass}"; + } + + return FieldClass; // Never null or empty + } + } + /// public override Task SetParametersAsync(ParameterCollection parameters) diff --git a/src/Components/Components/src/Forms/InputComponents/InputCheckbox.cs b/src/Components/Components/src/Forms/InputComponents/InputCheckbox.cs index 4cfcabd1aa..df84c8bdaa 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputCheckbox.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputCheckbox.cs @@ -26,10 +26,9 @@ namespace Microsoft.AspNetCore.Components.Forms builder.OpenElement(0, "input"); builder.AddMultipleAttributes(1, AdditionalAttributes); builder.AddAttribute(2, "type", "checkbox"); - builder.AddAttribute(3, "id", Id); - builder.AddAttribute(4, "class", CssClass); - builder.AddAttribute(5, "checked", BindMethods.GetValue(CurrentValue)); - builder.AddAttribute(6, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValue = __value, CurrentValue)); + builder.AddAttribute(3, "class", CssClass); + builder.AddAttribute(4, "checked", BindMethods.GetValue(CurrentValue)); + builder.AddAttribute(5, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValue = __value, CurrentValue)); builder.CloseElement(); } diff --git a/src/Components/Components/src/Forms/InputComponents/InputDate.cs b/src/Components/Components/src/Forms/InputComponents/InputDate.cs index 112bbf322d..add64bd6ff 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputDate.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputDate.cs @@ -25,10 +25,9 @@ namespace Microsoft.AspNetCore.Components.Forms builder.OpenElement(0, "input"); builder.AddMultipleAttributes(1, AdditionalAttributes); builder.AddAttribute(2, "type", "date"); - builder.AddAttribute(3, "id", Id); - builder.AddAttribute(4, "class", CssClass); - builder.AddAttribute(5, "value", BindMethods.GetValue(CurrentValueAsString)); - builder.AddAttribute(6, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); + builder.AddAttribute(3, "class", CssClass); + builder.AddAttribute(4, "value", BindMethods.GetValue(CurrentValueAsString)); + builder.AddAttribute(5, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); builder.CloseElement(); } diff --git a/src/Components/Components/src/Forms/InputComponents/InputNumber.cs b/src/Components/Components/src/Forms/InputComponents/InputNumber.cs index 2abc41c0d8..2a65f863c8 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputNumber.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputNumber.cs @@ -62,13 +62,12 @@ namespace Microsoft.AspNetCore.Components.Forms protected override void BuildRenderTree(RenderTreeBuilder builder) { builder.OpenElement(0, "input"); - builder.AddMultipleAttributes(1, AdditionalAttributes); - builder.AddAttribute(2, "type", "number"); - builder.AddAttribute(3, "step", _stepAttributeValue); - builder.AddAttribute(4, "id", Id); - builder.AddAttribute(5, "class", CssClass); - builder.AddAttribute(6, "value", BindMethods.GetValue(CurrentValueAsString)); - builder.AddAttribute(7, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); + builder.AddAttribute(1, "step", _stepAttributeValue); // Before the splat so the user can override + builder.AddMultipleAttributes(2, AdditionalAttributes); + builder.AddAttribute(3, "type", "number"); + builder.AddAttribute(4, "class", CssClass); + builder.AddAttribute(5, "value", BindMethods.GetValue(CurrentValueAsString)); + builder.AddAttribute(6, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); builder.CloseElement(); } diff --git a/src/Components/Components/src/Forms/InputComponents/InputSelect.cs b/src/Components/Components/src/Forms/InputComponents/InputSelect.cs index 7847b9f095..6705ce9320 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputSelect.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputSelect.cs @@ -21,11 +21,10 @@ namespace Microsoft.AspNetCore.Components.Forms { builder.OpenElement(0, "select"); builder.AddMultipleAttributes(1, AdditionalAttributes); - builder.AddAttribute(2, "id", Id); - builder.AddAttribute(3, "class", CssClass); - builder.AddAttribute(4, "value", BindMethods.GetValue(CurrentValueAsString)); - builder.AddAttribute(5, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); - builder.AddContent(6, ChildContent); + builder.AddAttribute(2, "class", CssClass); + builder.AddAttribute(3, "value", BindMethods.GetValue(CurrentValueAsString)); + builder.AddAttribute(4, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); + builder.AddContent(5, ChildContent); builder.CloseElement(); } diff --git a/src/Components/Components/src/Forms/InputComponents/InputText.cs b/src/Components/Components/src/Forms/InputComponents/InputText.cs index f1f08eb46a..ffb7004601 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputText.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputText.cs @@ -5,8 +5,6 @@ using Microsoft.AspNetCore.Components.RenderTree; namespace Microsoft.AspNetCore.Components.Forms { - // TODO: Support maxlength etc. - /* This is almost equivalent to a .razor file containing: * * @inherits InputBase @@ -26,10 +24,9 @@ namespace Microsoft.AspNetCore.Components.Forms { builder.OpenElement(0, "input"); builder.AddMultipleAttributes(1, AdditionalAttributes); - builder.AddAttribute(2, "id", Id); - builder.AddAttribute(3, "class", CssClass); - builder.AddAttribute(4, "value", BindMethods.GetValue(CurrentValue)); - builder.AddAttribute(5, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); + builder.AddAttribute(2, "class", CssClass); + builder.AddAttribute(3, "value", BindMethods.GetValue(CurrentValue)); + builder.AddAttribute(4, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); builder.CloseElement(); } diff --git a/src/Components/Components/src/Forms/InputComponents/InputTextArea.cs b/src/Components/Components/src/Forms/InputComponents/InputTextArea.cs index 3358d2e167..ed676d6404 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputTextArea.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputTextArea.cs @@ -5,8 +5,6 @@ using Microsoft.AspNetCore.Components.RenderTree; namespace Microsoft.AspNetCore.Components.Forms { - // TODO: Support rows/cols/etc - /* This is almost equivalent to a .razor file containing: * * @inherits InputBase @@ -26,10 +24,9 @@ namespace Microsoft.AspNetCore.Components.Forms { builder.OpenElement(0, "textarea"); builder.AddMultipleAttributes(1, AdditionalAttributes); - builder.AddAttribute(2, "id", Id); - builder.AddAttribute(3, "class", CssClass); - builder.AddAttribute(4, "value", BindMethods.GetValue(CurrentValue)); - builder.AddAttribute(5, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); + builder.AddAttribute(2, "class", CssClass); + builder.AddAttribute(3, "value", BindMethods.GetValue(CurrentValue)); + builder.AddAttribute(4, "onchange", EventCallback.Factory.CreateBinder(this, __value => CurrentValueAsString = __value, CurrentValueAsString)); builder.CloseElement(); } diff --git a/src/Components/Components/src/Routing/NavLink.cs b/src/Components/Components/src/Routing/NavLink.cs index 487981f12a..155a973cf1 100644 --- a/src/Components/Components/src/Routing/NavLink.cs +++ b/src/Components/Components/src/Routing/NavLink.cs @@ -3,43 +3,48 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.RenderTree; namespace Microsoft.AspNetCore.Components.Routing { - // NOTE: This could be implemented in a more performant way by iterating through - // the ParameterCollection only once (instead of multiple TryGetValue calls), and - // avoiding allocating a dictionary in the case where there are no additional params. - // However the intention here is to get a sense of what more high-level coding patterns - // will exist and what APIs are needed to support them. Later in the project when we - // have more examples of components implemented in pure C# (not Razor) we could change - // this one to the more low-level perf-sensitive implementation. - /// /// A component that renders an anchor tag, automatically toggling its 'active' /// class based on whether its 'href' matches the current URI. /// - public class NavLink : IComponent, IDisposable + public class NavLink : ComponentBase, IDisposable { private const string DefaultActiveClass = "active"; - private RenderHandle _renderHandle; private bool _isActive; - - private RenderFragment _childContent; - private string _cssClass; private string _hrefAbsolute; - private IReadOnlyDictionary _allAttributes; + private string _class; /// - /// Gets or sets the CSS class name applied to the NavLink when the + /// Gets or sets the CSS class name applied to the NavLink when the /// current route matches the NavLink href. /// [Parameter] public string ActiveClass { get; private set; } + /// + /// Gets or sets a collection of additional attributes that will be added to the generated + /// a element. + /// + [Parameter(CaptureUnmatchedValues = true)] + public IReadOnlyDictionary AdditionalAttributes { get; private set; } + + /// + /// Gets or sets the computed CSS class based on whether or not the link is active. + /// + protected string CssClass { get; private set; } + + /// + /// Gets or sets the child content of the component. + /// + [Parameter] + public RenderFragment ChildContent { get; set; } + /// /// Gets or sets a value representing the URL matching behavior. /// @@ -49,30 +54,32 @@ namespace Microsoft.AspNetCore.Components.Routing [Inject] private IUriHelper UriHelper { get; set; } /// - public void Configure(RenderHandle renderHandle) + protected override void OnInitialized() { - _renderHandle = renderHandle; - // We'll consider re-rendering on each location change UriHelper.OnLocationChanged += OnLocationChanged; } /// - public Task SetParametersAsync(ParameterCollection parameters) + protected override void OnParametersSet() { - // Capture the parameters we want to do special things with, plus all as a dictionary - parameters.TryGetValue(RenderTreeBuilder.ChildContent, out _childContent); - parameters.TryGetValue("class", out _cssClass); - parameters.TryGetValue("href", out string href); - ActiveClass = parameters.GetValueOrDefault(nameof(ActiveClass), DefaultActiveClass); - Match = parameters.GetValueOrDefault(nameof(Match), NavLinkMatch.Prefix); - _allAttributes = parameters.ToDictionary(); + // Update computed state + var href = (string)null; + if (AdditionalAttributes != null && AdditionalAttributes.TryGetValue("href", out var obj)) + { + href = Convert.ToString(obj); + } - // Update computed state and render _hrefAbsolute = href == null ? null : UriHelper.ToAbsoluteUri(href).AbsoluteUri; _isActive = ShouldMatch(UriHelper.GetAbsoluteUri()); - _renderHandle.Render(Render); - return Task.CompletedTask; + + _class = (string)null; + if (AdditionalAttributes != null && AdditionalAttributes.TryGetValue("class", out obj)) + { + _class = Convert.ToString(obj); + } + + UpdateCssClass(); } /// @@ -82,6 +89,11 @@ namespace Microsoft.AspNetCore.Components.Routing UriHelper.OnLocationChanged -= OnLocationChanged; } + private void UpdateCssClass() + { + CssClass = _isActive ? CombineWithSpace(_class, ActiveClass ?? DefaultActiveClass) : _class; + } + private void OnLocationChanged(object sender, LocationChangedEventArgs args) { // We could just re-render always, but for this component we know the @@ -90,7 +102,8 @@ namespace Microsoft.AspNetCore.Components.Routing if (shouldBeActiveNow != _isActive) { _isActive = shouldBeActiveNow; - _renderHandle.Render(Render); + UpdateCssClass(); + StateHasChanged(); } } @@ -137,22 +150,13 @@ namespace Microsoft.AspNetCore.Components.Routing return false; } - private void Render(RenderTreeBuilder builder) + protected override void BuildRenderTree(RenderTreeBuilder builder) { builder.OpenElement(0, "a"); - // Set class attribute - builder.AddAttribute(0, "class", - CombineWithSpace(_cssClass, _isActive ? ActiveClass : null)); - - // Pass through all other attributes unchanged - foreach (var kvp in _allAttributes.Where(kvp => kvp.Key != "class" && kvp.Key != nameof(RenderTreeBuilder.ChildContent))) - { - builder.AddAttribute(0, kvp.Key, kvp.Value); - } - - // Pass through any child content unchanged - builder.AddContent(1, _childContent); + builder.AddMultipleAttributes(1, AdditionalAttributes); + builder.AddAttribute(2, "class", CssClass); + builder.AddContent(3, ChildContent); builder.CloseElement(); } diff --git a/src/Components/Components/test/Forms/InputBaseTest.cs b/src/Components/Components/test/Forms/InputBaseTest.cs index a66078798c..62e40cb88b 100644 --- a/src/Components/Components/test/Forms/InputBaseTest.cs +++ b/src/Components/Components/test/Forms/InputBaseTest.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Components.Forms var inputComponent = new TestInputComponent(); var testRenderer = new TestRenderer(); var componentId = testRenderer.AssignRootComponentId(inputComponent); - + // Act/Assert var ex = await Assert.ThrowsAsync( () => testRenderer.RenderRootComponentAsync(componentId)); @@ -73,25 +73,6 @@ namespace Microsoft.AspNetCore.Components.Forms Assert.Equal("some value", inputComponent.CurrentValue); } - [Fact] - public async Task ExposesIdToSubclass() - { - // Arrange - var model = new TestModel(); - var rootComponent = new TestInputHostComponent> - { - Id = "test-id", - EditContext = new EditContext(model), - ValueExpression = () => model.StringProperty - }; - - // Act - var inputComponent = await RenderAndGetTestInputComponentAsync(rootComponent); - - // Assert - Assert.Same(rootComponent.Id, inputComponent.Id); - } - [Fact] public async Task ExposesEditContextToSubclass() { @@ -264,7 +245,10 @@ namespace Microsoft.AspNetCore.Components.Forms var model = new TestModel(); var rootComponent = new TestInputHostComponent> { - Class = "my-class other-class", + AdditionalAttributes = new Dictionary() + { + { "class", "my-class other-class" }, + }, EditContext = new EditContext(model), ValueExpression = () => model.StringProperty }; @@ -370,7 +354,7 @@ namespace Microsoft.AspNetCore.Components.Forms .OfType() .Single(); - private static async Task RenderAndGetTestInputComponentAsync(TestInputHostComponent hostComponent) where TComponent: TestInputComponent + private static async Task RenderAndGetTestInputComponentAsync(TestInputHostComponent hostComponent) where TComponent : TestInputComponent { var testRenderer = new TestRenderer(); var componentId = testRenderer.AssignRootComponentId(hostComponent); @@ -401,7 +385,7 @@ namespace Microsoft.AspNetCore.Components.Forms set { base.CurrentValueAsString = value; } } - public new string Id => base.Id; + public new IReadOnlyDictionary AdditionalAttributes => base.AdditionalAttributes; public new string CssClass => base.CssClass; @@ -437,11 +421,9 @@ namespace Microsoft.AspNetCore.Components.Forms } } - class TestInputHostComponent : AutoRenderComponent where TComponent: TestInputComponent + class TestInputHostComponent : AutoRenderComponent where TComponent : TestInputComponent { - public string Id { get; set; } - - public string Class { get; set; } + public Dictionary AdditionalAttributes { get; set; } public EditContext EditContext { get; set; } @@ -462,8 +444,7 @@ namespace Microsoft.AspNetCore.Components.Forms childBuilder.AddAttribute(1, "ValueChanged", EventCallback.Factory.Create(this, ValueChanged)); childBuilder.AddAttribute(2, "ValueExpression", ValueExpression); - childBuilder.AddAttribute(3, nameof(Id), Id); - childBuilder.AddAttribute(4, nameof(Class), Class); + childBuilder.AddMultipleAttributes(3, AdditionalAttributes); childBuilder.CloseComponent(); })); builder.CloseComponent();