Add AnchorTagHelper. #1334

NTaylorMullen · 2014-10-11T06:13:44Z

Added a TagHelper that targets the tag and allows users to do the equivalent of Html.ActionLink or Html.RouteLink.
Tag Helpers: <a/> tag helper #1247

yishaigalatzer · 2014-10-11T17:51:54Z

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

+                if (Action != null || Controller != null || Route != null)
+                {
+                    // User specified an href AND a Action, Controller or Route; can't determine the href attribute.
+                    throw new InvalidOperationException(


I think we shouldn't throw here.
example - The a tag is populated by javascript based on the id tag.
so instead do nothing?

CC @DamianEdwards

+1 this is another case where, like "controller" and "route-value" in the <form/> helper, we should be consistent. I agree we shouldn't second-guess the users's intent when they force the tag helper to no-op.

Talked in person.

NTaylorMullen · 2014-10-12T02:58:59Z

Addressed comments that didn't have a pending question.

Eilon · 2014-10-12T04:56:15Z

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

@@ -117,6 +117,12 @@
  <resheader name="writer">
    <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
  </resheader>
+  <data name="AnchorTagHelper_CannotDetermineHrefOneSpecified" xml:space="preserve">
+    <value>Cannot determine an href for {0}. {0}s with a specified href must not have a {1}, {2} or {3} attribute.</value>


I know it's just a design PR, but: recommend passing in href as a parameter to the string. And also recommend not pluralizing the {0}s in the second sentence, and instead saying A {0} with a specified...

NTaylorMullen · 2014-10-12T20:13:32Z

Addressed comments.

NTaylorMullen · 2014-10-13T06:29:08Z

Added tests.

dougbu · 2014-10-13T21:33:59Z

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

+        /// <summary>
+        /// The name of the action method.
+        /// </summary>
+        /// <remarks>Cannot be provided if <see cref="Route"/> is specified.</remarks>


nit: reword these "cannot" <remarks/> to avoid passive voice: Must be <c>null</c> if <see cref="Route"/> is non-<c>null</c>.

(hmm, updated comment is slightly passive but at least is in terms of the properties here and not some random "specifier".)

dougbu · 2014-10-13T21:50:12Z

⌚ and if I missed some form-related comments, assume they apply here as well

NTaylorMullen · 2014-10-14T05:33:12Z

Addressed code review comments & tests.

dougbu · 2014-10-15T04:30:20Z

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

+        public string Route { get; set; }
+
+        /// <inheritdoc />
+        /// <remarks>Does nothing if user provides an "href" attribute. Cannot specify an "href" attribute AND 


NTaylorMullen · 2014-10-15T07:04:15Z

Addressed code review comments.

dougbu · 2014-10-15T22:21:28Z

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

+        public string Protocol { get; set; }
+
+        /// <summary>
+        /// The host name for the URL.


teensy nit: leave off "for the URL" to be consistent w/ the other properties

dougbu · 2014-10-15T22:34:53Z

⌚ for the restore some / throw on others follow-up

NTaylorMullen · 2014-10-15T23:47:31Z

Addressed code review comments.

dougbu · 2014-10-16T00:07:34Z

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

+                    Fragment != null || 
+                    routePrefixedAttributes.Any())
+                {
+                    // User specified an href AND a Action, Controller or Route; can't determine the href attribute.


nits:

don't yell

remove list of attributes (it's incomplete): just "one of the bound attributes"

dougbu · 2014-10-16T00:15:57Z

after fixing Exception message arguments

- Added a TagHelper that targets the <a> tag and allows users to do the equivalent of Html.ActionLink or Html.RouteLink. - Added tests to validate AnchorTagHelper functionality. #1247

NTaylorMullen changed the title ~~Add AnchorTagHelper.~~ [Design] Add AnchorTagHelper. Oct 11, 2014

yishaigalatzer reviewed Oct 11, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_Form branch from 8f2cdfb to 8c441bf Compare October 12, 2014 02:57

NTaylorMullen force-pushed the TagHelpers_Anchor branch from ee5f9ca to 9626739 Compare October 12, 2014 02:58

Eilon reviewed Oct 12, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 9626739 to 761faf8 Compare October 12, 2014 20:11

NTaylorMullen force-pushed the TagHelpers_Form branch from c908f03 to 14cf681 Compare October 13, 2014 06:26

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 761faf8 to 7d0d910 Compare October 13, 2014 06:26

NTaylorMullen changed the title ~~[Design] Add AnchorTagHelper.~~ Add AnchorTagHelper. Oct 13, 2014

NTaylorMullen force-pushed the TagHelpers_Form branch from 14cf681 to 07f8dda Compare October 13, 2014 17:52

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 7d0d910 to 917d849 Compare October 13, 2014 17:52

dougbu reviewed Oct 13, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_Form branch from 07f8dda to 8b55118 Compare October 14, 2014 00:43

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 917d849 to af663a3 Compare October 14, 2014 00:48

NTaylorMullen force-pushed the TagHelpers_Form branch from 8b55118 to c0b944d Compare October 14, 2014 05:31

NTaylorMullen force-pushed the TagHelpers_Anchor branch from af663a3 to 88eb37e Compare October 14, 2014 05:32

NTaylorMullen force-pushed the TagHelpers_Form branch from c0b944d to ffdae80 Compare October 15, 2014 00:30

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 88eb37e to be195a9 Compare October 15, 2014 00:30

dougbu reviewed Oct 15, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_Form branch from ffdae80 to 4d221a1 Compare October 15, 2014 07:02

NTaylorMullen force-pushed the TagHelpers_Anchor branch from be195a9 to 0a09f0b Compare October 15, 2014 07:02

NTaylorMullen force-pushed the TagHelpers_Form branch from 4d221a1 to b275566 Compare October 15, 2014 18:20

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 0a09f0b to a73f0a5 Compare October 15, 2014 18:20

dougbu reviewed Oct 15, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_Form branch from b275566 to 011447d Compare October 15, 2014 23:45

NTaylorMullen force-pushed the TagHelpers_Anchor branch from a73f0a5 to 4642fbf Compare October 15, 2014 23:45

dougbu reviewed Oct 16, 2014
View reviewed changes

Add AnchorTagHelper.

e494f83

- Added a TagHelper that targets the <a> tag and allows users to do the equivalent of Html.ActionLink or Html.RouteLink. - Added tests to validate AnchorTagHelper functionality. #1247

NTaylorMullen force-pushed the TagHelpers_Form branch from 011447d to 3e2c256 Compare October 16, 2014 00:34

NTaylorMullen force-pushed the TagHelpers_Anchor branch from 4642fbf to e494f83 Compare October 16, 2014 00:35

NTaylorMullen deleted the TagHelpers_Anchor branch October 16, 2014 00:38

This pull request was closed.

Add AnchorTagHelper. #1334

Add AnchorTagHelper. #1334

Uh oh!

Conversation

NTaylorMullen commented Oct 11, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NTaylorMullen commented Oct 12, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NTaylorMullen commented Oct 12, 2014

Uh oh!

NTaylorMullen commented Oct 13, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougbu commented Oct 13, 2014

Uh oh!

NTaylorMullen commented Oct 14, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NTaylorMullen commented Oct 15, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougbu commented Oct 15, 2014

Uh oh!

NTaylorMullen commented Oct 15, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougbu commented Oct 16, 2014

Uh oh!

Uh oh!