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

Commit 0a09f0b

Browse files
author
NTaylorMullen
committed
Addressed code review comments.
1 parent 19ca5ea commit 0a09f0b

File tree

4 files changed

+42
-40
lines changed

4 files changed

+42
-40
lines changed

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

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,25 @@ public class AnchorTagHelper : TagHelper
5757
public string Route { get; set; }
5858

5959
/// <inheritdoc />
60-
/// <remarks>Does nothing if user provides an "href" attribute. Cannot specify an "href" attribute AND
61-
/// <see cref="Action"/>, <see cref="Controller"/> or <see cref="Route"/>.</remarks>
60+
/// <remarks>Does nothing if user provides an "href" attribute. Providing an "href" attribute and
61+
/// <see cref="Action"/>, <see cref="Controller"/> or <see cref="Route"/> throws an
62+
/// <see cref="InvalidOperationException"/>.</remarks>
6263
public override void Process(TagHelperContext context, TagHelperOutput output)
6364
{
64-
var routeValues = GetRouteValues(output);
65+
var routePrefixedAttributes = output.FindPrefixedAttributes(RouteAttributePrefix);
6566

6667
// If there's an "href" on the tag it means it's being used as a normal anchor.
6768
if (output.Attributes.ContainsKey(Href))
6869
{
69-
if (Action != null || Controller != null || Route != null || routeValues != null)
70+
if (Action != null || Controller != null || Route != null || routePrefixedAttributes.Any())
7071
{
7172
// User specified an href AND a Action, Controller or Route; can't determine the href attribute.
7273
throw new InvalidOperationException(
73-
Resources.FormatAnchorTagHelper_CannotDetermineHrefOneSpecified(
74+
Resources.FormatAnchorTagHelper_CannotOverrideSpecifiedHref(
7475
"<a>",
75-
nameof(Action),
76-
nameof(Controller),
77-
nameof(Route),
76+
nameof(Action).ToLowerInvariant(),
77+
nameof(Controller).ToLowerInvariant(),
78+
nameof(Route).ToLowerInvariant(),
7879
RouteAttributePrefix,
7980
Href));
8081
}
@@ -85,6 +86,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
8586
else
8687
{
8788
TagBuilder tagBuilder;
89+
var routeValues = GetRouteValues(output, routePrefixedAttributes);
8890

8991
if (Route == null)
9092
{
@@ -103,9 +105,9 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
103105
throw new InvalidOperationException(
104106
Resources.FormatAnchorTagHelper_CannotDetermineHrefRouteActionOrControllerSpecified(
105107
"<a>",
106-
nameof(Route),
107-
nameof(Action),
108-
nameof(Controller),
108+
nameof(Route).ToLowerInvariant(),
109+
nameof(Action).ToLowerInvariant(),
110+
nameof(Controller).ToLowerInvariant(),
109111
Href));
110112
}
111113
else
@@ -126,41 +128,41 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
126128
}
127129
}
128130

129-
private static Dictionary<string, object> GetRouteValues(TagHelperOutput output)
131+
// TODO: We will not need this method once https://github.com/aspnet/Razor/issues/89 is completed.
132+
private static Dictionary<string, object> GetRouteValues(
133+
TagHelperOutput output, IEnumerable<KeyValuePair<string, string>> routePrefixedAttributes)
130134
{
131-
var prefixedValues = output.PullPrefixedAttributes(RouteAttributePrefix);
132-
133135
Dictionary<string, object> routeValues = null;
134-
135-
if (prefixedValues.Any())
136+
if (routePrefixedAttributes.Any())
136137
{
137-
// Generator.GenerateActionLink || GenerateRouteLink does not accept a Dictionary<string, string> for
138-
// route values.
139-
routeValues = prefixedValues.ToDictionary(
138+
// Prefixed values should be treated as bound attributes, remove them from the output.
139+
output.RemoveRange(routePrefixedAttributes);
140+
141+
// Generator.GenerateForm does not accept a Dictionary<string, string> for route values.
142+
routeValues = routePrefixedAttributes.ToDictionary(
140143
attribute => attribute.Key.Substring(RouteAttributePrefix.Length),
141144
attribute => (object)attribute.Value);
142145
}
143146

144147
return routeValues;
145148
}
146149

147-
148150
// Restores bound HTML attributes when we detect that a user is using the AnchorTagHelper as a normal <a> tag.
149151
private void RestoreBoundHtmlAttributes(TagHelperContext context, TagHelperOutput output)
150152
{
151153
if (Protocol != null)
152154
{
153-
output.RestoreBoundHtmlAttribute(nameof(Protocol), context);
155+
output.CopyHtmlAttribute(nameof(Protocol), context);
154156
}
155157

156158
if (Host != null)
157159
{
158-
output.RestoreBoundHtmlAttribute(nameof(Host), context);
160+
output.CopyHtmlAttribute(nameof(Host), context);
159161
}
160162

161163
if (Fragment != null)
162164
{
163-
output.RestoreBoundHtmlAttribute(nameof(Fragment), context);
165+
output.CopyHtmlAttribute(nameof(Fragment), context);
164166
}
165167
}
166168
}

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

Lines changed: 12 additions & 12 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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,12 @@
117117
<resheader name="writer">
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120-
<data name="AnchorTagHelper_CannotDetermineHrefOneSpecified" xml:space="preserve">
121-
<value>Cannot determine an {5} for {0}. An {0} with a specified {5} must not have attributes starting with {4} or a {1}, {2} or {3} attribute.</value>
122-
</data>
123120
<data name="AnchorTagHelper_CannotDetermineHrefRouteActionOrControllerSpecified" xml:space="preserve">
124121
<value>Cannot determine an {4} for {0}. An {0} with a specified {1} must not have a {2} or {3} attribute.</value>
125122
</data>
123+
<data name="AnchorTagHelper_CannotOverrideSpecifiedHref" xml:space="preserve">
124+
<value>Cannot determine an {5} for {0}. An {0} with a specified {5} must not have attributes starting with {4} or a {1}, {2} or {3} attribute.</value>
125+
</data>
126126
<data name="FormTagHelper_CannotDetermineAction" xml:space="preserve">
127127
<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>
128128
</data>

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public async Task ProcessAsync_ThrowsIfHrefConflictsWithBoundAttributes(string p
217217
}
218218

219219
var expectedErrorMessage = "Cannot determine an href for <a>. An <a> with a specified href must not " +
220-
"have attributes starting with route- or a Action, Controller or Route " +
220+
"have attributes starting with route- or a action, controller or route " +
221221
"attribute.";
222222

223223
// Act & Assert
@@ -246,7 +246,7 @@ public async Task ProcessAsync_ThrowsIfRouteAndActionOrControllerProvided(string
246246
attributes: new Dictionary<string, string>(),
247247
content: string.Empty);
248248
var expectedErrorMessage = "Cannot determine an href for <a>. An <a> with a " +
249-
"specified Route must not have a Action or Controller attribute.";
249+
"specified route must not have a action or controller attribute.";
250250

251251
// Act & Assert
252252
var ex = await Assert.ThrowsAsync<InvalidOperationException>(

0 commit comments

Comments
 (0)