Behavior change for data-ajax-update target

[adschmu]

Describe the bug

For the release 9.1.2, the parsing behavior of the UpdateTargetId in AjaxOptions (i.e. "data-ajax-update" for unobtrusive ajax) has been changed: Previously, one would only specify the ID without prepending a hash symbol.

With 9.1.2, this has been changed [1] so one now has to provide a full identifier with hash symbol ('#myid').
If the code is not updated like this, i.e. when somebody just bumps the package version, switching to a page different than "1" will simply do nothing.

This is considered an issue because:

1. There was no notice about it, so updating the package breaks existing code at runtime. I consider this a "breaking change" and would label it as such e.g. in the release notes (if it was intended).
2. Adapting to the change requires finding and replacing all cases in the code, which means a lot of work for big projects.
3. The name "UpdateTargetId" implies that it is an "Id", and not an arbitrary HTML element, so I would expect to omit the hash symbol also based on that.

Of course, without the change one can only choose update targets based on IDs and not based on classes, for instance. However, this will probably be used with IDs most of the time anyway. For arbitrary elements, one should IMO then also rename the property to e.g. just "UpdateTarget". This would also have the upside of creating compile time errors for this breaking change.

[1] 5c94faf#diff-09743ab05c132d0d41dc5ded4a73b8b81c59b378f6c8c6b5bd1c8d1b256d2ecbL13

Example call (before 9.1.2):

@Html.PagedListPager(Model.Machines, page => Url.Action("GetMachines", "Machine"
        , new { SystemId = Model.SystemId, Model.ClientId, page = page })
        , PagedListRenderOptions.EnableUnobtrusiveAjaxReplacing(new PagedListRenderOptions { UlElementClasses = new List<string> { "pagination" } }
            , new AjaxOptions { AllowCache = false, HttpMethod = "GET", InsertionMode = InsertionMode.Replace, UpdateTargetId = "machineview" }))

Example call (since 9.1.2):

@Html.PagedListPager(Model.Machines, page => Url.Action("GetMachines", "Machine"
        , new { SystemId = Model.SystemId, Model.ClientId, page = page })
        , PagedListRenderOptions.EnableUnobtrusiveAjaxReplacing(new PagedListRenderOptions { UlElementClasses = new List<string> { "pagination" } }
            , new AjaxOptions { AllowCache = false, HttpMethod = "GET", InsertionMode = InsertionMode.Replace, UpdateTargetId = "#machineview" }))

Suggested resolution:

1. My preferred option would be to revert the change, so we do not have to invest several hours into replacing all the identifiers and test all of these cases. Based on the arguments above, this also matches the name.
2. If the change is not to be reverted, it should be communicated as a breaking change. One could make things easier by either renaming the UpdateTargetId to UpdateTarget, to cause build errors and thus make the change more obvious.
3. Or one could add multiple properties like UpdateTargetId and UpdateTargetClass, which then would be both translated to data-ajax-update by prepending the correct symbol. This would provide compatibility for the old code. Of course, this would not allow arbitrary identifiers with combinations of symbols.

I will revert to 8.4.7 for now.

[zeynelozturk]

Finding the source of this issue took my 1-2 hours off, after updating X.PagedList. I could only find this page about the issue, after debugging jquery.unobtrusive-ajax.js and see it doesn't get "data-ajax-update". Oh...

[adschmu]

Commit: 01d9633
Issue: #221

[adschmu]

I've proposed a revert in #264

The name UpdateTargetId is not properly chosen for anything else than an ID, and if it is an ID anyway we should not need to specify the hash.