Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage of .Text() in custom builder always gets overridden #99

Closed
mycroes opened this issue Aug 12, 2018 · 4 comments
Closed

Usage of .Text() in custom builder always gets overridden #99

mycroes opened this issue Aug 12, 2018 · 4 comments

Comments

@mycroes
Copy link

mycroes commented Aug 12, 2018

This is related to #95 (and a regression since HtmlTags 6.x). When creating a custom builder as following:

public class DateTimeBuilder : IElementBuilder
{
    public static string Token = "date-time-builder";

    public HtmlTag Build(ElementRequest request)
    {
        return new HtmlTag("span").Text(Token);
    }
}

the test below fails:

[Fact]
public void ShouldAllowOverridingOfConventionTextUsingCustomBuilder()
{
    var offset = new DateTimeOffset(2018, 1, 1, 12, 00, 00, TimeSpan.FromHours(-6));
    var subject = new Subject {DateValue = offset};
    var helper = GetHtmlHelper(subject);

    var display = helper.Display(m => m.DateValue);

    display.Text().ShouldBe(DateTimeBuilder.Token);
}

I'd love to know if you intend to fix this or if you'll accept this as expected behavior (I guess a modifier can still be used to override the text).

@jbogard
Copy link
Contributor

jbogard commented Aug 13, 2018

Do you have a larger example? What does your registration code look like?

@mycroes
Copy link
Author

mycroes commented Aug 13, 2018

Provided as pull request. I guess the test can't be more to the point (and for this issue I don't see any use for additional configuration), but the code is part of mycroes/SupportManager (...Conventions.cs).

@mycroes
Copy link
Author

mycroes commented Aug 13, 2018

Well actually, the issue is part of SupportManager, where I use a builder to properly format DateTime(Offset)s, the code is just a contrived test.

@jbogard
Copy link
Contributor

jbogard commented Dec 31, 2020

Finally coming back to this. This is because the ModelMetadata extensions (that make these behave like ASP.NET Core) are modifiers, not builders. Builders go first, then modifiers. You've replaced the builder, not the modifier.

Adding your modifier makes this work:

reg.Displays.IfPropertyIs<DateTimeOffset>().ModifyWith(m => m.CurrentTag.Text("date-time-builder"))

@jbogard jbogard closed this as completed Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants