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

Add ContentBehaviorAttribute for TagHelpers. #166

Closed

Conversation

NTaylorMullen
Copy link
Contributor

var typeInfo = type.GetTypeInfo();
var contentBehaviorAttribute = typeInfo.GetCustomAttribute<ContentBehaviorAttribute>(inherit: false);

return contentBehaviorAttribute?.ContentBehavior ?? DefaultContentBehavior;
Copy link
Member

Choose a reason for hiding this comment

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

Nice!!

@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from b7749f9 to 5b5111a Compare September 29, 2014 04:03
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 8f15a16 to 78a61d8 Compare September 29, 2014 04:03
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 5b5111a to 03115f9 Compare September 29, 2014 06:33
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 78a61d8 to 7565cd6 Compare September 29, 2014 06:34
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 03115f9 to c39c360 Compare September 29, 2014 21:56
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 7565cd6 to d3b481a Compare September 29, 2014 21:56
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from c39c360 to 41e44b3 Compare September 30, 2014 01:03
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from d3b481a to 55b4148 Compare September 30, 2014 01:03
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 41e44b3 to cabe3dc Compare September 30, 2014 07:00
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 55b4148 to 20762a7 Compare September 30, 2014 07:00
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from cabe3dc to 3e18f61 Compare October 1, 2014 04:14
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 20762a7 to ba8c7eb Compare October 1, 2014 04:15
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 3e18f61 to 42556ce Compare October 1, 2014 18:46
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from ba8c7eb to 2e8616f Compare October 1, 2014 18:46
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 42556ce to 09e5719 Compare October 2, 2014 06:06
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 2e8616f to 7501edc Compare October 2, 2014 06:07
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 09e5719 to 294fa36 Compare October 2, 2014 17:56
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 7501edc to 738104f Compare October 2, 2014 17:56
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 294fa36 to 0a5b27f Compare October 2, 2014 20:04
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 738104f to c08a823 Compare October 2, 2014 20:04
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 0a5b27f to 5355a6e Compare October 3, 2014 08:04
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from c08a823 to 7fef72c Compare October 3, 2014 08:04
namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
{
/// <summary>
/// Used to override <see cref="TagHelper"/>s functionality for when its
Copy link
Member

Choose a reason for hiding this comment

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

/// ... a <see cref="TagHelper"/>'s ... (add a word and apostrophe)

Copy link
Member

Choose a reason for hiding this comment

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

nit: "functionality" isn't used much (and doesn't mean much) and "for" is noise. suggest "behavior" instead of "functionality for".

Copy link
Member

Choose a reason for hiding this comment

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

... or perhaps just /// Used to override when the <see cref="TagHelper"/>'s ... method is called from generated code. I'm not convinced you need "from generated code" but don't mention TagHelperRunner since that's not visible to most.

var typeInfo = type.GetTypeInfo();
var contentBehaviorAttribute = typeInfo.GetCustomAttribute<ContentBehaviorAttribute>(inherit: false);

return contentBehaviorAttribute?.ContentBehavior ?? ContentBehavior.None;
Copy link
Member

Choose a reason for hiding this comment

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

nit: not sure the compiler is smart enough to skip null-checking the ContentBehavior property. either way, this looks like two null checks rather than the one you need. so a ternary expression is more straightforward.

@dougbu
Copy link
Member

dougbu commented Oct 3, 2014

:shipit: after addressing non-nit comments, especially allowing inheritance of the attribute

@NTaylorMullen
Copy link
Contributor Author

Will push this once its dependent PR's have been signed off on.

@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 9d125d2 to 40e755c Compare October 4, 2014 19:53
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from 38a1f50 to ed954ef Compare October 4, 2014 19:53
}

/// <summary>
/// =<see cref="Razor.TagHelpers.ContentBehavior"/> for the <see cref="ITagHelper"/>.
Copy link
Member

Choose a reason for hiding this comment

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

remove leading =

@dougbu
Copy link
Member

dougbu commented Oct 5, 2014

still :shipit: but address one small non-nit

- Added detection of custom ContentBehaviors via the ContentBehaviorAttribute in the TagHelperDescriptorFactory.
- Updated some comments in the ContentBehavior enum.
- Add tests to validate custom content behavior resolution.

#122
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ScopeManager branch from 40e755c to 46a7105 Compare October 5, 2014 21:43
@NTaylorMullen NTaylorMullen force-pushed the TagHelpers_ContentBehavior branch from ed954ef to 4dceae9 Compare October 5, 2014 21:43
@NTaylorMullen NTaylorMullen deleted the TagHelpers_ContentBehavior branch October 9, 2014 19:50
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