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

Improve IHtmlHelper, HtmlHelper, et cetera public API; correct inconsistencies #874

Closed
dougbu opened this issue Jul 28, 2014 · 1 comment
Closed

Improve IHtmlHelper, HtmlHelper, et cetera public API; correct inconsistencies #874

dougbu opened this issue Jul 28, 2014 · 1 comment
Assignees
@dougbu
Labels
3 - Done enhancement
Milestone
1.0.0-alpha4

Comments

@dougbu
Copy link
Member

dougbu commented Jul 28, 2014

This bug is about removing inconsistencies created as we ported HTML helpers to MVC 6. As we evolved a few decisions across the multiple commits, we haven't fixed up the earlier work to match.

  1. Copy XML comments from IHtmlHelper and IHtmlHelper<TModel> to corresponding extension methods.
  2. Remove remaining overloads with an IDictionary<string, object> parameter e,g, IHtmlHelper.TextBox() and IHtmlHelper.ValidationSummary(). We decided to stick with just the object parameters.
  3. Add missing [NotNull] for Expression<Func<TModel, TProperty>> expression parameter in HtmlHelper.ValueFor() and throughout EditorExtensions.
  4. Add missing [NotNull] for this IHtmlHelper html parameters in EditorExtensions.
  5. Correct extension class names to match containing file names (which are all correct) e.g. EditorExtensions and SelectExtensions.
  6. Support null expression names everywhere e.g. HtmlHelper.Value(). Distinct handling of null and empty parameters in some helpers (but not others) has no value.
  7. Call interface methods from extension methods, not other extension methods e.g. html.LabelForModel(), html.TextArea(), html.TextBox(), and html.ValueForModel() overloads. Our code should reference these extension methods only in tests. (Part of the background here is our default parameter values are now consistently null, making "provide default in one place" much less important.)
    1. Similar issues exist in XML comment references e.g. those from ViewContext to html.ValidationMessage() and html.ValidationSummary().
    2. In addition our template actions should avoid extension methods e.g. calls from DefaultEditorTemplates to html.DropDownList(), html.Label(), and html.TextBox().
  8. Avoid public virtual methods in HtmlHelper (ignoring HtmlHelper.Contextualize()) e.g. HtmlHelper.Name(), HtmlHelper.ValidationSummary() and HtmlHelper.TextArea(). public methods that are extensibility points should have a corresponding protected virtual method.
  9. nit: May want to reorder methods alphabetically and with public methods before others. HtmlHelper is getting fairly large.
@yishaigalatzer yishaigalatzer added this to the 6.0.0-alpha4 milestone Jul 28, 2014
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 added a commit that referenced this issue Aug 4, 2014
@dougbu
Remove HTML helpers with IDictionary<string, object> parameters
3db0a80 
- see line 2 of #874
- focus on `TextBox[For]()` and `ValidationSummary()`

related fixes included here:
- `TextArea[For]()` documentation incorrectly indicated their `htmlAttributes` parameters were dictionaries
- handle `htmlAttributes` parameters more consistently; create a dictionary only when necessary
@dougbu dougbu mentioned this issue Aug 4, 2014
Merged
dougbu added a commit that referenced this issue Aug 4, 2014
@dougbu
Make HTML helper null handling consistent
3d97fed 
- #874 lines 3, 4, and 6
- correct `Value()` to treat a `null` expression name the same as `string.Empty`
- add missing `[NotNull]` attributes in `EditorExtensions` and for `GenerateIdFromName()`
- consistently pass `null` for default expression names to the helpers
 - for example, from extension methods
- add test cases using `null` for expression name

nits:
- correct summary XML comment for `HtmlHelper` class
- use named parameters and prefer interface (not extension) methods in changed calls
@dougbu dougbu mentioned this issue Aug 4, 2014
Merged
dougbu added a commit that referenced this issue Aug 5, 2014
@dougbu
Make HTML helper null handling consistent
7347844 
- #874 lines 3, 4, and 6
- correct `Value()` to treat a `null` expression name the same as `string.Empty`
- add missing `[NotNull]` attributes in `EditorExtensions` and for `GenerateIdFromName()`
- consistently pass `null` for default expression names to the helpers
 - for example, from extension methods
- add test cases using `null` for expression name

nits:
- correct summary XML comment for `HtmlHelper` class
- use named parameters and prefer interface (not extension) methods in changed calls
- use `string.Empty` instead of `""` in a few tests
dougbu added a commit that referenced this issue Aug 5, 2014
@dougbu
Make HTML helper null handling consistent
3b6041b 
- #874 lines 3, 4, and 6
- correct `Value()` to treat a `null` expression name the same as `string.Empty`
- add missing `[NotNull]` attributes in `EditorExtensions` and for `GenerateIdFromName()`
- consistently pass `null` for default expression names to the helpers
 - for example, from extension methods
- add test cases using `null` for expression name

nits:
- correct summary XML comment for `HtmlHelper` class
- use named parameters and prefer interface (not extension) methods in changed calls
- use `string.Empty` instead of `""` in a few tests
dougbu added a commit that referenced this issue Aug 5, 2014
@dougbu
Correct extension class names to match containing files
57f4403 
- #874 line 5
- `EditorExtensions` -> `HtmlHelperEditorExtensions`
- `SelectExtensions` -> `HtmlHelperSelectExtensions`
dougbu added a commit that referenced this issue Aug 6, 2014
@dougbu
Make HTML helper null handling consistent
56d66c0 
- #874 lines 3, 4, and 6
- correct `Value()` to treat a `null` expression name the same as `string.Empty`
- add missing `[NotNull]` attributes in `EditorExtensions` and for `GenerateIdFromName()`
- consistently pass `null` for default expression names to the helpers
 - for example, from extension methods
- add test cases using `null` for expression name

nits:
- correct summary XML comment for `HtmlHelper` class
- use named parameters and prefer interface (not extension) methods in changed calls
- use `string.Empty` instead of `""` in a few tests
dougbu added a commit that referenced this issue Aug 6, 2014
@dougbu
Correct extension class names to match containing files
5bdf795 
- #874 line 5
- `EditorExtensions` -> `HtmlHelperEditorExtensions`
- `SelectExtensions` -> `HtmlHelperSelectExtensions`
@dougbu
Copy link
Member Author

dougbu commented Aug 14, 2014

Fixed in PRs listed above and merged in the following commits (newest on top). Unfortunately a few of the commits mention #847 instead of this bug 😦

@dougbu dougbu closed this as completed Aug 14, 2014
@dougbu dougbu changed the title Correct inconsistencies in IHtmlHelper, HtmlHelper, et cetera Improve IHtmlHelper, HtmlHelper, et cetera public API; correct inconsistencies Aug 14, 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-alpha4
Development

No branches or pull requests
2 participants
@yishaigalatzer @dougbu