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

Update HtmlHelper to use IHtmlGenerator #1348

Closed
wants to merge 8 commits into from

Conversation

dougbu
Copy link
Contributor

@dougbu dougbu commented Oct 13, 2014

nits:

  • add IHtmlGenerator.IdAttributeDotReplacement
  • move DefaultHtmlGenerator.IdAttributeDotReplacement after constructor
  • move HtmlHelper.ActionLink() below static methods
  • move newly-internal methods together in DefaultHtmlGenerator
  • correct placement of DefaultHtmlGenerator.GetValidationAttributes() comment

@dougbu dougbu force-pushed the TagHelpersFeature branch from ca63b8b to 511285d Compare October 14, 2014 04:50
@dougbu dougbu force-pushed the ihtmlgenerator.in.htmlhelper branch from 0647d8c to 2ed038a Compare October 14, 2014 04:59
@dougbu
Copy link
Contributor Author

dougbu commented Oct 14, 2014

rebased on latest TagHelperFeature, which I rebased on origin/dev just a bit ago

@dougbu
Copy link
Contributor Author

dougbu commented Oct 14, 2014

/cc @NTaylorMullen or @pranavkm and @sornaks (who should have some familiarity with the changed HtmlHelper methods)

return HtmlString.Empty;
}

return tagBuilder.ToHtmlString(TagRenderMode.Normal);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: if you think it's cleaner you could do across the board:

return tagBuilder?.ToHtmlString(TagRenderMode.Normal) ?? HtmlString.Empty;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely less clean. also uselessly null-checks the ToHtmlString() return value.

@NTaylorMullen
Copy link

Holy massive change. I have to say it does make me a bit nervous having suchhh a large change set without #453 in.

@dougbu
Copy link
Contributor Author

dougbu commented Oct 14, 2014

we do have a few tests that cover the helpers. I've also checked the MVC sample out...

var byteArrayValue = value as byte[];
if (byteArrayValue != null)
var tagBuilder =
_htmlGenerator.GenerateHidden(ViewContext, metadata, name, value, useViewData, htmlAttributes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you format it so that the parameters are in separate lines (like the MvcForm) above? Makes it seem more consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

…scussion

- lots of new comments in Create.cshtml
- part of #1243 (kind-of)
- mostly copied from `HtmlHelper` but refactored to
 - consistently take a `ViewContext` parameter and return a `TagBuilder`
 - provide `GenerateActionLink()` and `GenerateRouteLink()`
 - provide a separate `GenerateHiddenForCheckBox()`, allowing
   `GenerateCheckBox()` to return a `TagBuilder`
 - `GenerateForm()`'s `method` parameter is a `string`, not `FormMethod`

nits: format document, consistent line wrapping, variable name changes, ...
- rest of #1243 and all of #1244
- needs a few utility methods for copying `TagBuilder` to `TagHelperOutput`
- but works as-is
- add version to Microsoft.AspNet.Mvc.TagHelpers/project.json
- also turn on warnings-as-errors
- enable MVC tag helpers in TagHelperSample.Web
- also fix a couple of issues in TagHelperSample.Web
@dougbu dougbu force-pushed the TagHelpersFeature branch from 511285d to ce38d26 Compare October 14, 2014 21:50
@dougbu dougbu force-pushed the ihtmlgenerator.in.htmlhelper branch from 2ed038a to d576aec Compare October 14, 2014 21:56
@pranavkm
Copy link
Contributor

:shipit:

- make a few more methods available as `internal static` in `DefaultHtmlGenerator`
 - remove `IHtmlGenerator.GenerateOption()`; now `internal static`

nits:
- add `IHtmlGenerator.IdAttributeDotReplacement`
- move `DefaultHtmlGenerator.IdAttributeDotReplacement` after constructor
- move `HtmlHelper.ActionLink()` below static methods
- move newly-`internal` methods together in `DefaultHtmlGenerator`
- correct placement of `DefaultHtmlGenerator.GetValidationAttributes()` comment
@dougbu dougbu force-pushed the ihtmlgenerator.in.htmlhelper branch from 3a30e87 to de5884f Compare October 14, 2014 22:43
@dougbu dougbu force-pushed the TagHelpersFeature branch from ce38d26 to 50f7748 Compare October 14, 2014 22:46
@dougbu
Copy link
Contributor Author

dougbu commented Oct 14, 2014

Checked into TagHelperFeature w/ commit 50f7748

@dougbu dougbu closed this Oct 14, 2014
@dougbu dougbu deleted the ihtmlgenerator.in.htmlhelper branch October 15, 2014 00:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants