aspnet / Mvc

[Archived] ASP.NET Core MVC is a model view controller framework for building dynamic web sites with clean separation of concerns, including the merged MVC, Web API, and Web Pages w/ Razor. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
5.62k stars 2.14k forks source link

Multiple view folders, ViewImports.cshtml & Customer TagHelper duplicates html elements. #8708

Closed Steveiwonder closed 5 years ago

Steveiwonder commented 5 years ago

Is this a Bug or Feature request?: Bug (I think)

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

https://github.com/Steveiwonder/MvcTagHelperBugExample/tree/master/WebApplication11

Description of the problem:

I'm not sure how to explain this without over complicating it but I'll try. Everything i talk about can be seen in the example repository linked above.

The problem: Duplicate html elements generated when using asp-for and custom tag helper.

If you run the example project and inspect the html generated it will look like this:

<form>
    <input type="checkbox" data-val="true" data-val-required="The ACheckbox field is required." id="ACheckbox" name="ACheckbox" value="true">

    <select data-val="true" data-val-required="The SelectedPerson field is required." id="SelectedPerson" name="SelectedPerson">
       <option value="0" selected="selected">Select person...</option>
       <option value="1">Bob</option>
       <option value="1">Bob</option>
    </select>
    <input name="ACheckbox" type="hidden" value="false">
    <input name="ACheckbox" type="hidden" value="false">
</form>

when it should look like

<form>
    <input type="checkbox" data-val="true" data-val-required="The ACheckbox field is required." id="ACheckbox" name="ACheckbox" value="true">

    <select data-val="true" data-val-required="The SelectedPerson field is required." id="SelectedPerson" name="SelectedPerson">
        <option value="0" selected="selected">Select person...</option>
        <option value="1">Bob</option>
    </select>
    <input name="ACheckbox" type="hidden" value="false">
</form>

You can see in the first code snippet that my list items have been duplicated, and there are two hidden input elements with the same name.

This comes when I to have a _ViewImports.cshtml inside my features folder (which is require to get the asp-for attribute working + intellisense) & the fact that I have a MyInputTagHelper and MySelectTagHelper both inheriting from either InputTagHelper or SelectTagHelper they seem to not handle multiple _ViewImport.cshtml files between directories.

I don't know if this is a bug or it's the way I've configured it. If it's not a bug how do i separate views out into different folders like the above example

mkArtakMSFT commented 5 years ago

Thanks for contacting us, @Steveiwonder. @NTaylorMullen, can you please look into this? Thanks!

pranavkm commented 5 years ago

My wild guess is that both built-in InputTagHelper and MyInputTagHelper are being applied. You'd have to remove or not register (via @addTagHelper) the built in ones.

Steveiwonder commented 5 years ago

@pranavkm - I agree, however I would hope it would work without having to manually de-duplicate.

NTaylorMullen commented 5 years ago

@pranavkm is correct as in it's your duplication of the InputTagHelper that's causing the redundant hidden input.

If you're trying to replace the default InputTagHelper behavior the recommendation is to @removeTagHelper Microsoft.AspNetCore.Mvc.TagHelpers.InputTagHelper, Microsoft.AspNetCore.Mvc.TagHelpers.

Lastly, it's unfortunate that we currently aren't smart enough to de-duplicate those bits ourselves; i'll leave it to @mkArtakMSFT and folks as to if we want to invest the time to change that behavior.

poke commented 5 years ago

Lastly, it's unfortunate that we currently aren't smart enough to de-duplicate those bits ourselves

I’m not sure how that would be expected to work though. Just going by inheritance is not really enough to turn off the base class tag helper. I am sure there could be use cases where keeping a base tag helper and an inherited one active at the same time. So the inheriting tag helper would need a way to mark itself as a replacement for the base tag helper. And at that time, simply unregistering it is probably clearer.

NTaylorMullen commented 5 years ago

Sorry @poke, doing something to the extent of unregistering what we think are duplicate TagHelpers would be a breaking change we wouldn't be able to make.

As for making the InputTagHelper understand if it's already run on an element. Are you confident that every application of running it twice on a TagHelper is invalid?

poke commented 5 years ago

@NTaylorMullen You are misunderstanding me. I was saying that such a “smart detection” wouldn’t work that easily since there is no clear criteria on when one tag helper is supposed to replace another one. So you would have to declare that explicitly in some way; and at that point manually unregistering a specific tag helper because you want a different one to run instead would be the clearer and easier solution.

And yeah, as I said, I am sure that there are valid use cases where you want to run the original tag helper and its supposed-to replacement at the same time. So a smart detection simply wouldn’t work.

(Basically, I was agreeing with you, but dimissing the idea that we might need to look into this in the future)

NTaylorMullen commented 5 years ago

Thanks for contacting us. We believe that the question you've raised has been answered. If you still feel a need to continue the discussion, feel free to reopen it and add your comments.