Incorrect List Model Binding indices when using HT

Question

this is a tricky one to explain, so I'll try bullet pointing.

Issue:

1. Dynamic rows (collection) available to user on View (add/delete)
2. User deletes row and saves (POST)
3. Collection passed back to controller with non-sequential indices
4. Stepping through code, everything looks fine, collection items, indices etc.
5. Once the page is rendered, items are not displaying correctly - They are all out by 1 and therefore duplicating the top item at the new 0 location.

What I've found:

This happens ONLY when using the HTML Helpers in Razor code. If I use the traditional <input> elements (not ideal), it works fine.

Question:

Has anyone ever run into this issue before? Or does anyone know why this is happening, or what I'm doing wrong?

Please check out my code below and thanks for checking my question!

Controller:

    [HttpGet]
    public ActionResult Index()
    {
        List<Car> cars = new List<Car>
        {
            new Car { ID = 1, Make = "BMW 1", Model = "325" },
            new Car { ID = 2, Make = "Land Rover 2", Model = "Range Rover" },
            new Car { ID = 3, Make = "Audi 3", Model = "A3" },
            new Car { ID = 4, Make = "Honda 4", Model = "Civic" }

        };

        CarModel model = new CarModel();
        model.Cars = cars;

        return View(model);
    }

    [HttpPost]
    public ActionResult Index(CarModel model)
    {
        // This is for debugging purposes only
        List<Car> savedCars = model.Cars;

        return View(model);
    }

Index.cshtml:

As you can see, I have "Make" and "Actual Make" inputs. One being a HTML Helper and the other a traditional HTML Input, respectively.

    @using (Html.BeginForm())
{
    <div class="col-md-4">

        @for (int i = 0; i < Model.Cars.Count; i++)
        {
            <div id="car-row-@i" class="form-group row">
                <br />
                <hr />

                <label class="control-label">Make (@i)</label>
                @Html.TextBoxFor(m => m.Cars[i].Make, new { @id = "car-make-" + i, @class = "form-control" })

                <label class="control-label">Actual Make</label>
                <input class="form-control" id="car-make-@i" name="Cars[@i].Make" type="text" value="@Model.Cars[i].Make" />

                <div>
                    <input type="hidden" name="Cars.Index" value="@i" />
                </div>

                <br />

                <button id="delete-btn-@i" type="button" class="btn btn-sm btn-danger" onclick="DeleteCarRow(@i)">Delete Entry</button>

            </div>
        }

        <div class="form-group">
            <input type="submit" class="btn btn-sm btn-success" value="Submit" />
        </div>

    </div>
}

Javascript Delete Function

function DeleteCarRow(id) {

    $("#car-row-" + id).remove();

}

What's happening in the UI:

Step 1 (delete row) Step 2 (Submit form) Step 3 (results)

Answer 1

The reason for this behavior is that the HtmlHelper methods use the value from ModelState (if one exists) to set the value attribute rather that the actual model value. The reason for this behavior is explained in the answer to TextBoxFor displaying initial value, not the value updated from code.

In your case, when you submit, the following values are added to ModelState

Cars[1].Make: Land Rover 2
Cars[2].Make: Audi 3
Cars[3].Make: Honda 4

Note that there is no value for Cars[0].Make because you deleted the first item in the view.

When you return the view, the collection now contains

Cars[0].Make: Land Rover 2
Cars[1].Make: Audi 3
Cars[2].Make: Honda 4

So in the first iteration of the loop, the TextBoxFor() method checks ModelState for a match, does not find one, and generates value="Land Rover 2" (i.e. the model value) and your manual input also reads the model value and sets value="Land Rover 2"

In the second iteration, the TextBoxFor() does find a match for Cars[1]Make in ModelState so it sets value="Land Rover 2" and manual inputs reads the model value and sets value="Audi 3".

I'm assuming this question is just to explain the behavior (in reality, you would save the data and then redirect to the GET method to display the new list), but you can generate the correct output when you return the view by calling ModelState.Clear() which will clear all ModelState values so that the TextBoxFor() generates the value attribute based on the model value.

Side note:You view contains a lot of bad practice, including polluting your markup with behavior (use Unobtrusive JavaScript), creating label element that do not behave as labels (clicking on them will not set focus to the associated control), unnecessary use of <br/> elements (use css to style your elements with margins etc) and unnecessary use of new { @id = "car-make-" + i }. The code in your loop can be

@for (int i = 0; i < Model.Cars.Count; i++)
{
    <div class="form-group row">
        <hr />
        @Html.LabelFor(m => m.Cars[i].Make, "Make (@i)")
        @Html.TextBoxFor(m => m.Cars[i].Make, new { @class = "form-control" })
        ....
        <input type="hidden" name="Cars.Index" value="@i" />
        <button type="button" class="btn btn-sm btn-danger delete">Delete Entry</button>
    </div>
}

$('.delete').click(function() {
    $(this).closest('.form-group').remove();
}