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

Named Tag Helpers? #3233

Closed
RehanSaeed opened this issue Sep 30, 2015 · 16 comments
Closed

Named Tag Helpers? #3233

RehanSaeed opened this issue Sep 30, 2015 · 16 comments
Assignees

Comments

@RehanSaeed
Copy link

I have built two-dozen HTML helpers I am converting to tag helpers for ASP.NET 5 MVC 6 to generate Facebook Open Graph meta tags. Here is an example of the 'website' type:

Index.cshtml

<open-graph-website title="Page Title" 
                    main-image="new OpenGraphImage(
                        "~/img/open-graph-1200x630.png", 
                        "image/png", 
                        1200, 
                        630)"
                    determiner="Blank"
                    site-name="Site Title">

Each Open Graph object type has a special namespace which has to be added to the head tag e.g.

_Layout.cshtml

<html>
<head prefix="website: http://ogp.me/ns/website#">
</head>
<body>
    <!-- ...Omitted -->
</body>
</html>

I managed to do this using HTML Helpers in the past by using the ViewBag and the Namespace getter property on the OpenGraphWebsite class like so:

Index.cshtml

ViewBag.OpenGraph = new OpenGraphWebsite(
    "Page Title",
    new OpenGraphImage("/img/open-graph-1200x630.png", "image/png", 1200, 630)
    {
        Determiner = OpenGraphDeterminer.Blank,
        SiteName = "Site Title"
    };

_Layout.cshtml

<html>
<head @(ViewBag.OpenGraph == null ? null : ViewBag.OpenGraph.Namespace)>
    @Html.OpenGraph((OpenGraphMetadata)ViewBag.OpenGraph);
</head>
<body>
    <!-- ...Omitted -->
</body>
</html>

Is there a way to achieve a similar result with tag helpers? Some way of naming a tag helper or referring to it?

@dougbu
Copy link
Member

dougbu commented Sep 30, 2015

@RehanSaeed I suggest creating a TagHelper that targets the <head/> element when it has a prefix attribute. Then use TagHelperContent.PreContent to add what you need before whatever else happens to be inside the <head/> element.

If you need to split things between two tag helpers, use TagHelperContext.Items to communicate between them. Best key is usually the type of the parent tag helper.

@Eilon
Copy link
Member

Eilon commented Sep 30, 2015

@dougbu I believe the scenario here is that he needs to add the prefix to the <head> element automatically. So the tag helper would need to target a plain <head> tag, no attributes. But the rest is probably the same.

@Eilon Eilon added the question label Sep 30, 2015
@RehanSaeed
Copy link
Author

I was not aware of the TagHelperContext.Items property. It seems like a very good idea and perfect for my needs. So something like this:

[TargetElement("open-graph-website", Attributes = nameof(Title) + "," + nameof(MainImage), TagStructure = TagStructure.WithoutEndTag)]
public class OpenGraphWebsiteTagHelper : OpenGraphMetadataTagHelper // Inherits from TagHelper
{
    // ... Omitted
    public override void Process(TagHelperContext context, TagHelperOutput output)
    {
        context.Items.Add(typeof(OpenGraphMetadata), this.Namespaces);
        output.Content.SetContent(this.ToString());
    }
}

[TargetElement("head")]
public class OpenGraphNamespacePrefixTagHelper : TagHelper
{
    private const string OpenGraphPrefixAttributeName = "asp-open-graph-prefix";

    [HtmlAttributeName(OpenGraphPrefixAttributeName)]
    public bool DUMMY { get; set; }

    public override int Order { get; } = 1;

    public override void Process(TagHelperContext context, TagHelperOutput output)
    {
        if (context.Items.Contains(typeof(OpenGraphMetadata))
        {
            string namespaces = context.Items[typeof(OpenGraphMetadata)];
            output.Attributes.Add("prefix", namespaces);
        }
    }
}

I don't actually need any properties on OpenGraphNamespacePrefixTagHelper. What is the convention in this case? I've just added a dummy property but I don't care that it has a value, I would just be using it to apply the tag helper to the head element.

@dougbu
Copy link
Member

dougbu commented Sep 30, 2015

Looks like it should work with one caveat: The OpenGraphNamespacePrefixTagHelper helper needs to call TagHelperContext.GetChildContentAsync() before checking context.Items.Contains(). Otherwise the OpenGraphWebsiteTagHelper will not yet have executed. (Order only controls the execution order of tag helpers targeting the same element.)

Likely easiest to use await and override ProcessAsync() in OpenGraphNamespacePrefixTagHelper.

Separately,

I don't actually need any properties on OpenGraphNamespacePrefixTagHelper.

It's fine to define a tag helper without properties.

@RehanSaeed
Copy link
Author

So in conclusion:

[TargetElement("head")]
public class OpenGraphNamespacePrefixTagHelper : TagHelper
{
    private const string PrefixAttributeName = "prefix";

    public override Task ProcessAsync(TagHelperContext context, TagHelperOutput output)
    {
        await context.GetChildContentAsync();

        if (context.Items.Contains(typeof(OpenGraphMetadata)))
        {
            string namespaces = context.Items[typeof(OpenGraphMetadata)];
            output.Attributes.Add(PrefixAttributeName, namespaces);
        }
    }
}

Presumably the tag helper is automatically added. But what If I don't use the OpenGraphWebsiteTagHelper on the page, the above code will still get executed anyway, unless I remove @addTagHelper "*, Boilerplate.Web.Mvc6" from _ViewImports.cshtml? Sound like something to be careful about then for best performance.

@dougbu
Copy link
Member

dougbu commented Sep 30, 2015

The code will execute but the Contains() check will exit the ProcessAsync() method without doing more..

I misspoke earlier. The OpenGraphNamespacePrefixTagHelper also needs to add the OpenGraphMetadata entry to the Items collection before calling GetChildContentAsync(). Then check that the lower-level tag helper changed something in that entry. E.g. context.Items[typeof(OpenGraphMetadata)] == null then context.Items[typeof(OpenGraphMetadata)] != null instead of the Contains() check.

@RehanSaeed
Copy link
Author

I can't seem to find any examples in the ASP.NET source using context.Items. This is what I've now done following on from your comments:

[TargetElement("open-graph-website", Attributes = nameof(Title) + "," + nameof(MainImage), TagStructure = TagStructure.WithoutEndTag)]
public class OpenGraphWebsiteTagHelper : OpenGraphMetadataTagHelper // Inherits from TagHelper
{
    // ... Omitted
    public override void Process(TagHelperContext context, TagHelperOutput output)
    {
        context.Items[typeof(OpenGraphMetadata)] = this.GetNamespaces();
        output.Content.SetContent(this.ToString());
    }
}

[TargetElement("head")]
public class OpenGraphNamespacePrefixTagHelper : TagHelper
{
    private const string PrefixAttributeName = "prefix";

    public override Task ProcessAsync(TagHelperContext context, TagHelperOutput output)
    {
        context.Items.Add(typeof(OpenGraphMetadata), null);

        await context.GetChildContentAsync();

        string namespaces = context.Items[typeof(OpenGraphMetadata)] as string;
        if (namespaces != null)
        {
            output.Attributes.Add(PrefixAttributeName, namespaces);
        }
    }
}

I am still going to experiment with adding a dummy required property. This would force users to opt-in to using the tag helper, rather than it running automatically just because they added the tag helper DLL. It would be nice if value-less attributes could be added to tag helpers to facilitate this feature for bool properties:

<head asp-open-graph-prefix>
Rather than:
<head asp-open-graph-prefix="true">

[TargetElement("head", Attributes = OpenGraphPrefixAttributeName)]
public class OpenGraphNamespacePrefixTagHelper : TagHelper
{
    private const string OpenGraphPrefixAttributeName = "asp-open-graph-prefix";

    [HtmlAttributeName(OpenGraphPrefixAttributeName)]
    public bool DUMMY { get; set; }

    // ..Omitted
}

@dougbu
Copy link
Member

dougbu commented Oct 1, 2015

@RehanSaeed are you running into any issues with this approach? If not it's probably time to close this issue.

@RehanSaeed RehanSaeed reopened this Oct 4, 2015
@RehanSaeed
Copy link
Author

@dougbu I've finally managed to find time to test this out and it doesn't seem to work.

The OpenGraphWebsiteTagHelper always runs first and adds the namespaces to the Items dictionary. The OpenGraphNamespacePrefixTagHelper runs second but the Items dictionary is always empty before and after the call to GetChildContentAsync.

I can find no examples of how to use the Items dictionary, do you know of any?

@dougbu
Copy link
Member

dougbu commented Oct 5, 2015

Is the <open-graph-website/> an immediate child of the <head/> element?
Any other tag helpers used in this application?
Do you have a repo project, preferably something hosted on GitHub?

@RehanSaeed
Copy link
Author

Yes but using a RenderSection call, as I'm using a _Layout.cshtml file. You can take a look at the full code here: OpenGraphPrefixTagHelper, OpenGraphMetadata and OpenGraphWebsite. I don't have a usage example checked in because it does not work but it looks like this:

Index.cshtml

@section head
{
    <twitter-card-summary username="@@RehanSaeedUK">
    <open-graph-website title="Title"
                        main-image="@(new OpenGraphImage(Url.AbsoluteContent("~/img/icons/open-graph-1200x630.png"), ContentType.Png, 1200, 630))"
                        determiner="OpenGraphDeterminer.Blank"
                        site-name="Site Title">
}

_Layout.cshtml

<!DOCTYPE html>
<html>
<head asp-open-graph-prefix="true">
@RenderSection("head", required: false)
</head>
<body>
</body>
</html>

@dougbu
Copy link
Member

dougbu commented Oct 6, 2015

@RehanSaeed individual .cshtml files are mostly isolated from each other in MVC and Razor. Though the OpenGraphPrefixTagHelper starts execution first and its GetChildContentAsync() call runs the OpenGraphWebsite tag helper, they get distinct TagHelperExecutionContent instances and unrelated Items collections. Perhaps that is a design issue on our side -- I'll open a separate question on the point. But the scenario doesn't require a framework change.

You have at least three options with the current framework:

  1. Move your two tag helpers into the same file -- Index.cshtml, _Layout.cshtml or something else.
  2. Use ViewContext.ViewData to communicate between the tag helpers e.g.
[HtmlAttributeNotBound]
[ViewContext]
public ViewContext ViewContext { get; set; }
...
   ViewContext.ViewData[nameof(OpenGraphPrefixTagHelper))] = ...;
  1. Register a scoped service (meaning the instance is specific to execution of a single request), register it in your Startup class, and use that service to communicate between the tag helpers. A number of MVC functional tests demonstrate scoped service use e.g. https://github.com/aspnet/Mvc/blob/dev/test/WebSites/RequestServicesWebSite/Startup.cs

@dougbu
Copy link
Member

dougbu commented Oct 6, 2015

Filed aspnet/Razor#564 about the potential design point. I don't expect changes in this area before v1 of MVC 6 hits RTM.

@dougbu dougbu self-assigned this Oct 6, 2015
@dougbu dougbu closed this as completed Oct 6, 2015
@RehanSaeed
Copy link
Author

Thanks @dougbu. I think option two is the simplest workaround so that's the approach I'll go with. Can that ViewContext property be made internal? My TagHelper doubles as a HTML helper so it would be useful to cut out the noise.

@dougbu
Copy link
Member

dougbu commented Oct 10, 2015

Can that ViewContext property be made internal?

No, tag helper activation ignores properties that are static or not public. This is consistent across MVC e.g. the framework handles ViewComponentContext properties similarly when activating view components. IHtmlHelper instances are different because they are not activated and are instead used when activating other classes e.g. a generated RazorPage subclass.

When used as an HTML helper, how does your class get the current ViewContext?

@RehanSaeed
Copy link
Author

Ok, no problem.

No need to use ViewContext. The ViewBag is available in the view itself, so you can do something like this:

@{
    ViewBag.OpenGraph = new OpenGraphWebsite(...);
}

@Html.OpenGraph((OpenGraphMetadata)ViewBag.OpenGraph)

@Html.OpenGraphNamespaces((OpenGraphMetadata)ViewBag.OpenGraph)

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

4 participants