53

This question is a follow-up for Why is my DisplayFor not looping through my IEnumerable<DateTime>?


A quick refresh.

When:

  • the model has a property of type IEnumerable<T>
  • you pass this property to Html.EditorFor() using the overload that only accepts the lambda expression
  • you have an editor template for the type T under Views/Shared/EditorTemplates

then the MVC engine will automatically invoke the editor template for each item in the enumerable sequence, producing a list of the results.

E.g., when there is a model class Order with property Lines:

public class Order
{
    public IEnumerable<OrderLine> Lines { get; set; }
}

public class OrderLine
{
    public string Prop1 { get; set; }
    public int Prop2 { get; set; }
}

And there is a view Views/Shared/EditorTemplates/OrderLine.cshtml:

@model TestEditorFor.Models.OrderLine

@Html.EditorFor(m => m.Prop1)
@Html.EditorFor(m => m.Prop2)

Then, when you invoke @Html.EditorFor(m => m.Lines) from the top-level view, you will get a page with text boxes for each order line, not just one.


However, as you can see in the linked question, this only works when you use that particular overload of EditorFor. If you provide a template name (in order to use a template that is not named after the OrderLine class), then the automatic sequence handling will not happen, and a runtime error will happen instead.

At which point you will have to declare your custom template's model as IEnumebrable<OrderLine> and manually iterate over its items in some way or another to output all of them, e.g.

@foreach (var line in Model.Lines) {
    @Html.EditorFor(m => line)
}

And that is where problems begin.

The HTML controls generated in this way all have same ids and names. When you later POST them, the model binder will not be able to construct an array of OrderLines, and the model object you get in the HttpPost method in the controller will be null.
This makes sense if you look at the lambda expression - it does not really link the object being constructed to a place in the model from which it comes.

I have tried various ways of iterating over the items, and it would seem the only way is to redeclare the template's model as IList<T> and enumerate it with for:

@model IList<OrderLine>

@for (int i = 0; i < Model.Count(); i++)
{ 
    @Html.EditorFor(m => m[i].Prop1)
    @Html.EditorFor(m => m[i].Prop2)
}

Then in the top-level view:

@model TestEditorFor.Models.Order

@using (Html.BeginForm()) {
    @Html.EditorFor(m => m.Lines, "CustomTemplateName")
}

which gives properly named HTML controls that are properly recognized by the model binder on a submit.


While this works, it feels very wrong.

What is the correct, idiomatic way to use a custom editor template with EditorFor, while preserving all the logical links that allow the engine to generate HTML suitable for the model binder?

Community
  • 1
  • 1
