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

Add IHtmlGenerator and TextAreaTagHelper #1309

Closed
wants to merge 7 commits into from

Conversation

dougbu
Copy link
Contributor

@dougbu dougbu commented Oct 9, 2014

@dougbu dougbu changed the title Add IHtmlGenerator and its default implementation [Design] Add IHtmlGenerator and its default implementation Oct 9, 2014
@dougbu
Copy link
Contributor Author

dougbu commented Oct 9, 2014

/cc @NTaylorMullen

  • "[Design]" PR primarily because HtmlHelper doesn't yet use an IHtmlGenerator
  • missing anti-forgery support in IHtmlGenerator
  • will likely file a Task to add missing XML docs (mostly copies from IHtmlHelper w/ slight edits)

@NTaylorMullen NTaylorMullen force-pushed the TagHelpersFeature branch 2 times, most recently from d948242 to b68fae9 Compare October 9, 2014 19:52
@dougbu dougbu force-pushed the taghelper.refactor.1243 branch from 045a127 to 2535a4b Compare October 9, 2014 20:28
@dougbu dougbu force-pushed the taghelper.refactor.1243 branch from 2535a4b to d1bf693 Compare October 9, 2014 22:14
@NTaylorMullen
Copy link

👍 should I just inject the AntiForgery class for now? Assuming you'll add the MvcServices DI registration piece in the non-design piece

@dougbu
Copy link
Contributor Author

dougbu commented Oct 9, 2014

  • I just pushed the new project outline and one tag helper
  • in next update, will add DI registration and IAntiForgery -- that should be done consistently
  • saving HtmlHelper changes 'til last since you don't depend on that
  • what "non-design piece"? this is all runtime code

@@ -0,0 +1,15 @@
{

Choose a reason for hiding this comment

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

Add a version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌

Choose a reason for hiding this comment

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

Also, add this project to the Mvc.NoFun.sln 😄 VS exploding at home more than I like haha

@dougbu
Copy link
Contributor Author

dougbu commented Oct 10, 2014

@natemcmaster updated. haven't added project.json version yet but have added anti-forgery.

/cc @harshgMSFT @GrabYourPitchforks please take a quick look at change to AntiForgery.GetHtml() signature. any issues w/ returning TagBuilder and not HtmlString?

dougbu added 3 commits October 9, 2014 23:05
- 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
@GrabYourPitchforks
Copy link
Contributor

I have no concerns re: the signature change.

- add version to Microsoft.AspNet.Mvc.TagHelpers/project.json
- also turn on warnings-as-errors
@dougbu dougbu changed the title [Design] Add IHtmlGenerator and its default implementation Add IHtmlGenerator and TextAreaTagHelper Oct 10, 2014
- enable MVC tag helpers in TagHelperSample.Web
- also fix a couple of issues in TagHelperSample.Web
@dougbu dougbu force-pushed the taghelper.refactor.1243 branch from 89b19e2 to 42a508d Compare October 10, 2014 19:30
@dougbu
Copy link
Contributor Author

dougbu commented Oct 10, 2014

  • updated commits, PR description, and title
  • rebased and checked into TagHelpersFeature branch
  • leaving this open for additional issues and because <textarea/> tag helper needs unit tests

@dougbu
Copy link
Contributor Author

dougbu commented Oct 13, 2014

Will open a new PR to complete this work later today...

@dougbu dougbu closed this Oct 13, 2014
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