Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit d5d5f7f

Browse files
committed
Address PR II comments for <input/> tag helper
- also catch up to match recent changes made in this branch nit: deliniate attribute names in other resource strings
1 parent dc3658c commit d5d5f7f

File tree

7 files changed

+82
-62
lines changed

7 files changed

+82
-62
lines changed

src/Microsoft.AspNet.Mvc.TagHelpers/InputTagHelper.cs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,13 @@ public class InputTagHelper : TagHelper
5353
{ "time", "{0:HH:mm:ss.fff}" },
5454
};
5555

56+
// Protected to ensure subclasses are correctly activated. Internal for ease of use when testing.
5657
[Activate]
57-
private IHtmlGenerator Generator { get; set; }
58+
protected internal IHtmlGenerator Generator { get; set; }
5859

60+
// Protected to ensure subclasses are correctly activated. Internal for ease of use when testing.
5961
[Activate]
60-
private ViewContext ViewContext { get; set; }
62+
protected internal ViewContext ViewContext { get; set; }
6163

6264
/// <summary>
6365
/// An expression to be evaluated against the current model.
@@ -70,28 +72,34 @@ public class InputTagHelper : TagHelper
7072
/// attribute to that formatted <see cref="string"/>.
7173
/// </summary>
7274
/// <remarks>
73-
/// Used only if <see cref="Value "/> is <c>null</c> and <see cref="Process"/> calls
75+
/// Used only the calculated "type" attribute is "text" (the most common value) e.g.
76+
/// <see cref="InputTypeName"/> is "String". That is, <see cref="Format"/> is used when calling
7477
/// <see cref="IHtmlGenerator.GenerateTextBox"/>.
7578
/// </remarks>
7679
public string Format { get; set; }
7780

7881
/// <summary>
79-
/// The type of the &lt;input&gt; element. Passed through to the generated HTML in all cases. Also used to
80-
/// determine the <see cref="IHtmlGenerator"/> helper <see cref="Process"/> will call and the default
81-
/// <see cref="Format"/> value.
82+
/// The type of the &lt;input&gt; element.
8283
/// </summary>
84+
/// <remarks>
85+
/// Passed through to the generated HTML in all cases. Also used to determine the <see cref="IHtmlGenerator"/>
86+
/// helper to call and the default <see cref="Format"/> value (when calling
87+
/// <see cref="IHtmlGenerator.GenerateTextBox"/>).
88+
/// </remarks>
8389
[HtmlAttributeName("type")]
8490
public string InputTypeName { get; set; }
8591

8692
/// <summary>
87-
/// The value of the &lt;input&gt; element. Passed through to the generated HTML in all cases. Also used to
88-
/// determine the generated "checked" attribute if <see cref="InputTypeName"/> is "radio".
93+
/// The value of the &lt;input&gt; element.
8994
/// </summary>
90-
/// <remarks>Must not be <c>null</c> if <see cref="InputTypeName"/> is "radio".</remarks>
95+
/// <remarks>
96+
/// Passed through to the generated HTML in all cases. Also used to determine the generated "checked" attribute
97+
/// if <see cref="InputTypeName"/> is "radio". Must not be <c>null</c> in that case.
98+
/// </remarks>
9199
public string Value { get; set; }
92100

