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

Tag Helpers: Add "asp-" prefix to all custom attributes on Tag Helpers that target traditional HTML elements #1618

Closed
DamianEdwards opened this issue Nov 22, 2014 · 10 comments

Comments

@DamianEdwards
Copy link
Member

We're changing our guidance and convention such that Tag Helpers that target existing HTML elements should prefix any attributes they add or where they change the semantic meaning of an existing HTML attribute. Tag Helpers that effectively create new custom elements (e.g. <cache> #1552) will not be prefixed, neither their tag name or attribute names. We may introduce support for defining an element prefix via the @addtaghelper directive in the future (aspnet/Razor#88).

We'll update the following Tag Helpers:

  • FormTagHelper
  • InputTagHelper
  • LabelTagHelper
  • SelectTagHelper
  • ScriptTagHelper
  • AnchorTagHelper

Example

public class FormTagHelper : TagHelper {
    [HtmlAttributeName("asp-controller")]
    public string Controller { get; set; }

    [HtmlAttributeName("asp-action")]
    public string Action { get; set; }    
}

Example Before Change

<form controller="Account" action="Login" route-returnurl="@ViewBag.ReturnUrl" method="post" class="form-horizontal" role="form">
    <h4>Use a local account to log in.</h4>
    <hr />
    <div validation-summary="ModelOnly" class="text-danger"></div>
    <div class="form-group">
        <label for="UserName" class="col-md-2 control-label"></label>
        <div class="col-md-10">
            <input for="UserName" class="form-control" />
            <span validation-for="UserName" class="text-danger"></span>
        </div>
    </div>
</form>

Example After Change

<form asp-controller="Account" asp-action="Login" asp-route-returnurl="@ViewBag.ReturnUrl" method="post" class="form-horizontal" role="form">
    <h4>Use a local account to log in.</h4>
    <hr />
    <div asp-validation-summary="ModelOnly" class="text-danger"></div>
    <div class="form-group">
        <label asp-for="UserName" class="col-md-2 control-label"></label>
        <div class="col-md-10">
            <input asp-for="UserName" class="form-control" />
            <span asp-validation-for="UserName" class="text-danger"></span>
        </div>
    </div>
</form>
@DamianEdwards DamianEdwards added this to the 6.0.0-rc1 milestone Nov 22, 2014
@StuartQ
Copy link

StuartQ commented Nov 24, 2014

What's the rationale for not prefixing helpers that create new custom elements? Isn't there still ambiguity on whether a given piece of markup is server-side or client-side?

@mbeckenbach
Copy link

Good idea. Especially angular devs wont like it without a prefix.

@mbeckenbach
Copy link

Maybe make the prefixes configurable by naming convention like angular directives do. asp-whatever would be written AspWhatever in angular. This makes it more flexible for component vendors.

@DamianEdwards
Copy link
Member Author

@StuartQ honestly, because that's my personal preference right now. Like anything, that could still change. We're still considering adding support for specifying a prefix when you import Tag Helpers (aspnet/Razor#88) which would let you add your own prefix to custom elements too (although right now it would also mean you'd have to prefix the input tag too). Configurable prefixing is a lot more complicated than it first appears and I'm hesitant to start prefixing everything as everything we add makes the markup look that little bit less elegant to my eyes.

@mbeckenbach the naming convention of HTML element/tag names is already being tracked at aspnet/Razor#240

@SharpNoiZy
Copy link

Please please don't prefix the tag names!!
My intention is to write valid and clean HTML with parts (attributes) that are evaluated by ASP.
So, to prefix the attributes is the right way!

But I personally would use asp:attribute="" syntax, because it shows a little bit better that this attribute is evaluated by ASP, and that it's is not maybe an self invented "HTML 5" attribute that I want to evaluate by some jquery selector magic "Find all elements with an attribute which begins with asp-".

I hope you know what I mean.

And for own namespaces I go for this solution:

// configure per page or globally
@prefix y Custom.Namespace
<a class="link" asp:controller="foo" asp:action="bar" y:class="baz">...</a>

This is extremely sexy!
This are my 2 cents

@marinasundstrom
Copy link

Using prefixes is a good standard.

But I think that they should be customizable and optional if someone would like that. That is my preference.

What guidelines should be used is up to the community to decide.

@sheryever
Copy link

@DamianEdwards

May I name write my attribute like

    [HtmlAttributeName("@controller")] 

so i would use it as

    <form @controller="Home">   
    </form>

I want to differentiate among Razor, Html and Angular attributes. because while reading other developers code it will creates confusion for me. Last time my developer wrote the same attribute data-sum-field and utilize the same attribute for both javascirpt (jquery) and angular, the only way to recognize was to check the angular directive as injected or not.

One more question, If I have two third party TagHelper for anchor tag and both take "action" attribute one for href and second for theme, what will happened?
1, Priority
2, Ambiguity Error
or something else?

Sometime open source have the same names.

@yishaigalatzer yishaigalatzer modified the milestones: 6.0.0-beta2, 6.0.0-rc1 Nov 26, 2014
@yishaigalatzer
Copy link
Contributor

@sheryever you won't be able to use @controller as a name reliably.

All this is going to do is interfere with the Razor parser, that takes @ as a transition.

It is something we spent a lot of time thinking about, and realized it is not something reasonably achievable at least not for RTM.

dougbu added a commit that referenced this issue Dec 1, 2014
- update XML docs to reflect new HTML / custom attribute separation
- update `Exception` messages to use new attribute names
- update MVC tag helper sample to use new custom attribute names
- add missing `<input/>` tag helper `throw`s test

nits:
- reword a few comments and messages for clarity and consistency
- use `<exception/>` sections to describe what's thrown
- add "Reviewers" comments about current throws
 - note `<a/>` and `<form/>` are slightly inconsistent with others: `throw`
   if unable to override specified `href` or `action` attributes; rest
   `throw` only if custom attributes are inconsistent e.g. `<input/>` with
   `asp-format` but no `asp-for`
- create test tag helpers after all property values are available
dougbu added a commit that referenced this issue Dec 5, 2014
- update XML docs to reflect new HTML / custom attribute separation
- update `Exception` messages to use new attribute names
- update MVC tag helper sample to use new custom attribute names
- add missing `<input/>` tag helper `throw`s test

nits:
- reword a few comments and messages for clarity and consistency
- use `<exception/>` sections to describe what's thrown
- add "Reviewers" comments about current throws
 - note `<a/>` and `<form/>` are slightly inconsistent with others: `throw`
   if unable to override specified `href` or `action` attributes; rest
   `throw` only if custom attributes are inconsistent e.g. `<input/>` with
   `asp-format` but no `asp-for`
- create test tag helpers after all property values are available
@dougbu dougbu closed this as completed in 27beca7 Dec 5, 2014
@dougbu
Copy link
Member

dougbu commented Dec 5, 2014

commit 27beca7

@sheryever
Copy link

@yishaigalatzer, OKh, No problem, I will take Tag Helper as Html Tag Helper not Razor MVC Tag Helper at least till RTM.

Well, overall the work is really great 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants