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

Add copy helpers for TagHelperOutput from TagBuilder. #1322

Closed
wants to merge 39 commits into from

Conversation

NTaylorMullen
Copy link


namespace Microsoft.AspNet.Mvc.TagHelpers
{
internal class TagHelperOutputHelper
Copy link
Contributor

Choose a reason for hiding this comment

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

leave this public but move it into Microsoft.AspNet.Mvc.TagHelpers.Internals

@dougbu
Copy link
Contributor

dougbu commented Oct 10, 2014

@NTaylorMullen
Copy link
Author

Addressed code review comments. Will wait for rebase to pick up Mvc.sln fix. My VS is in ruins 😢

@@ -0,0 +1,17 @@
{
Copy link
Author

Choose a reason for hiding this comment

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

Add to Mvc.NoFun.sln once the Microsoft.AspNet.Mvc.TagHelpers project is there.

@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_CopyHelpers branch from 5ff3854 to 3b4fe50 Compare October 10, 2014 06:42
@dougbu dougbu force-pushed the taghelper.refactor.1243 branch from 89b19e2 to 42a508d Compare October 10, 2014 19:30
rynowak and others added 11 commits October 10, 2014 13:41
This change includes the basic properties that we're providing for
compatability as well as some functional tests and unit tests that verify
that ApiController can be a controller class.
Adds an options class, as well as a default options setup that will
configure the default set of formatters.

Currently most of what options needs to do is a placeholder, but it later
do things like add ApplicationModelConventions, filters, formatters, model
binders, etc. Those will be added in follow up items.
This change adds ApplicationModel conventions that can enable WebAPI
action conventions (verb mapping) and WebAPI overloading.

The conventions activate when a controller has a marker attribute.
ApiController has this attribute, so any ported code will automatically
opt-in.

Also ported some old tests for action selection to our new functional test
framework.
This change adds a .Request property to the ApiController class that can
be used to access an HttpRequestMessage wrapping the HttpContext.

The HttpRequestMessage is stored in an http feature to make it accessible
to model binders and other infrastructure.
This change adds a ModelBinder that can bind an HttpRequestMessage to an
action parameter.

This builds on an earlier change to construct and store the request
message in the HttpContext via an http feature.
Adds a formatter that can convert an HttpResponseMessage returned from an
action into HttpContext.Response output.
Adds the set of CreateResponse/CreateErrorResponse extension methods that
return an HttpResponseMessage.

For the overloads that perform content negotiation they will access the
collection of MediaTypeFormatters through the shim options. Note that
CreateResponse and friends use the OLD formatters.

Also, HttpError and CreateErrorResponse assume ErrorDetail == false. Using
the shim you will not get detailed error messages unless you construct the
HttpError instance yourself.
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_CopyHelpers branch from 3b4fe50 to b1918ba Compare October 10, 2014 23:39
attribute.Key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)).ToArray();

// Since we're "pulling" the prefixed attribute values we need to remove the attributes that have the given
// prefix from our output.
Copy link
Contributor

Choose a reason for hiding this comment

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

nits:

  • "the attributes that have the given prefix" is (surprisingly) more confusing than "them"
  • comma before "we need"

@dougbu
Copy link
Contributor

dougbu commented Oct 11, 2014

:shipit: (all my comments are nits or less)

@NTaylorMullen NTaylorMullen force-pushed the taghelper.refactor.1243 branch from 42a508d to 11c1354 Compare October 13, 2014 17:52
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_CopyHelpers branch from be24e23 to c175091 Compare October 13, 2014 17:53
@dougbu
Copy link
Contributor

dougbu commented Oct 13, 2014

please close this PR and open one based on TagHelpersFeature

pranavkm and others added 3 commits October 13, 2014 14:34
1) Expose the simplified relative path template by cleaning up constraints, optional and catch all tokens from the template.
2) Expose the parameters on the route template as API parameters.
3) Combine parameters from the route and the action descriptor when the parameter doesn't come from the body. #886 will refine this.
4) Expose optionality and constraints for path parameters. Open question: Should we explicitly expose IsCatchAll?
@NTaylorMullen NTaylorMullen force-pushed the taghelper.refactor.1243 branch from 11c1354 to ff2f78d Compare October 14, 2014 00:42
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_CopyHelpers branch from c175091 to e0a0206 Compare October 14, 2014 00:43
@NTaylorMullen
Copy link
Author

Going to hold off on rebasing onto TagHelpersFeature until your textarea.tests PR is in. The other PR's in the chain starting with this PR point to taghelper.refactor.1243 which we forced to be a copy of your textarea.tests branch.

pranavkm and others added 12 commits October 14, 2014 12:22
…data to Enum parameter.

Fix: Using TypeConverter solves this problem.
-Issue #1123 - TypeConverterModelBinder cannot bind "byte" and "short".
Fix: Modified code to use TypeConverter which can handle these scenarios.
-Removing the GetConverterDelegate method and making the code similar to the WebApi.
…ing, PathString, etc. To build Urls.

Added tests to describe the current behaviour with Unicode hosts.
- 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, ...
…ple.Web

- also fix a couple of minor issues in TagHelperSample.Web
- rest of #1243 and all of #1244
- needs a few utility methods for copying `TagBuilder` to `TagHelperOutput`
 - but works as-is
- new Microsoft.AspNet.Mvc.TagHelpers.Test project
- bit of test infrastructure -- `TestableHtmlGenerator`
 - short-curcuits validation attribute and AntiForgery additions
 - should help when testing all MVC tag helpers
- 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
- Tested out Merge and MergeAttributes.

#1319
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_CopyHelpers branch from e0a0206 to 199a8e2 Compare October 15, 2014 00:30
@NTaylorMullen
Copy link
Author

Closing so I can reopen against TagHelpersFeature.

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.

9 participants