2

I have a Contact object that has a number properties, including a child that is a list of Addresses.

public class Contact
{ 
  public int? Id { get; set; }
  public string Name { get; set; }
  public IReadOnlyList<IAddress> Addresses
  {
      [Lazy Load Code to populate and return list]
  } 
  [...]
}

I want to allow the user to edit the addresses without having to edit (or post) the whole Contact object. Currently in the UI I have the addresses listed out with an Edit button next each one:

enter image description here

I'm using the Modal syntax that is part of Bootstrap3, and have hidden DIVs that contain a form and the fields to edit an Address. When the user clicks on edit, a modal window form appears that allows the user to edit the Address. Model Binding and validation work within this form.

It works pretty well but I have a few underlying issues with the implementation. I wanted to use the builtin Validation and Model Binding with the Address objects, but I didn't want to post back the whole object just to edit one address.

So I ended up having to create a for loop that writes out the hidden DIVs calling a Partial View and passing Address as the model:

@for (int i = 0; i < Model.Addresses.Count; i++)
{
        address = (Address)Model.Addresses[i];
        @Html.Partial("_AddressModal", address);
}

The unfortunate side-effect is that the model binding cannot uniquely identify which Address to apply the ModelState to, so Model Binding applies it to all the Address in in the hidden DIVs, instead of the one that was updated. This is because they have the exact same property names.

I've got a work-around that was part of an earlier question. Basically it doesn't write the ModelState unless the object is invalid. For the invalid scenario I don't show the address list which basically hides the problem. Otherwise every edit button would show the same form content.

What I want to know is if there is a better UX approach to allow a user to edit one of the child Addresses in the list?

My goals are to find a better approach using ASP.NET MVC that:

  1. Follow the PRG (Post-Redirect-Get) pattern
  2. Don't redirect to a separate Page/View for editing, which forces the user to navigate back to the contact edit form again
  3. Avoid popups because of the blockers
  4. The solution allows the use Model Binding and Validation for the Address object
Community
  • 1
  • 1