GSerg
  • 71,102
  • 17
  • 141
  • 299
  • 4
    This is a good question. I tried to use EditorFor and DisplayFor quite a bit in the beginning, but I ultimately gave up due to many problems like this. When binding to deep object graphs the partials have no contextual information to properly generate Name attributes to support proper model binding on post. – AaronLS Aug 15 '14 at 20:03
  • @AaronLS They do seem to work properly, even when the object is deep - but only when you stay very default in what overloads you use. I often find it not enough, and MVC is at version 5.1 already, so one'd expect these problems to be solved already. – GSerg Aug 15 '14 at 20:10
  • Yeh, probably not related to your problem directly, but to clarify what I mean "In the generated HTML, the input fields for Name and Address are (obviously) no longer properly prefixed so the Model Binder" unless you take special steps to deal with this(or something has changed in the framework since I tried this): http://thatextramile.be/blog/2011/01/prefixing-input-elements-of-partial-views-with-asp-net-mvc/ – AaronLS Aug 25 '14 at 20:36
  • 1
    Quoted from here.. http://stackoverflow.com/questions/43021/how-do-you-get-the-index-of-the-current-iteration-of-a-foreach-loop Ian Mercer posted this solution on Phil Haack's blog. foreach (var item in Model.Select((value,i) => new {i, value})) This gets you the item (item.value) and its index (item.i). http://haacked.com/archive/2011/04/14/a-better-razor-foreach-loop.aspx Hth, O –  Aug 26 '14 at 09:26
  • 1
    @ojf I have seen this post, but it does not help. If you `select(value,i)` with `IEnumerable` as the model, you'd then have to call `EditorFor(m => m.ElementAt(item.i).PropertyName)`, and that creates HTML without indices, just like regular `foreach` would, because `ElementAt` is not recognized by Razor in this way. – GSerg Aug 26 '14 at 13:39
  • @GSerg I tried to reproduce you final solution but It doesn't work for me, In the POST the list of OrderLines is null. – Jaider Aug 27 '14 at 23:18
  • @Jaider I'm using MVC 5.2 if that makes a difference. Do your rendered HTML controls have array indicies in their names? – GSerg Aug 28 '14 at 08:44
  • @GSerg maybe, I was using VS2010 - MVC4. I also couldn't find any useful info that allow me to modify the "convention over configuration", so we easily config something like: in this view, for this `MyDataType` use this template `CustomTemplateName`. – Jaider Aug 28 '14 at 16:26
  • >The HTML controls generated in this way all have same ids and names Is this the real problem you're trying to solve? – Mrchief Aug 31 '14 at 23:31
  • If all you want is for razor to generate elements with proper names and indices, this can be solved a much easier way. – Mrchief Aug 31 '14 at 23:32
  • Why is it necessary to use a custom template name? Do you have multiple editor templates for the same type (`OrderLine`)? If so you can create an `EditorTemplates` folder in the Views folder associated with the Controller (`Views/ControllerName/EditorTemplates`). Razor will search this folder first an use it, even if another `OrderLine.cshtml` file exists in the `Views/Shared/EditorTemplates` folder –  Sep 08 '14 at 10:00
  • @StephenMuecke The two different editors are triggered from two methods of the same controller. – GSerg Sep 08 '14 at 19:22

5 Answers5

34

After discussion with Erik Funkenbusch, which led to looking into the MVC source code, it would appear there are two nicer (correct and idiomatic?) ways to do it.

Both involve providing correct html name prefix to the helper, and generate HTML identical to the output of the default EditorFor.

I'll just leave it here for now, will do more testing to make sure it works in deeply nested scenarios.

For the following examples, suppose you already have two templates for OrderLine class: OrderLine.cshtml and DifferentOrderLine.cshtml.


Method 1 - Using an intermediate template for IEnumerable<T>

Create a helper template, saving it under any name (e.g. "ManyDifferentOrderLines.cshtml"):

@model IEnumerable<OrderLine>

@{
    int i = 0;

    foreach (var line in Model)
    { 
        @Html.EditorFor(m => line, "DifferentOrderLine", "[" + i++ + "]")
    }
}

Then call it from the main Order template:

@model Order

@Html.EditorFor(m => m.Lines, "ManyDifferentOrderLines")

Method 2 - Without an intermediate template for IEnumerable<T>

In the main Order template:

@model Order

@{
    int i = 0;

    foreach (var line in Model.Lines)
    {
        @Html.EditorFor(m => line, "DifferentOrderLine", "Lines[" + i++ + "]")
    }
}
Community
  • 1
  • 1
GSerg
  • 71,102
  • 17
  • 141
  • 299
4

There seems to be no easier way of achieving this than described in the answer by @GSerg. Strange that the MVC Team has not come up with a less messy way of doing it. I've made this Extension Method to encapsulate it at least to some extent:

public static MvcHtmlString EditorForEnumerable<TModel, TValue>(this HtmlHelper<TModel> html, Expression<Func<TModel, IEnumerable<TValue>>> expression, string templateName)
{
    var fieldName = html.NameFor(expression).ToString();
    var items = expression.Compile()(html.ViewData.Model);
    return new MvcHtmlString(string.Concat(items.Select((item, i) => html.EditorFor(m => item, templateName, fieldName + '[' + i + ']'))));
}
Community
  • 1
  • 1
Anders
  • 674
  • 8
  • 11
