From 87bb1d0ff580478ceccc3679671a765f5d6738c2 Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Fri, 5 May 2017 11:53:53 -0700 Subject: [PATCH] Refactor GenerateCheckBox (#6229) Addresses #5981 and #5983 --- .../InputTagHelper.cs | 59 ++++++------------ ...ite.HtmlGeneration_Home.Order.Encoded.html | 2 +- ...tionWebSite.HtmlGeneration_Home.Order.html | 2 +- .../InputTagHelperTest.cs | 61 +++++++++---------- 4 files changed, 48 insertions(+), 76 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs index 8934baca73..820f966fb8 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs @@ -194,8 +194,8 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers break; case "checkbox": - GenerateCheckBox(modelExplorer, output); - return; + tagBuilder = GenerateCheckBox(modelExplorer, output); + break; case "password": tagBuilder = Generator.GeneratePassword( @@ -252,7 +252,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers return inputTypeHint; } - private void GenerateCheckBox(ModelExplorer modelExplorer, TagHelperOutput output) + private TagBuilder GenerateCheckBox(ModelExplorer modelExplorer, TagHelperOutput output) { if (modelExplorer.ModelType == typeof(string)) { @@ -280,53 +280,30 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers "checkbox")); } - // Prepare to move attributes from current element to generated just below. - var htmlAttributes = new Dictionary(StringComparer.OrdinalIgnoreCase); - - // Perf: Avoid allocating enumerator - // Construct attributes correctly (first attribute wins). - for (var i = 0; i < output.Attributes.Count; i++) + // hiddenForCheckboxTag always rendered after the returned element + var hiddenForCheckboxTag = Generator.GenerateHiddenForCheckbox(ViewContext, modelExplorer, For.Name); + if (hiddenForCheckboxTag != null) { - var attribute = output.Attributes[i]; - if (!htmlAttributes.ContainsKey(attribute.Name)) + var renderingMode = + output.TagMode == TagMode.SelfClosing ? TagRenderMode.SelfClosing : TagRenderMode.StartTag; + hiddenForCheckboxTag.TagRenderMode = renderingMode; + + if (ViewContext.FormContext.CanRenderAtEndOfForm) { - htmlAttributes.Add(attribute.Name, attribute.Value); + ViewContext.FormContext.EndOfFormContent.Add(hiddenForCheckboxTag); + } + else + { + output.PostElement.AppendHtml(hiddenForCheckboxTag); } } - var checkBoxTag = Generator.GenerateCheckBox( + return Generator.GenerateCheckBox( ViewContext, modelExplorer, For.Name, isChecked: null, - htmlAttributes: htmlAttributes); - if (checkBoxTag != null) - { - // Do not generate current element's attributes or tags. Instead put both and - // into the output's Content. - output.Attributes.Clear(); - output.TagName = null; - - var renderingMode = - output.TagMode == TagMode.SelfClosing ? TagRenderMode.SelfClosing : TagRenderMode.StartTag; - checkBoxTag.TagRenderMode = renderingMode; - output.Content.AppendHtml(checkBoxTag); - - var hiddenForCheckboxTag = Generator.GenerateHiddenForCheckbox(ViewContext, modelExplorer, For.Name); - if (hiddenForCheckboxTag != null) - { - hiddenForCheckboxTag.TagRenderMode = renderingMode; - - if (ViewContext.FormContext.CanRenderAtEndOfForm) - { - ViewContext.FormContext.EndOfFormContent.Add(hiddenForCheckboxTag); - } - else - { - output.Content.AppendHtml(hiddenForCheckboxTag); - } - } - } + htmlAttributes: null); } private TagBuilder GenerateRadio(ModelExplorer modelExplorer) diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html index 97428777dc..aee1e808a2 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html @@ -65,7 +65,7 @@
- +
diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html index 2d40eeca1f..54a6af28f8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html @@ -65,7 +65,7 @@
- +
diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/InputTagHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/InputTagHelperTest.cs index a92a669983..e9799a4cbb 100644 --- a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/InputTagHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/InputTagHelperTest.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { "hello", "world" }, { "hello", "world2" } }, - "hello=\"HtmlEncode[[world]]\"" + "hello=\"HtmlEncode[[world]]\" hello=\"HtmlEncode[[world2]]\"" }, { new TagHelperAttributeList @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { "hello", "world2" }, { "hello", "world3" } }, - "hello=\"HtmlEncode[[world]]\"" + "hello=\"HtmlEncode[[world]]\" hello=\"HtmlEncode[[world2]]\" hello=\"HtmlEncode[[world3]]\"" }, { new TagHelperAttributeList @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { "HelLO", "world" }, { "HELLO", "world2" } }, - "HelLO=\"HtmlEncode[[world]]\"" + "HelLO=\"HtmlEncode[[world]]\" HELLO=\"HtmlEncode[[world2]]\"" }, { new TagHelperAttributeList @@ -60,7 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { "HELLO", "world2" }, { "hello", "world3" } }, - "Hello=\"HtmlEncode[[world]]\"" + "Hello=\"HtmlEncode[[world]]\" HELLO=\"HtmlEncode[[world2]]\" hello=\"HtmlEncode[[world3]]\"" }, { new TagHelperAttributeList @@ -68,7 +68,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { "HeLlO", "world" }, { "hello", "world2" } }, - "HeLlO=\"HtmlEncode[[world]]\"" + "HeLlO=\"HtmlEncode[[world]]\" hello=\"HtmlEncode[[world2]]\"" }, }; } @@ -76,15 +76,14 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers [Theory] [MemberData(nameof(MultiAttributeCheckBoxData))] - public async Task CheckBoxHandlesMultipleAttributesSameNameCorrectly( + public async Task CheckBoxHandlesMultipleAttributesSameNameArePreserved( TagHelperAttributeList outputAttributes, string expectedAttributeString) { // Arrange var originalContent = "original content"; - var originalTagName = "not-input"; - var expectedContent = $"{originalContent}" + + var expectedContent = $"" + ""; var context = new TagHelperContext( @@ -94,12 +93,13 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers items: new Dictionary(), uniqueId: "test"); var output = new TagHelperOutput( - originalTagName, + "input", outputAttributes, getChildContentAsync: (useCachedResult, encoder) => Task.FromResult(result: null)) { TagMode = TagMode.SelfClosing, }; + output.Content.AppendHtml(originalContent); var htmlGenerator = new TestableHtmlGenerator(new EmptyModelMetadataProvider()); var tagHelper = GetTagHelper(htmlGenerator, model: false, propertyName: nameof(Model.IsACar)); @@ -108,10 +108,9 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers await tagHelper.ProcessAsync(context, output); // Assert - Assert.Empty(output.Attributes); // Moved to Content and cleared - Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output.Content)); - Assert.Equal(TagMode.SelfClosing, output.TagMode); - Assert.Null(output.TagName); // Cleared + Assert.NotNull(output.PostElement); + Assert.Equal(originalContent, HtmlContentUtilities.HtmlContentToString(output.Content)); + Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output)); } [Theory] @@ -213,13 +212,13 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers // Arrange const string content = "original content"; const string tagName = "input"; - const string isCheckedAttr = " checked=\"HtmlEncode[[checked]]\""; + const string isCheckedAttr = "checked=\"HtmlEncode[[checked]]\" "; var isChecked = (bool.Parse(possibleBool) ? isCheckedAttr : string.Empty); - var expectedContent = $"{content}"; - + var expectedPostElement = ""; var attributes = new TagHelperAttributeList { @@ -245,12 +244,11 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers // Act tagHelper.Process(context, output); - + // Assert - Assert.Empty(output.Attributes); - Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output.Content)); - Assert.Equal(TagMode.SelfClosing, output.TagMode); - Assert.Null(output.TagName); + Assert.Equal(content, HtmlContentUtilities.HtmlContentToString(output.Content)); + Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output)); + Assert.Equal(expectedPostElement, output.PostElement.GetContent()); } // Top-level container (List or Model instance), immediate container type (Model or NestModel), @@ -464,10 +462,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { // Arrange var originalContent = "original content"; - var originalTagName = "not-input"; var expectedPreContent = "original pre-content"; - var expectedContent = originalContent + ""; + var expectedContent = ""; var expectedPostContent = "original post-content"; + var expectedPostElement = ""; var context = new TagHelperContext( tagName: "input", @@ -480,7 +478,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { "class", "form-control" }, }; var output = new TagHelperOutput( - originalTagName, + "input", originalAttributes, getChildContentAsync: (useCachedResult, encoder) => { @@ -501,10 +499,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers var tagBuilder = new TagBuilder("input") { - Attributes = - { - { "class", "form-control" }, - }, TagRenderMode = TagRenderMode.SelfClosing }; htmlGenerator @@ -530,12 +524,13 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers // Assert htmlGenerator.Verify(); - Assert.Empty(output.Attributes); // Moved to Content and cleared + Assert.NotEmpty(output.Attributes); Assert.Equal(expectedPreContent, output.PreContent.GetContent()); - Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output.Content)); + Assert.Equal(originalContent, HtmlContentUtilities.HtmlContentToString(output.Content)); + Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output)); Assert.Equal(expectedPostContent, output.PostContent.GetContent()); + Assert.Equal(expectedPostElement, output.PostElement.GetContent()); Assert.Equal(TagMode.SelfClosing, output.TagMode); - Assert.Null(output.TagName); // Cleared } [Theory]