Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Decouple HtmlHelper.GetValidationAttributes from UnobtrusiveValidationAttributesGenerator.GetValidationAttributes #659

Closed
roryprimrose opened this issue Jun 13, 2014 · 9 comments
Closed

Decouple HtmlHelper.GetValidationAttributes from UnobtrusiveValidationAttributesGenerator.GetValidationAttributes #659

roryprimrose opened this issue Jun 13, 2014 · 9 comments
Assignees
@dougbu
Labels
3 - Done enhancement
Milestone
1.0.0-alpha3

Comments

@roryprimrose
Copy link

MVC should not be tightly coupled to Microsoft's implementation of unobtrusive validation. For example, I want to use bootstrapValidator because it is a much better validation framework for working with bootstrap. As it stands, HtmlHelper is tightly coupled to UnobtrusiveValidationAttributesGenerator such that a different implementation cannot be used. This locks everyone into using unobtrusive validation rather than allowing support for custom validation attribute rendering on form fields.
@dougbu
Copy link
Member

dougbu commented Jun 13, 2014

@roryprimrose the general HTML helper extensibility point is the overall IHtmlHelper / IHtmlHelper<TModel> implementations placed in DI. You're free to subclass HtmlHelper / HtmlHelper<TModel> or replace the default implementations with something entirely new.

For this particular coupling, I'd normally recommend overriding protected IDictionary<string, object> GetValidationAttributes(string name, ModelMetadata metadata). If we made that method virtual, would we solve your issue?

@Eilon
Copy link
Member

Eilon commented Jun 13, 2014

@DamianEdwards I think you were toying with some ideas around validation extensibility - were you?

@roryprimrose
Copy link
Author

It would certainly help. Presumably most of that method is still valid however. The only line that would require overriding is

return UnobtrusiveValidationAttributesGenerator.GetValidationAttributes(clientRules);

@yishaigalatzer
Copy link
Contributor

@DamianEdwards / @dougbu - Could you get together and come up with an update for this design.

@yishaigalatzer yishaigalatzer added this to the 6.0.0-alpha3 milestone Jul 22, 2014
@dougbu
Copy link
Member

dougbu commented Jul 22, 2014

👌

@dougbu
Copy link
Member

dougbu commented Jul 25, 2014

Will generalize this ask a little bit and make a couple of other corrections to the HtmlHelper methods and properties available in extension methods and subclasses. See https://github.com/aspnet/MusicStore/blob/dev/src/MusicStore.Spa/Helpers/AngularHtmlHelper'T.cs for an example of current ugly workarounds for the general issue.

@dougbu
Copy link
Member

dougbu commented Jul 28, 2014

Small update related to the AngularHtmlHelper<TModel> class: The GenerateId() method should not need to be public. For one thing Id() is a direct and public wrapper around this method. Further ViewData.TemplateInfo.GetFullHtmlFieldName() is already public though admittedly it returns a string and does not HTML encode that value. Most importantly GenerateIdFromName() is what really should be used (see #704) and it's also public.

@DamianEdwards
Copy link
Member

Thanks, feel free to fix it for me :)

dougbu added a commit that referenced this issue Jul 31, 2014
@dougbu
Improve HtmlHelper extension points
ddc3698 
- address all of #659 and a bit of #874 (avoid `public virtual` methods in
  `HtmlHelper`)

- make `MetadataProvider` and `GetClientValidationRules()` `public` and
  therefore available to extension methods
- remove unused `GetValidationAttributes()` overload
- make remaining `GetValidationAttributes()` overload (and not
  `GetClientValidationRules()`) `virtual`, allowing derived classes to
  change the attributes without overriding all callers
- reverse `GetValidationAttributes()` and `GetClientValidationRules()`
  parameter order to match precedence
- add `GenerateName()` and `GenerateValidationSummary()` to make
  `protected virtual` method names consistent
- `Name()`, `ValidationSummary()` and `TextArea()` are no longer `virtual`
  because `protected virtual Generate*()` methods exist for all

- move `GenerateDisplay()` to a more obvious location in the file
dougbu added a commit that referenced this issue Jul 31, 2014
@dougbu
Improve HtmlHelper extension points
3310964 
- address all of #659 and a bit of #874 (avoid `public virtual` methods in
  `HtmlHelper`)

- make `MetadataProvider` and `GetClientValidationRules()` `public` and
  therefore available to extension methods
- remove unused `GetValidationAttributes()` overload
- make remaining `GetValidationAttributes()` overload (and not
  `GetClientValidationRules()`) `virtual`, allowing derived classes to
  change the attributes without overriding all callers
- reverse `GetValidationAttributes()` and `GetClientValidationRules()`
  parameter order to match precedence
- add `GenerateName()` and `GenerateValidationSummary()` to make
  `protected virtual` method names consistent
- `Name()`, `ValidationSummary()` and `TextArea()` are no longer `virtual`
  because `protected virtual Generate*()` methods exist for all

- move `GenerateDisplay()` to a more obvious location in the file
dougbu added a commit that referenced this issue Jul 31, 2014
@dougbu
Improve HtmlHelper extension points
4bb6929 
- address all of #659 and a bit of #874 (avoid `public virtual` methods in
  `HtmlHelper`)

- make `MetadataProvider` and `GetClientValidationRules()` `public` and
  therefore available to extension methods
- remove unused `GetValidationAttributes()` overload
- make remaining `GetValidationAttributes()` overload (and not
  `GetClientValidationRules()`) `virtual`, allowing derived classes to
  change the attributes without overriding all callers
- reverse `GetValidationAttributes()` and `GetClientValidationRules()`
  parameter order to match precedence
- add `GenerateName()` and `GenerateValidationSummary()` to make
  `protected virtual` method names consistent
- `Name()`, `ValidationSummary()` and `TextArea()` are no longer `virtual`
  because `protected virtual Generate*()` methods exist for all
dougbu added a commit that referenced this issue Jul 31, 2014
@dougbu
Improve HtmlHelper extension points
40eb05f 
- address all of #659 and a bit of #874 (avoid `public virtual` methods in
  `HtmlHelper`)

- make `MetadataProvider` and `GetClientValidationRules()` `public` and
  therefore available to extension methods
- remove unused `GetValidationAttributes()` overload
- make remaining `GetValidationAttributes()` overload (and not
  `GetClientValidationRules()`) `virtual`, allowing derived classes to
  change the attributes without overriding all callers
- reverse `GetValidationAttributes()` and `GetClientValidationRules()`
  parameter order to match precedence
- add `GenerateName()` and `GenerateValidationSummary()` to make
  `protected virtual` method names consistent
- `Name()`, `ValidationSummary()` and `TextArea()` are no longer `virtual`
  because `protected virtual Generate*()` methods exist for all
@dougbu
Copy link
Member

dougbu commented Jul 31, 2014

Closed with commit 40eb05f

@dougbu dougbu closed this as completed Jul 31, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dougbu dougbu

Labels
3 - Done enhancement
Projects
None yet
Milestone
1.0.0-alpha3
Development

No branches or pull requests
6 participants
@Eilon @DamianEdwards @yishaigalatzer @danroth27 @roryprimrose @dougbu