Skip to content

Support for 'multiple' attribute in '<select>' elements. #33950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Jul 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
59c7deb
Support for multiple select in onchanged callback.
MackinnonBuck Jun 26, 2021
821b629
Prototype for array binding.
MackinnonBuck Jun 29, 2021
983f092
InputSelect support for the "multiple" attribute.
MackinnonBuck Jun 29, 2021
aa48d16
Some cleanup and improvements.
MackinnonBuck Jun 29, 2021
488e7e0
E2E tests for select element with 'multiple' attribute.
MackinnonBuck Jun 30, 2021
fdc8904
E2E test for InputSelect with 'multiple' attribute.
MackinnonBuck Jun 30, 2021
7ea73ce
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jun 30, 2021
78f1235
Fixed reflection analysis errors.
MackinnonBuck Jun 30, 2021
dfb1ea4
Add accidentally removed .gitattributes file.
MackinnonBuck Jun 30, 2021
d6a3a3c
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jun 30, 2021
6012d95
PR feedback
MackinnonBuck Jun 30, 2021
ccb61f7
PR feedback
MackinnonBuck Jul 1, 2021
6058d36
InputSelect infers the 'multiple' attribute
MackinnonBuck Jul 1, 2021
3763a26
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jul 1, 2021
9e69e2e
More PR feedback
MackinnonBuck Jul 1, 2021
55cd443
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jul 1, 2021
08aca0f
Apply CultureInfo to array formatting.
MackinnonBuck Jul 1, 2021
1b8b5d5
Added isMultipleSelect helper.
MackinnonBuck Jul 2, 2021
8d0b8bf
JSON encode array elements in BindConverter.
MackinnonBuck Jul 2, 2021
17ede0e
Create .gitattributes
MackinnonBuck Jul 2, 2021
dc49266
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jul 2, 2021
204662f
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jul 6, 2021
5f27f9a
Merge branch 'main' into t-mbuck/select-multiple-support
MackinnonBuck Jul 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Components/Components.slnf
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@
"src\\Http\\Routing.Abstractions\\src\\Microsoft.AspNetCore.Routing.Abstractions.csproj",
"src\\Http\\Routing\\src\\Microsoft.AspNetCore.Routing.csproj",
"src\\Http\\WebUtilities\\src\\Microsoft.AspNetCore.WebUtilities.csproj",
"src\\Identity\\ApiAuthorization.IdentityServer\\src\\Microsoft.AspNetCore.ApiAuthorization.IdentityServer.csproj",
"src\\Identity\\EntityFrameworkCore\\src\\Microsoft.AspNetCore.Identity.EntityFrameworkCore.csproj",
"src\\Identity\\UI\\src\\Microsoft.AspNetCore.Identity.UI.csproj",
"src\\JSInterop\\Microsoft.JSInterop\\src\\Microsoft.JSInterop.csproj",
"src\\Middleware\\CORS\\src\\Microsoft.AspNetCore.Cors.csproj",
"src\\Middleware\\Diagnostics.Abstractions\\src\\Microsoft.AspNetCore.Diagnostics.Abstractions.csproj",
Expand Down Expand Up @@ -96,8 +99,10 @@
"src\\Mvc\\Mvc\\src\\Microsoft.AspNetCore.Mvc.csproj",
"src\\Razor\\Razor.Runtime\\src\\Microsoft.AspNetCore.Razor.Runtime.csproj",
"src\\Razor\\Razor\\src\\Microsoft.AspNetCore.Razor.csproj",
"src\\Security\\Authentication\\Cookies\\src\\Microsoft.AspNetCore.Authentication.Cookies.csproj",
"src\\Security\\Authorization\\Core\\src\\Microsoft.AspNetCore.Authorization.csproj",
"src\\Security\\Authorization\\Policy\\src\\Microsoft.AspNetCore.Authorization.Policy.csproj",
"src\\Security\\CookiePolicy\\src\\Microsoft.AspNetCore.CookiePolicy.csproj",
"src\\Servers\\Connections.Abstractions\\src\\Microsoft.AspNetCore.Connections.Abstractions.csproj",
"src\\Servers\\IIS\\IISIntegration\\src\\Microsoft.AspNetCore.Server.IISIntegration.csproj",
"src\\Servers\\IIS\\IIS\\src\\Microsoft.AspNetCore.Server.IIS.csproj",
Expand Down
99 changes: 94 additions & 5 deletions src/Components/Components/src/BindConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Reflection;
using System.Text;
using System.Text.Json;