3

There are a number of ways to address this problem. There is no way to get default IEnumerable support in editor templates when specifying a template name in the EditorFor. First, i'd suggest that if you have multiple templates for the same type in the same controller, your controller probably has too many responsibilities and you should consider refactoring it.

Having said that, the easiest solution is a custom DataType. MVC uses DataTypes in addition to UIHints and typenames. See:

Custom EditorTemplate not being used in MVC4 for DataType.Date

So, you need only say:

[DataType("MyCustomType")]
public IEnumerable<MyOtherType> {get;set;}

Then you can use MyCustomType.cshtml in your editor templates. Unlike UIHint, this does not suffer from the lack of IEnuerable support. If your usage supports a default type (say, Phone or Email, then prefer to use the existing type enumeration instead). Alternatively, you could derive your own DataType attribute and use DataType.Custom as the base.

You can also simply wrap your type in another type to create a different template. For example:

public class MyType {...}
public class MyType2 : MyType {}

Then you can create a MyType.cshtml and MyType2.cshtml quite easily, and you can always treat a MyType2 as a MyType for most purposes.

If this is too "hackish" for you, you can always build your template to render differently based on parameters passed via the "additionalViewData" parameter of the editor template.

Another option would be to use the version where you pass the template name to do "setup" of the type, such as create table tags, or other kinds of formatting, then use the more generic type version to render just the line items in a more generic form from inside the named template.

This allows you to have a CreateMyType template and an EditMyType template which are different except for the individual line items (which you can combine with the previous suggestion).

One other option is, if you're not using DisplayTemplates for this type, you can use DisplayTempates for your alternate template (when creating a custom template, this is just a convention.. when using the built-in template then it will just create display versions). Granted, this is counter-intuitive but it does solve the problem if you only have two templates for the same type you need to use, with no corresponding Display template.

Of course, you could always just convert the IEnumerable to an array in the template, which does not require redeclaring the model type.

@model IEnumerable<MyType>
@{ var model = Model.ToArray();}
@for(int i = 0; i < model.Length; i++)
{
    <p>@Html.TextBoxFor(x => model[i].MyProperty)</p>
}

I could probably think of a dozen other ways to solve this problem, but in all reality, any time I've ever had it, I've found that if I think about it, I can simply redesign my model or views in such a way as to no longer require it to be solved.

In other words, I consider having this problem to be a "code smell" and a sign that i'm probably doing something wrong, and rethinking the process usually yields a better design that doesn't have the problem.

So to answer your question. The correct, idiomatic way would be to redesign your controllers and views so that this problem does not exist. barring that, choose the least offensive "hack" to achieve what you want.

Community
  • 1
  • 1
