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

Commit

Permalink
Addressed code review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
NTaylorMullen committed Oct 22, 2015
1 parent f2365b5 commit 2089f0c
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 22 deletions.
6 changes: 2 additions & 4 deletions src/Microsoft.AspNet.Mvc.TagHelpers/OptionTagHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public override int Order
/// <inheritdoc />
/// <remarks>
/// Does nothing unless <see cref="TagHelperContext.Items"/> contains a
/// <see cref="SelectTagHelper.SelectedValuesFormDataKey"/> entry and that entry is a non-empty
/// <see cref="SelectTagHelper"/> <see cref="Type"/> entry and that entry is a non-empty
/// <see cref="ICollection{string}"/> instance. Also does nothing if the associated &lt;option&gt; is already
/// selected.
/// </remarks>
Expand All @@ -82,9 +82,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
{
// Is this <option/> element a child of a <select/> element the SelectTagHelper targeted?
object formDataEntry;
context.Items.TryGetValue(
SelectTagHelper.SelectedValuesFormDataKey,
out formDataEntry);
context.Items.TryGetValue(typeof(SelectTagHelper), out formDataEntry);

// ... And did the SelectTagHelper determine any selected values?
var selectedValues = formDataEntry as ICollection<string>;
Expand Down
12 changes: 1 addition & 11 deletions src/Microsoft.AspNet.Mvc.TagHelpers/SelectTagHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,6 @@ public class SelectTagHelper : TagHelper
private const string ForAttributeName = "asp-for";
private const string ItemsAttributeName = "asp-items";

/// <summary>
/// Key used for selected values in <see cref="TagHelperContext.Items"/>.
/// </summary>
/// <remarks>
/// Value for this dictionary entry will either be <c>null</c> (indicating no <see cref="SelectTagHelper"/> has
/// executed within this &lt;form/&gt;) or an <see cref="ICollection{string}"/> instance. Elements of the
/// collection are based on current <see cref="ViewDataDictionary.Model"/>.
/// </remarks>
public static readonly string SelectedValuesFormDataKey = nameof(SelectTagHelper) + "-SelectedValues";

/// <summary>
/// Creates a new <see cref="SelectTagHelper"/>.
/// </summary>
Expand Down Expand Up @@ -131,7 +121,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output)

// Whether or not (not being highly unlikely) we generate anything, could update contained <option/>
// elements. Provide selected values for <option/> tag helpers. They'll run next.
context.Items[SelectedValuesFormDataKey] = currentValues;
context.Items[typeof(SelectTagHelper)] = currentValues;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public async Task ProcessAsync_GeneratesExpectedOutput(
model: null,
htmlGenerator: htmlGenerator,
metadataProvider: metadataProvider);
tagHelperContext.Items[SelectTagHelper.SelectedValuesFormDataKey] = selectedValues;
tagHelperContext.Items[typeof(SelectTagHelper)] = selectedValues;
var tagHelper = new OptionTagHelper(htmlGenerator)
{
Value = value,
Expand Down Expand Up @@ -491,7 +491,7 @@ public async Task ProcessAsync_DoesNotUseGenerator_IfSelectedNullOrNoSelectedVal
model: null,
htmlGenerator: htmlGenerator,
metadataProvider: metadataProvider);
tagHelperContext.Items[SelectTagHelper.SelectedValuesFormDataKey] = selectedValues;
tagHelperContext.Items[typeof(SelectTagHelper)] = selectedValues;
var tagHelper = new OptionTagHelper(htmlGenerator)
{
Value = value,
Expand Down
10 changes: 5 additions & 5 deletions test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public async Task ProcessAsync_GeneratesExpectedOutput(

Assert.Single(
tagHelperContext.Items,
entry => (string)entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
entry => (Type)entry.Key == typeof(SelectTagHelper));
}

[Theory]
Expand Down Expand Up @@ -344,7 +344,7 @@ public async Task ProcessAsync_WithItems_GeneratesExpectedOutput_DoesNotChangeSe

Assert.Single(
tagHelperContext.Items,
entry => (string)entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
entry => (Type)entry.Key == typeof(SelectTagHelper));

Assert.Equal(savedDisabled, items.Select(item => item.Disabled));
Assert.Equal(savedGroup, items.Select(item => item.Group));
Expand Down Expand Up @@ -448,7 +448,7 @@ public async Task ProcessAsyncInTemplate_WithItems_GeneratesExpectedOutput_DoesN

Assert.Single(
tagHelperContext.Items,
entry => (string)entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
entry => (Type)entry.Key == typeof(SelectTagHelper));

Assert.Equal(savedDisabled, items.Select(item => item.Disabled));
Assert.Equal(savedGroup, items.Select(item => item.Group));
Expand Down Expand Up @@ -540,7 +540,7 @@ public async Task ProcessAsync_CallsGeneratorWithExpectedValues_ItemsAndAttribut

var keyValuePair = Assert.Single(
tagHelperContext.Items,
entry => (string)entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
entry => (Type)entry.Key == typeof(SelectTagHelper));
Assert.Same(currentValues, keyValuePair.Value);
}

Expand Down Expand Up @@ -613,7 +613,7 @@ public async Task TagHelper_CallsGeneratorWithExpectedValues_RealModelType(

var keyValuePair = Assert.Single(
tagHelperContext.Items,
entry => (string)entry.Key == SelectTagHelper.SelectedValuesFormDataKey);
entry => (Type)entry.Key == typeof(SelectTagHelper));
Assert.Same(currentValues, keyValuePair.Value);
}

Expand Down

0 comments on commit 2089f0c

Please sign in to comment.