Mechanism for providing validation CSS class names through DI

[FWest98]

Adding mechanism for providing validation CSS class names through DI
This PR adds an interface for providing CSS class names for field/form validation with an associated default implementation. Previously, these were hardcoded in read-only static variables. Now, users can provide their own implementation and thus their own class names to fit any front-end framework, by simply providing a custom implementation of the interface.

PR Description
This is a first shot at fixing #17412. I hope this is the desired format/solution approach. I am open to suggestions on other approaches. There are no added tests, since testing for the right class names is already included in many other tests. I am able to add dedicated tests for delivering a custom implementation, but that seemed a bit overkill to me, since none of these strings are hardcoded anywhere else in the codebase.

I believe this technically does change the API, since a number of previously-public fields are now gone. Personally, I would think that removing these in the next major release would be acceptable, but you might beg to differ. In that case, I would see a number of approaches:

• Keeping the static fields in HtmlHelper as they always were, but that might be a bit confusing
• Keeping the static fields for now and marking them obsolete

This PR would probably need some follow-up:

• Adding documentation (separate repo)
• Currently, the unobtrusive validation jQuery library has the default values hardcoded. There is a way to carry the custom values from C# into the jQuery lib (https://gist.github.com/FWest98/9141b3c2f260bee0e46058897d2017d2#file-_validationscriptspartial-cshtml, based on slightly different implementation), however, I am not sure whether it is possible to implement something like that in this repo.

Fixes #17412

[dnfadmin]

All CLA requirements met.

[pranavkm]

@javiercn didn't you look at this at one point?

[pranavkm]

Could we make these properties on HtmlHelperOptions rather than a separate type? We generally don't expect users to customize options by injecting new DI services. That also avoids a bunch of source / binary breaking changes that were introduced as part of this PR.

[FWest98]

Ah, yes, I had not spotted that type. That sounds like a good alternative that has less impact on the code base.

What binary breaking change do you refer to? Because the HtmlHelper properties will still be removed, right?

[pranavkm]

We would keep the HtmlHelper properties as is since removing them would be breaking. Adding a new constructor parameter is also a breaking change.

[FWest98]

I agree this is a much better approach. I implemented it. I did not yet re-add the static fields to HtmlHelper, as it might be confusing for people that these variables are defined there while they are not used anymore... Should we annotate them Obsolete, or add some comment?

[pranavkm]

@FWest98 do you mind updating the issue with an API proposal? Here's the template for it:

Proposed API

Please provide the specific public API signature diff that you are proposing. For example:

namespace Microsoft.AspNetCore.Http
{
    public static class HttpResponseWritingExtensions
    {
+       public Task WriteAsync(this HttpResponse response, StringBuilder builder);
    }

You may find the Framework Design Guidelines helpful.

Usage Examples

Please provide code examples that highlight how the proposed API additions are meant to be consumed.
This will help suggest whether the API has the right shape to be functional, performant and useable.
You can use code blocks like this:

// some lines of code here

Alternative Designs

Were there other options you considered, such as alternative API shapes?
How does this compare to analogous APIs in other ecosystems and libraries?

Risks

Please mention any risks that to your knowledge the API proposal might entail, such as breaking changes, performance regressions, etc.

[FWest98]

The API proposal can be found in #17412 (comment)

[javiercn]

I don't think this change is enough.

I believe this will not work with client-side validation.

The other aspect to consider here is that if we open the way to configure these things we should do it in a way is configurable locally to a given view rather than having a global default that can interfere with existing libraries.

[FWest98]

As I mentioned, this indeed will not change the client-side validation. However, there is a relatively easy way to change that yourself (ref: https://gist.github.com/FWest98/9141b3c2f260bee0e46058897d2017d2#file-_validationscriptspartial-cshtml as an example). Furthermore, not everyone uses the default client-side validation, but they might roll their own solution. Those people will still benefit from this change. I think this narrows down to properly documenting the side-effects of changing the default and documenting the manual change to fix the client-side validation.

I agree that it would be much better to have this configurable in a view, but that would require a lot more engineering work (and would be even more complicated to integrate in the client-side validation). I think this would be a great first step, considering that this feature has been requested for almost 2 years now.

Of course, I am open to contribute a more extensive solution for this; but I am not sure what the best design would be for either the client-side validation or the per-view configuration.

[javiercn]

As I mentioned, this indeed will not change the client-side validation. However, there is a relatively easy way to change that yourself (ref: https://gist.github.com/FWest98/9141b3c2f260bee0e46058897d2017d2#file-_validationscriptspartial-cshtml as an example). Furthermore, not everyone uses the default client-side validation, but they might roll their own solution. Those people will still benefit from this change. I think this narrows down to properly documenting the side-effects of changing the default and documenting the manual change to fix the client-side validation.

This is not a behavior we want to have. If you change the validation you expect it to work in both places, that's been the case for this feature since the beginning of MVC, that's a breaking change we aren't willing to make I think.

[FWest98]

How would you propose to integrate updating the client-side validation as well? I would see a few options:

• Distinguish between "styling" and "functionality" classes - i.e. the user can set their own custom styling classes to fit their UI framework but the default classes will always be included since they also impact functionality.
• Think of some way to update the client-side options automatically, but that might be difficult to achieve with the versatility of the client-side model; we already ask users to include the js file manually (or it is included in a template) so I do not see how we can transfer this information easily
• A last option would be to add additional data- attributes (such as data-validation-error-classname) to encode this information - which would also allow us to do so on an element-specific level. That would be the most fine-grained solution, but might break compatibility with the existing jQuery-validation plugin. (although rewriting it to get rid of the jQuery dependency is a change I personally would welcome a lot, and there seems to be wider demand for that: https://stackoverflow.com/questions/43149603/asp-net-core-validation-without-jquery)

[FWest98]

@javiercn Could you maybe give some directions on how to implement the client-side validation properly as well? I posted some suggestions before, and I would be happy to help implement a full solution!

[pranavkm]

Thanks for the PR @FWest98. Unfortunately based on @javiercn's comments, it sounds like addressing this in isolation is not an option we'd like to consider. We do have #8573 parked in the .NET 7 milestone that should help with this particular scenario. Closing this PR for now.