Erik Funkenbusch
  • 90,480
  • 27
  • 178
  • 274
  • @GSerg - So no comment on any of this? – Erik Funkenbusch Oct 08 '14 at 17:17
  • Sorry, I've seen your answer and I'm supposed to get back to this issue soon, but a bit busy with other issues at the moment. I couldn't (and didn't) get the notification on that comment of yours though. – GSerg Oct 09 '14 at 12:35
  • With MVC 5.2.2, `[DataType("NamedOrderLine")]` on the enumerable property does not work - causes the same error as providing the template name in `EditorFor(m => m.Lines, "NamedOrderLine")` call (namely, *The model item passed into the dictionary is of type ``'System.Collections.Generic.List`1[Test_MVC_Templates.Models.OrderLine]'``, but this dictionary requires a model item of type `'Test_MVC_Templates.Models.OrderLine'`.*) – GSerg Oct 09 '14 at 14:36
  • Creating a wrapping type does not work either. Because MVC looks at the declared data types, not the actual runtime data types, you would need to create wrappers for both `OrderLine` and `Order`, with `OrderLineDerived` being the stupid no-op derivation, and `OrderDerived` being something different, where public `IEnumerable Lines { get; set; }` is replaced with `public IEnumerable Lines { get; set; }`. This is not possible with inheritance - the properties will have to coexist and have different names. – GSerg Oct 09 '14 at 14:44
  • Using display templates as another editor templates is indeed too hacky an unobvious. I'd rather not do it in any project where there would be maintenance in the future. – GSerg Oct 09 '14 at 14:46
  • Converting the model to array is something I already describe in the question itself - and it feels wrong to me. Maybe it is after all the correct way of doing it, who knows - hence the question. – GSerg Oct 09 '14 at 14:48
  • Redesigning the controllers seems to be sort of a wrong thing to do too. The default editing template is supposed to be used in editing a single entity (many fields, big sizes, lots of javascript editing helpers), and the second editing template is to edit a list of these entities, presented in a table, with only two fields visible and other fields hidden (small size, no helpers). Sometimes it is more convenient to do one-by-one editing, sometimes you want to see a whole lot to quickly change the relevant numbers on each line, but the idea behind this is the same - editing the entities. – GSerg Oct 09 '14 at 14:54
  • @GSerg - I assure you, using the custom datatype does not have the same problem. I've tested it, on MVC 5.2.2. It works fine. My guess is you are still doing something that is causing a different template to get called. I suggest creating a simple default test app and test it yourself, without any of your other code. – Erik Funkenbusch Oct 09 '14 at 14:56
  • That is what I did - `Test_MVC_Templates` from the error message is the empty test project I created. – GSerg Oct 09 '14 at 15:01
  • @GSerg - Also, EditorTemplates do not use the declared type, they use the actual type. I've used them extensively for "polymorphic" rendering where I pass different derived types and let the framework figure out which template to call. It works fine. This also suggests you are doing something to still call the wrong template. – Erik Funkenbusch Oct 09 '14 at 15:01
  • @GSerg - Ok, so i'm stupid.. you are right of course about the custom DataType. I mistyped and it was using the default template by mistake. I am certain about the derived type though, but I will verify as well. Of course this makes sense when I think about it.. The framework has a default template for IEnumerable, which follows the conventions to find sub-templates, and when you specify your own template you are overriding that. – Erik Funkenbusch Oct 09 '14 at 15:11
  • You are correct, polymorphic rendering does work. I failed to make the test templates different enough without realising it, so I was staring at the rendered custom template thinking it's the default one. So if we are sure there are no fundamentally more correct options, you can remove `DataType` from your answer so I can accept it. – GSerg Oct 09 '14 at 15:14
  • @GSerg - Yes, I just verified, polymorphic rendering does in fact work as I knew it did. I apologize for misleading you about the DataType, that was a brain lock on my part. Of course it will have the same problem, because ultimately it's replacing the default template that does the IEnumerable rendering. I actually like this behavior though, as it allows me to create wrapper code around the collection like table headers and what not which you wouldn't be able to do otherwise. – Erik Funkenbusch Oct 09 '14 at 15:18
  • @GSerg - By the way, if you are really interested, you could probably modify the existing EditorFor and call it EditorFor2 or something to create a version that does what you want. All the code is available here: https://aspnetwebstack.codeplex.com/SourceControl/latest#src/System.Web.Mvc/Html/EditorExtensions.cs and https://aspnetwebstack.codeplex.com/SourceControl/latest#src/System.Web.Mvc/Html/TemplateHelpers.cs – Erik Funkenbusch Oct 09 '14 at 15:22
  • And now that you've mentioned it, where is that default `IEnumerable` template you are talking about? The correct, idiomatic way should really be in its code. – GSerg Oct 09 '14 at 15:22
  • @GSerg - I was already ahead of you.. it's not really "a template" though, it's more of a set of code... that acts like a template, so you'll have to read the code. – Erik Funkenbusch Oct 09 '14 at 15:25
  • @GSerg - Actually, I think this is the one you want https://aspnetwebstack.codeplex.com/SourceControl/latest#src/System.Web.Mvc/Html/DefaultEditorTemplates.cs specifically, CollectionTemplate – Erik Funkenbusch Oct 09 '14 at 15:28
  • That was actually a useful piece of code. See if you think the answer I've posted is correct. – GSerg Oct 09 '14 at 17:10