93101
/// <inheritdoc />
94-
/// <remarks>Does nothing unless user binds "for" attribute in Razor source.</remarks>
102+
/// <remarks>Does nothing if <see cref="For"/> is <c>null</c></remarks>
95103
public override void Process(TagHelperContext context, TagHelperOutput output)
96104
{
97105
// Pass through attributes that are also well-known HTML attributes. Must be done prior to any copying
@@ -112,6 +120,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
112120
if (Format != null)
113121
{
114122
throw new InvalidOperationException(Resources.FormatInputTagHelper_UnableToFormat(
123+
"<input>",
115124
nameof(For).ToLowerInvariant(),
116125
nameof(Format).ToLowerInvariant()));
117126
}
@@ -124,6 +133,8 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
124133
if (metadata == null)
125134
{
126135
throw new InvalidOperationException(Resources.FormatTagHelpers_NoProvidedMetadata(
136+
"<input>",
137+
nameof(For).ToLowerInvariant(),
127138
nameof(IModelMetadataProvider),
128139
For.Name));
129140
}
@@ -143,7 +154,9 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
143154
if (!string.IsNullOrEmpty(inputType))
144155
{
145156
// inputType may be more specific than default the generator chooses below.
146-
if (!output.Attributes.ContainsKey("type"))
157+
// TODO: Use Attributes.ContainsKey once aspnet/Razor#186 is fixed.
158+
if (!output.Attributes.Any(
159+
item => string.Equals("type", item.Key, StringComparison.OrdinalIgnoreCase)))
147160
{
148161
output.Attributes["type"] = inputType;
149162
}
@@ -186,6 +199,8 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
186199

187200
if (tagBuilder != null)
188201
{
202+
// This TagBuilder contains the one <input/> element of interest. Since this is not the "checkbox"
203+
// special -case, output is a self-closing element and can merge the TagBuilder in directly.
189204
output.SelfClosing = true;
190205
output.Merge(tagBuilder);
191206
}
@@ -197,10 +212,11 @@ private void GenerateCheckBox(ModelMetadata metadata, TagHelperOutput output)
197212
if (typeof(bool) != metadata.RealModelType)
198213
{
199214
throw new InvalidOperationException(Resources.FormatInputTagHelper_InvalidExpressionResult(
200-
metadata.RealModelType.FullName,
215+
"<input>",
201216
nameof(For).ToLowerInvariant(),
217+
metadata.RealModelType.FullName,
202218
typeof(bool).FullName,
203-
nameof(InputTypeName).ToLowerInvariant(),
219+
"type",
204220
"checkbox"));
205221
}
206222

@@ -239,6 +255,7 @@ private TagBuilder GenerateRadio(ModelMetadata metadata)
239255
if (Value == null)
240256
{
241257
throw new InvalidOperationException(Resources.FormatInputTagHelper_ValueRequired(
258+
"<input>",
242259
nameof(Value).ToLowerInvariant(),
243260
"type",
244261
"radio"));

src/Microsoft.AspNet.Mvc.TagHelpers/Properties/Resources.Designer.cs

Lines changed: 22 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Microsoft.AspNet.Mvc.TagHelpers/Resources.resx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,27 +118,27 @@
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120120
<data name="AnchorTagHelper_CannotDetermineHrefRouteActionOrControllerSpecified" xml:space="preserve">
121-
<value>Cannot determine an {4} for {0}. An {0} with a specified {1} must not have an {2} or {3} attribute.</value>
121+
<value>Cannot determine an '{4}' for {0}. An {0} with a specified '{1}' must not have an '{2}' or '{3}' attribute.</value>
122122
</data>
123123
<data name="AnchorTagHelper_CannotOverrideSpecifiedHref" xml:space="preserve">
124-
<value>Cannot determine an {8} for {0}. An {0} with a specified {8} must not have attributes starting with {7} or an {1}, {2}, {3}, {4}, {5} or {6} attribute.</value>
124+
<value>Cannot determine an '{8}' for {0}. An {0} with a specified '{8}' must not have attributes starting with '{7}' or an '{1}', '{2}', '{3}', '{4}', '{5}', or '{6}' attribute.</value>
125125
</data>
126126
<data name="InputTagHelper_InvalidExpressionResult" xml:space="preserve">
127-
<value>Unexpected '{1}' result type '{0}'. '{1}' must be of type '{2}' if '{3}' is '{4}'.</value>
127+
<value>Unexpected '{1}' expression result type '{2}' for {0}. '{1}' must be of type '{3}' if '{4}' is '{5}'.</value>
128128
</data>
129129
<data name="InputTagHelper_UnableToFormat" xml:space="preserve">
130-
<value>Unable to format without a '{0}' expression. '{1}' must be null if '{0}' is null.</value>
130+
<value>Unable to format without a '{1}' expression for {0}. '{2}' must be null if '{1}' is null.</value>
131131
</data>
132132
<data name="InputTagHelper_ValueRequired" xml:space="preserve">
133-
<value>'{0}' must not be null if '{1}' is '{2}'.</value>
133+
<value>'{1}' must not be null for {0} if '{2}' is '{3}'.</value>
134134
</data>
135135
<data name="FormTagHelper_CannotDetermineAction" xml:space="preserve">
136-
<value>Cannot determine an {1} for {0}. A {0} with a URL-based {1} must not have attributes starting with {3} or a {2} attribute.</value>
136+
<value>Cannot determine an '{1}' for {0}. A {0} with a URL-based '{1}' must not have attributes starting with '{3}' or a '{2}' attribute.</value>
137137
</data>
138138
<data name="ValidationSummaryTagHelper_InvalidValidationSummaryValue" xml:space="preserve">
139139
<value>Cannot parse '{1}' value '{2}' for {0}. Acceptable values are '{3}', '{4}' and '{5}'.</value>
140140
</data>
141141
<data name="TagHelpers_NoProvidedMetadata" xml:space="preserve">
142-
<value>The {0} was unable to provide metadata for expression '{1}'.</value>
142+
<value>The {2} was unable to provide metadata about '{1}' expression value '{3}' for {0}.</value>
143143
</data>
144144
</root>

src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ public static void MergeAttributes(this TagHelperOutput tagHelperOutput, TagBuil
8989
{
9090
foreach (var attribute in tagBuilder.Attributes)
9191
{
92-
if (!tagHelperOutput.Attributes.ContainsKey(attribute.Key))
92+
// TODO: Use Attributes.ContainsKey once aspnet/Razor#186 is fixed.
93+
if (!tagHelperOutput.Attributes.Any(
94+
item => string.Equals(attribute.Key, item.Key, StringComparison.OrdinalIgnoreCase)))
9395
{
9496
tagHelperOutput.Attributes.Add(attribute.Key, attribute.Value);
9597
}

test/Microsoft.AspNet.Mvc.TagHelpers.Test/AnchorTagHelperTest.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,9 @@ public async Task ProcessAsync_ThrowsIfHrefConflictsWithBoundAttributes(string p
176176
typeof(AnchorTagHelper).GetProperty(propertyName).SetValue(anchorTagHelper, "Home");
177177
}
178178

179-
var expectedErrorMessage = "Cannot determine an href for <a>. An <a> with a specified href must not " +
180-
"have attributes starting with route- or an action, controller, route, " +
181-
"protocol, host or fragment attribute.";
179+
var expectedErrorMessage = "Cannot determine an 'href' for <a>. An <a> with a specified 'href' must not " +
180+
"have attributes starting with 'route-' or an 'action', 'controller', " +
181+
"'route', 'protocol', 'host', or 'fragment' attribute.";
182182

183183
// Act & Assert
184184
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
@@ -202,8 +202,8 @@ public async Task ProcessAsync_ThrowsIfRouteAndActionOrControllerProvided(string
202202
"a",
203203
attributes: new Dictionary<string, string>(),
204204
content: string.Empty);
205-
var expectedErrorMessage = "Cannot determine an href for <a>. An <a> with a " +
206-
"specified route must not have an action or controller attribute.";
205+
var expectedErrorMessage = "Cannot determine an 'href' for <a>. An <a> with a " +
206+
"specified 'route' must not have an 'action' or 'controller' attribute.";
207207

208208
// Act & Assert
209209
var ex = await Assert.ThrowsAsync<InvalidOperationException>(

0 commit comments

Comments
 (0)