namespace Microsoft.AspNetCore.Components
{
Expand Down Expand Up @@ -523,7 +525,7 @@ public static bool TryConvertToString(object? obj, CultureInfo? culture, out str
return ConvertToStringCore(obj, culture, out value);
}

internal readonly static BindParser<string?> ConvertToString = ConvertToStringCore;
internal static readonly BindParser<string?> ConvertToString = ConvertToStringCore;

private static bool ConvertToStringCore(object? obj, CultureInfo? culture, out string? value)
{
Expand Down Expand Up @@ -556,8 +558,8 @@ public static bool TryConvertToNullableBool(object? obj, CultureInfo? culture, o
return ConvertToNullableBoolCore(obj, culture, out value);
}

internal readonly static BindParser<bool> ConvertToBool = ConvertToBoolCore;
internal readonly static BindParser<bool?> ConvertToNullableBool = ConvertToNullableBoolCore;
internal static readonly BindParser<bool> ConvertToBool = ConvertToBoolCore;
internal static readonly BindParser<bool?> ConvertToNullableBool = ConvertToNullableBoolCore;

private static bool ConvertToBoolCore(object? obj, CultureInfo? culture, out bool value)
{
Expand Down Expand Up @@ -1278,8 +1280,18 @@ private static bool ConvertToNullableEnum<T>(object? obj, CultureInfo? culture,

private static class FormatterDelegateCache
{
private readonly static ConcurrentDictionary<Type, Delegate> _cache = new ConcurrentDictionary<Type, Delegate>();
private static readonly ConcurrentDictionary<Type, Delegate> _cache = new ConcurrentDictionary<Type, Delegate>();

private static MethodInfo? _makeArrayFormatter;

[UnconditionalSuppressMessage(
"ReflectionAnalysis",
"IL2060:MakeGenericMethod",
Justification = "The referenced methods don't have any DynamicallyAccessedMembers annotations. See https://github.com/mono/linker/issues/1727")]
[UnconditionalSuppressMessage(
"ReflectionAnalysis",
"IL2075:MakeGenericMethod",
Justification = "The referenced methods don't have any DynamicallyAccessedMembers annotations. See https://github.com/mono/linker/issues/1727")]
public static BindFormatter<T> Get<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>()
{
if (!_cache.TryGetValue(typeof(T), out var formatter))
Expand Down Expand Up @@ -1366,6 +1378,12 @@ private static class FormatterDelegateCache
{
formatter = (BindFormatter<T>)FormatEnumValueCore<T>;
}
else if (typeof(T).IsArray)
{
var method = _makeArrayFormatter ??= typeof(FormatterDelegateCache).GetMethod(nameof(MakeArrayFormatter), BindingFlags.NonPublic | BindingFlags.Static)!;
var elementType = typeof(T).GetElementType()!;
formatter = (Delegate)method.MakeGenericMethod(elementType).Invoke(null, null)!;
}
else
{
formatter = MakeTypeConverterFormatter<T>();
Expand All @@ -1377,6 +1395,36 @@ private static class FormatterDelegateCache
return (BindFormatter<T>)formatter;
}

private static BindFormatter<T[]> MakeArrayFormatter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>()
{
var elementFormatter = Get<T>();

return FormatArrayValue;

string? FormatArrayValue(T[] value, CultureInfo? culture)
{
if (value.Length == 0)
{
return "[]";
}

var builder = new StringBuilder("[\"");
builder.Append(JsonEncodedText.Encode(elementFormatter(value[0], culture)?.ToString() ?? string.Empty));
builder.Append('\"');

for (var i = 1; i < value.Length; i++)
{
builder.Append(", \"");
builder.Append(JsonEncodedText.Encode(elementFormatter(value[i], culture)?.ToString() ?? string.Empty));
builder.Append('\"');
}

builder.Append(']');

return builder.ToString();
}
}

private static BindFormatter<T> MakeTypeConverterFormatter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>()
{
var typeConverter = TypeDescriptor.GetConverter(typeof(T));
Expand All @@ -1400,15 +1448,20 @@ string FormatWithTypeConverter(T value, CultureInfo? culture)

internal static class ParserDelegateCache
{
private readonly static ConcurrentDictionary<Type, Delegate> _cache = new ConcurrentDictionary<Type, Delegate>();
private static readonly ConcurrentDictionary<Type, Delegate> _cache = new ConcurrentDictionary<Type, Delegate>();

private static MethodInfo? _convertToEnum;
private static MethodInfo? _convertToNullableEnum;
private static MethodInfo? _makeArrayTypeConverter;

[UnconditionalSuppressMessage(
"ReflectionAnalysis",
"IL2060:MakeGenericMethod",
Justification = "The referenced methods don't have any DynamicallyAccessedMembers annotations. See https://github.com/mono/linker/issues/1727")]
[UnconditionalSuppressMessage(
"ReflectionAnalysis",
"IL2075:MakeGenericMethod",
Justification = "The referenced methods don't have any DynamicallyAccessedMembers annotations. See https://github.com/mono/linker/issues/1727")]
public static BindParser<T> Get<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>()
{
if (!_cache.TryGetValue(typeof(T), out var parser))
Expand Down Expand Up @@ -1503,6 +1556,12 @@ internal static class ParserDelegateCache
var method = _convertToNullableEnum ??= typeof(BindConverter).GetMethod(nameof(ConvertToNullableEnum), BindingFlags.NonPublic | BindingFlags.Static)!;
parser = method.MakeGenericMethod(innerType).CreateDelegate(typeof(BindParser<T>), target: null);
}
else if (typeof(T).IsArray)
{
var method = _makeArrayTypeConverter ??= typeof(ParserDelegateCache).GetMethod(nameof(MakeArrayTypeConverter), BindingFlags.NonPublic | BindingFlags.Static)!;
var elementType = typeof(T).GetElementType()!;
parser = (Delegate)method.MakeGenericMethod(elementType).Invoke(null, null)!;
}
else
{
parser = MakeTypeConverterConverter<T>();
Expand All @@ -1514,6 +1573,36 @@ internal static class ParserDelegateCache
return (BindParser<T>)parser;
}

private static BindParser<T[]?> MakeArrayTypeConverter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>()
{
var elementParser = Get<T>();

return ConvertToArray;

bool ConvertToArray(object? obj, CultureInfo? culture, out T[]? value)
{
if (obj is not Array initialArray)
{
value = default;
return false;
}

var convertedArray = new T[initialArray.Length];

for (var i = 0; i < initialArray.Length; i++)
{
if (!elementParser(initialArray.GetValue(i), culture, out convertedArray[i]!))
{
value = default;
return false;
}
}

value = convertedArray;
return true;
}
}

private static BindParser<T> MakeTypeConverterConverter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>()
{
var typeConverter = TypeDescriptor.GetConverter(typeof(T));
Expand Down
25 changes: 25 additions & 0 deletions src/Components/Shared/src/WebEventData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,36 @@ private static ChangeEventArgs DeserializeChangeEventArgs(string eventArgsJson,
case JsonValueKind.False:
changeArgs.Value = jsonElement.GetBoolean();
break;
case JsonValueKind.Array:
changeArgs.Value = GetJsonElementStringArrayValue(jsonElement);
break;
default:
throw new ArgumentException($"Unsupported {nameof(ChangeEventArgs)} value {jsonElement}.");
}

return changeArgs;
}

private static string?[] GetJsonElementStringArrayValue(JsonElement jsonElement)
{
var result = new string?[jsonElement.GetArrayLength()];
var elementIndex = 0;

foreach (var arrayElement in jsonElement.EnumerateArray())
{
if (arrayElement.ValueKind != JsonValueKind.String)
{
throw new InvalidOperationException(
$"Unsupported {nameof(JsonElement)} value kind '{arrayElement.ValueKind}' " +
$"(expected '{JsonValueKind.String}').");
}

result[elementIndex] = arrayElement.GetString();
elementIndex++;
}

return result;
}
}

[JsonSerializable(typeof(WebEventDescriptor))]
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.server.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.webview.js

Large diffs are not rendered by default.

70 changes: 52 additions & 18 deletions src/Components/Web.JS/src/Rendering/BrowserRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,19 +267,29 @@ export class BrowserRenderer {
this.trySetSelectValueFromOptionElement(newDomElementRaw);
} else if (deferredValuePropname in newDomElementRaw) {
// Situation 2
const deferredValue: string | null = newDomElementRaw[deferredValuePropname];
setDeferredElementValue(newDomElementRaw, deferredValue);
setDeferredElementValue(newDomElementRaw, newDomElementRaw[deferredValuePropname]);
}
}

private trySetSelectValueFromOptionElement(optionElement: HTMLOptionElement) {
const selectElem = this.findClosestAncestorSelectElement(optionElement);
if (selectElem && (deferredValuePropname in selectElem) && selectElem[deferredValuePropname] === optionElement.value) {
setDeferredElementValue(selectElem, optionElement.value);

if (!selectElem || !(deferredValuePropname in selectElem)) {
return false;
}

if (isMultipleSelectElement(selectElem)) {
optionElement.selected = selectElem[deferredValuePropname].indexOf(optionElement.value) !== -1;
} else {
if (selectElem[deferredValuePropname] !== optionElement.value) {
return false;
}

setSingleSelectElementValue(selectElem, optionElement.value);
delete selectElem[deferredValuePropname];
return true;
}
return false;

return true;
}

private insertComponent(batch: RenderBatch, parent: LogicalElement, childIndex: number, frame: RenderTreeFrame) {
Expand Down Expand Up @@ -378,7 +388,7 @@ export class BrowserRenderer {
case 'INPUT':
case 'SELECT':
case 'TEXTAREA': {
const value = attributeFrame ? frameReader.attributeValue(attributeFrame) : null;
let value = attributeFrame ? frameReader.attributeValue(attributeFrame) : null;

// <select> is special, in that anything we write to .value will be lost if there
// isn't yet a matching <option>. To maintain the expected behavior no matter the
Expand All @@ -390,8 +400,13 @@ export class BrowserRenderer {
// default attribute values that may incorrectly constain the specified 'value'.
// For example, range inputs have default 'min' and 'max' attributes that may incorrectly
// clamp the 'value' property if it is applied before custom 'min' and 'max' attributes.
element[deferredValuePropname] = value;

if (value && element instanceof HTMLSelectElement && isMultipleSelectElement(element)) {
value = JSON.parse(value);
}

setDeferredElementValue(element, value);
element[deferredValuePropname] = value;

return true;
}
Expand Down Expand Up @@ -515,17 +530,36 @@ function stripOnPrefix(attributeName: string) {
throw new Error(`Attribute should be an event name, but doesn't start with 'on'. Value: '${attributeName}'`);
}

function setDeferredElementValue(element: Element, value: string | null) {
function isMultipleSelectElement(element: HTMLSelectElement) {
return element.type === 'select-multiple';
}

function setSingleSelectElementValue(element: HTMLSelectElement, value: string | null) {
// There's no sensible way to represent a select option with value 'null', because
// (1) HTML attributes can't have null values - the closest equivalent is absence of the attribute
// (2) When picking an <option> with no 'value' attribute, the browser treats the value as being the
// *text content* on that <option> element. Trying to suppress that default behavior would involve
// a long chain of special-case hacks, as well as being breaking vs 3.x.
// So, the most plausible 'null' equivalent is an empty string. It's unfortunate that people can't
// write <option value=@someNullVariable>, and that we can never distinguish between null and empty
// string in a bound <select>, but that's a limit in the representational power of HTML.
element.value = value || '';
}

function setMultipleSelectElementValue(element: HTMLSelectElement, value: string[] | null) {
value ||= [];
for (let i = 0; i < element.options.length; i++) {
element.options[i].selected = value.indexOf(element.options[i].value) !== -1;
}
}

function setDeferredElementValue(element: Element, value: any) {
if (element instanceof HTMLSelectElement) {
// There's no sensible way to represent a select option with value 'null', because
// (1) HTML attributes can't have null values - the closest equivalent is absence of the attribute
// (2) When picking an <option> with no 'value' attribute, the browser treats the value as being the
// *text content* on that <option> element. Trying to suppress that default behavior would involve
// a long chain of special-case hacks, as well as being breaking vs 3.x.
// So, the most plausible 'null' equivalent is an empty string. It's unfortunate that people can't
// write <option value=@someNullVariable>, and that we can never distinguish between null and empty
// string in a bound <select>, but that's a limit in the representational power of HTML.
element.value = value || '';
if (isMultipleSelectElement(element)) {
setMultipleSelectElementValue(element, value);
} else {
setSingleSelectElementValue(element, value);
}
} else {
(element as any).value = value;
}
Expand Down
12 changes: 11 additions & 1 deletion src/Components/Web.JS/src/Rendering/Events/EventTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ function parseChangeEvent(event: Event): ChangeEventArgs {
if (isTimeBasedInput(element)) {
const normalizedValue = normalizeTimeBasedValue(element);
return { value: normalizedValue };
} else if (isMultipleSelectInput(element)) {
const selectElement = element as HTMLSelectElement;
const selectedValues = Array.from(selectElement.options)
.filter(option => option.selected)
.map(option => option.value);
return { value: selectedValues };
} else {
const targetIsCheckbox = isCheckbox(element);
const newValue = targetIsCheckbox ? !!element['checked'] : element['value'];
Expand Down Expand Up @@ -243,6 +249,10 @@ function isTimeBasedInput(element: Element): element is HTMLInputElement {
return timeBasedInputs.indexOf(element.getAttribute('type')!) !== -1;
}

function isMultipleSelectInput(element: Element): element is HTMLSelectElement {
return element instanceof HTMLSelectElement && element.type === 'select-multiple';
}

function normalizeTimeBasedValue(element: HTMLInputElement): string {
const value = element.value;
const type = element.type;
Expand All @@ -264,7 +274,7 @@ function normalizeTimeBasedValue(element: HTMLInputElement): string {
// The following interfaces must be kept in sync with the EventArgs C# classes

interface ChangeEventArgs {
value: string | boolean;
value: string | boolean | string[];
}

interface DragEventArgs {
Expand Down
Loading