-1

Use FilterUIHint instead of the regular UIHint, on the IEnumerable<T> property.

public class Order
{
    [FilterUIHint("OrderLine")]
    public IEnumerable<OrderLine> Lines { get; set; }
}

No need for anything else.

@Html.EditorFor(m => m.Lines)

This now displays an "OrderLine" EditorTemplate for each OrderLine in Lines.

Apache
  • 602
  • 5
  • 20
  • FilterUIHint is not used by MVC, it's a WebForms attribute. MVC would have used OrderLine template anyway because of it's type. Your example is meaningless. – Erik Funkenbusch Oct 07 '14 at 02:44
  • `FilterUIHint` is in the namespace `System.ComponentModel.DataAnnotations`, same as `UIHint`. I'm using it with MVC. I followed the steps to reproduce the error and hit the same problem. Without adding any references, or implementing anything from WebForms, `FilterUIHint` solved the problem. Could you elaborate on what you mean? – Apache Oct 07 '14 at 03:50
  • 1
    FilterUIHint did not solve the problem, removing the template name from the EditorFor allowed default behavior. If you remove the attribute, it will do the same thing. System.ComponentModel.DataAnnotation is a namespace that is not specific to MVC, it contains annotations for WebForms, MVC, Entity Framework, and many other technologies... MVC only uses some of those attributes, and FilterUIHint is not one of them that it uses. If you still don't believe me, try to find *ANY* reference to FilterUIHint being used with MVC anywhere. You won't find it. – Erik Funkenbusch Oct 07 '14 at 16:01
  • 1
    Your code above works because that is the default behavior for EditorTemplates. EditorTemplates use the typename.cshtml for the EditorTemplate name by default. Change your attribute to say `[FilterUIHInt("SomethingElse")]` and you will see that it *STILL* uses OrderLine.cshtml. – Erik Funkenbusch Oct 07 '14 at 16:04
  • Thank you for your explanation, and for your downvote. ;) I'm still unsure as to how this doesn't solve the OP's question, even noting your comments here. The resulting form was able to be passed back and forth between Model, View and Controller, and CRUD operations worked fully as expected. I've tried multiple variations on the theme, and it seems like it solves everything the OP was asking. I know I will be missing something, somewhere though. I agree with you that changing the structure of the Models is preferable to devising hacks around it. – Apache Oct 07 '14 at 22:40
  • It doesn't solve his question because a) you aren't actually solving his question, and b) you are suggesting something that is incorrect, and c) doesn't even seem to understand the question enough to know why it doesn't solve the question. Once again, FilterUIHint does ABSOLUTELY NOTHING in MVC. Change the parameter to the attribute to something else, and you will see that it simply does nothing. – Erik Funkenbusch Oct 08 '14 at 03:40
  • I had deleted this post, but have chosen to undelete for posterity. The code did work as expected, even though the comments would have you believe otherwise, so it may form the beginning of a solution, taking in mind the conversation within the comments. I was able to replicate the initial problem, and whatever I have done here did solve that problem. Whether it's the correct way to do it or not. It didn't work before these changes, and it did work after the changes. Take from it what you will. – Apache Feb 08 '21 at 04:11
-2

You can use an UIHint attribute to direct MVC which view you would like to load for the editor. So your Order object would look like this using the UIHint

public class Order
{
    [UIHint("Non-Standard-Named-View")]
    public IEnumerable<OrderLine> Lines { get; set; }
}
solidau
  • 3,824
  • 1
  • 22
  • 40
  • If you read through the original question I am linking to, you will find "*Once you specify a custom name for the display template (either as second argument of the `DisplayFor` helper **or as `[UIHint]` attribute**) it will no longer loop for collection properties*". Besides using an attribute does not make sense in this situation, because there is more than one template. If there only was one, I could simply name it after the type name and it would work automatically. – GSerg Aug 28 '14 at 08:42