Josh
  • 8,219
  • 13
  • 76
  • 123
  • This question is going to be too subjective for SO - you should either: 1) decide on an approach, try and implement it and ask a question if you hit a *technical* stumbling block; or 2) form a pure UX question and ask it on http://ux.stackexchange.com/. If you really want to ask the question you're asking, StackExchange is not the right place to ask it - you're asking for ideas, not asking a question that actually has an answer. – Ant P Feb 07 '15 at 20:33
  • I've updated it to ask for a better UX approach to the one I have, since it has some underlying problems. – Josh Feb 07 '15 at 20:35
  • Does each 'hidden div' you are rendering with the partial contain a form for editing the address? If that's the case I would suggest a different approach where you only have a single form and when you click `Edit`, populate the form using either (1) ajax by calling a method that returns json containing the address properties; or (2) store the address properties in the button (using `data-` atributes). You only have one set of controls (no validation issues) and post the form using ajax (and stay on the same page to continue editing other addresses) –  Feb 08 '15 at 01:15
  • Yes, they each have a form that after submit follows the PRG pattern. Option 1 sounds good, would option 2 with Ajax still support validation such as ModelState containing a server-side error since you wouldn't have a page refresh? – Josh Feb 08 '15 at 01:32
  • @Josh, Yes it would still support validation as long as the 'blank' form based on `Address` is rendered in the view (say using a partial) and you include the associated `@Html.ValidationMessageFor()` for each property. Since its an ajax post you would need to call `.Valid()` and prevent the post is it returns `false`. Happy to give an example if helps. –  Feb 08 '15 at 01:54
  • @StephenMuecke doesn't have to be ajax. OP didn't even mention that. Ajax here is complete optional. – Bart Calixto Feb 08 '15 at 02:14
  • @Bart, Yes I know, its why I stated _I would **suggest** a different approach_ (which will be far less code and give better performance that the code which will be necessary to solve OP's issue) –  Feb 08 '15 at 02:18
  • @StephenMuecke "which will be far less code" ? Why and how is it going to be less code? I suggest you to stop doing bold **wrong** statements. – Bart Calixto Feb 08 '15 at 02:23

1 Answers1

-1

What you want is :

1 Form for the contact Another form for each address

in your controller you will have an action that expects a contact as a parameter (without addresses) and another action that expects an address.

boilerplate code:

    [HttpPost]
    public RedirectToRouteResult EditAddress(int id, ContactAddressBindingModel address) {
        // ...
    }

    [HttpPost]
    public RedirectToRouteResult EditContact(int id, ContactBindingModel contact)
    {
        // ...
    }

    public class ContactViewModel : ContactBindingModel
    {
        public IReadOnlyList<IAddress> Addresses { // ...}
    }
    public class ContactBindingModel
    {
        public int? Id { get; set; }
        public string Name { get; set; }
    }
    public class ContactAddressBindingModel : IAddress
    {
        public int? Id { get; set; }
        public string City { get; set; }
        public string Country { get; set; }

    }

and in the view :

<form action="EditContact">
<!-- contacts inputs etc -->
</form>

// in razor you can do EditorFor(m => m.Addresses) instead of foreach
// and have partialview, or whatever you like.

@foreach (Model.Addresses) { 
<form action="EditAddress">
<!-- address inputs etc -->
</form>
}
Bart Calixto
  • 19,210
  • 11
  • 78
  • 114
  • Your misunderstanding the issue. By having multiple forms (each having controls with duplicate `name` and `id` attributes - which is invalid html), `jquery-validate-unobtrusive` will not be able to assign any validation error message to the correct control (because there are multiple inputs with (say) `name="Address.City"` –  Feb 08 '15 at 01:59
  • @StephenMuecke your statement is wrong. First, duplicate name is valid HTML and also is used extensively on radio button yet they won't be duplicated on same form, also you wont have duplicate ids because MVC take care of this when editing collections. it creates something like `id="Address[0].City"`.Also, if you don't go the Helpers route you don't need the ids on any input. I don't misunderstand the issue, actually I'm really well aware of it. Last, I don't like your tone. – Bart Calixto Feb 08 '15 at 02:06
  • Oh, and btw, since you won't have duplicate names on the same form, `jquery-validate-unobtrusive` will work without issues. – Bart Calixto Feb 08 '15 at 02:12
  • By invalid html I was referring to duplicate `id` attributes! (duplicate `name` attributes are fine) And when you render a collection using a loop and a partial your **DO** generate duplicate `name` and `id` attributes (unless using a helper such as `@Html.BeginCollectionItem()` –  Feb 08 '15 at 02:15
  • @StephenMuecke my sample show code without Helpers so I don't see how I am generating duplicates ID. If using partials, (as I noted on my commented code) you don't generate duplicate ids because you do EditorFor and not foreach. You don't need BeginCollectionItem, nor foreach, which I stated clearly on my response. I think you misunderstand the inner workings of mvc. – Bart Calixto Feb 08 '15 at 02:18
  • All I can suggest is you try it. Rendering a collection using a `foreach` loop will render duplicate `id` and `name` attributes. It would need to be a `for(int i = 0; i < Model.Addesses.Count; i++) { @Html.TextBoxFor(m => m.Address[i].City) }` which renders the correct name with indexers, but of course this wont post back to your `EditAddress()` method because it accepts type of `Address`, not `IEnumerable
    `. Even if you changed the signature it would not work anyway because the `ModelBinder` will only bind collections where the indexer starts at `0`
    –  Feb 08 '15 at 02:24
  • @StephenMuecke **you don't need foreach nor for**. You can EditorFor to collection that render a view with a `@model` directive of Address and not IEnumerable. – Bart Calixto Feb 08 '15 at 02:27
  • Yes you can use `EditorFor` and it will add the correct `name` attributes with indexers, but you cannot then post it back as I explained in my last comment (and in any case your code shows a `foreach` loop) –  Feb 08 '15 at 02:29
  • @StephenMuecke geez! you can post it, what you explained is all wrong. you don't need to be right, but if its that you want, you are right. – Bart Calixto Feb 08 '15 at 02:30
  • I'm trying to help you. Please try your code so you can understand why it won't work. –  Feb 08 '15 at 02:32
  • @StephenMuecke stay away from my answer, my code **DOES** work. – Bart Calixto Feb 08 '15 at 02:33