From 8b39961ab3c4ca56c29c4e0ad6d91eebe022c805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Tue, 7 May 2024 20:06:32 +0200 Subject: [PATCH 1/2] Fix SelectorItem with boolean value * broken JS coercion from 'false' to boolean fixed * SelectorItems renders all non-string values as `data-bind='value: value'`. Knockout.js then reads the value binding instead of the value attribute, so we avoid the roundtrip through strings resolves #1812 --- .../Framework/Binding/ValueOrBinding.cs | 9 +++ .../Framework/Controls/SelectorItem.cs | 11 +++- .../Scripts/metadata/primitiveTypes.ts | 2 +- .../Resources/Scripts/tests/coercer.test.ts | 22 ++++++- .../ComboBox/ComboBoxBooleanViewModel.cs | 11 ++++ .../ComboBox/BooleanProperty.dothtml | 60 +++++++++++++++++++ .../Abstractions/SamplesRouteUrls.designer.cs | 1 + .../Tests/Tests/Control/ComboBoxTests.cs | 27 +++++++++ 8 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 src/Samples/Common/ViewModels/ControlSamples/ComboBox/ComboBoxBooleanViewModel.cs create mode 100644 src/Samples/Common/Views/ControlSamples/ComboBox/BooleanProperty.dothtml diff --git a/src/Framework/Framework/Binding/ValueOrBinding.cs b/src/Framework/Framework/Binding/ValueOrBinding.cs index 82814c2805..84d36e99d3 100644 --- a/src/Framework/Framework/Binding/ValueOrBinding.cs +++ b/src/Framework/Framework/Binding/ValueOrBinding.cs @@ -160,6 +160,15 @@ public TResult ProcessValueBinding(DotvvmBindableObject control, Func If this contains a `resource` binding, it is evaluated and its value placed in property. `value`, and all other bindings are untouched and remain in the property. + public ValueOrBinding EvaluateResourceBinding(DotvvmBindableObject control) + { + if (binding is null or IValueBinding or not IStaticValueBinding) return this; + + var value = this.Evaluate(control); + return new ValueOrBinding(value); + } + public static explicit operator ValueOrBinding(T val) => new ValueOrBinding(val); public const string EqualsDisabledReason = "Equals is disabled on ValueOrBinding as it may lead to unexpected behavior. Please use object.ReferenceEquals for reference comparison or evaluate the ValueOrBinding and compare the value. Or use IsNull/NotNull for nullchecks on bindings."; diff --git a/src/Framework/Framework/Controls/SelectorItem.cs b/src/Framework/Framework/Controls/SelectorItem.cs index 962e701b1e..800e85a665 100644 --- a/src/Framework/Framework/Controls/SelectorItem.cs +++ b/src/Framework/Framework/Controls/SelectorItem.cs @@ -51,7 +51,16 @@ public SelectorItem(ValueOrBinding text, ValueOrBinding value) protected override void AddAttributesToRender(IHtmlWriter writer, IDotvvmRequestContext context) { - writer.AddAttribute("value", Value + ""); + var value = this.GetValueOrBinding(ValueProperty).EvaluateResourceBinding(this); + if (value.ValueOrDefault is string s) + { + writer.AddAttribute("value", s); + } + else + { + // anything else than string is better to pass as knockout value binding to avoid issues with `false != 'false'`, ... + writer.AddKnockoutDataBind("value", value.GetJsExpression(this)); + } base.AddAttributesToRender(writer, context); } diff --git a/src/Framework/Framework/Resources/Scripts/metadata/primitiveTypes.ts b/src/Framework/Framework/Resources/Scripts/metadata/primitiveTypes.ts index 88dc7e0577..6ca1c11fc8 100644 --- a/src/Framework/Framework/Resources/Scripts/metadata/primitiveTypes.ts +++ b/src/Framework/Framework/Resources/Scripts/metadata/primitiveTypes.ts @@ -26,7 +26,7 @@ export const primitiveTypes: PrimitiveTypes = { } else if (value === "true" || value === "True") { return { value: true, wasCoerced: true }; } else if (value === "false" || value === "False") { - return { value: true, wasCoerced: true }; + return { value: false, wasCoerced: true }; } else if (typeof value === "number") { return { value: !!value, wasCoerced: true }; } diff --git a/src/Framework/Framework/Resources/Scripts/tests/coercer.test.ts b/src/Framework/Framework/Resources/Scripts/tests/coercer.test.ts index 81e252ddef..41712b5e26 100644 --- a/src/Framework/Framework/Resources/Scripts/tests/coercer.test.ts +++ b/src/Framework/Framework/Resources/Scripts/tests/coercer.test.ts @@ -449,12 +449,32 @@ test("boolean - valid, converted from number", () => { expect(result.value).toEqual(true); }) -test("boolean - valid, converted from string", () => { +test("boolean - valid, true converted from string", () => { const result = tryCoerce("true", "Boolean"); expect(result.wasCoerced).toBeTruthy(); expect(result.value).toEqual(true); }) +test("boolean - valid, false converted from string", () => { + const result = tryCoerce("false", "Boolean"); + expect(result.wasCoerced).toBeTruthy(); + expect(result.value).toEqual(false); +}) +test("boolean - valid, False converted from string", () => { + const result = tryCoerce("False", "Boolean"); + expect(result.wasCoerced).toBeTruthy(); + expect(result.value).toEqual(false); +}) + +test("boolean - invalid, invalid string", () => { + const result = tryCoerce("", "Boolean"); + expect(result.isError).toBeTruthy(); +}) +test("boolean - invalid, invalid string", () => { + const result = tryCoerce("bazmek", "Boolean"); + expect(result.isError).toBeTruthy(); +}) + test("boolean - invalid, null", () => { const result = tryCoerce(null, "Boolean"); expect(result.isError).toBeTruthy(); diff --git a/src/Samples/Common/ViewModels/ControlSamples/ComboBox/ComboBoxBooleanViewModel.cs b/src/Samples/Common/ViewModels/ControlSamples/ComboBox/ComboBoxBooleanViewModel.cs new file mode 100644 index 0000000000..0cdca6115b --- /dev/null +++ b/src/Samples/Common/ViewModels/ControlSamples/ComboBox/ComboBoxBooleanViewModel.cs @@ -0,0 +1,11 @@ +namespace DotVVM.Samples.BasicSamples.ViewModels.ControlSamples.ComboBox +{ + public class ComboBoxBooleanViewModel + { + public bool? NullableSelectedValue { get; set; } + public bool NonNullableSelectedValue { get; set; } + + public bool[] Items { get; } = new bool[] { true, false }; + public bool?[] NullableItems { get; } = new bool?[] { true, false, null }; + } +} diff --git a/src/Samples/Common/Views/ControlSamples/ComboBox/BooleanProperty.dothtml b/src/Samples/Common/Views/ControlSamples/ComboBox/BooleanProperty.dothtml new file mode 100644 index 0000000000..2a4f601a61 --- /dev/null +++ b/src/Samples/Common/Views/ControlSamples/ComboBox/BooleanProperty.dothtml @@ -0,0 +1,60 @@ +@viewModel DotVVM.Samples.BasicSamples.ViewModels.ControlSamples.ComboBox.ComboBoxBooleanViewModel + + + + + Hello from DotVVM! + + + + +

+ Demonstrates ComboBox working when bound to bool or bool?. Each table column should display the same value. +

+ + + + + + + + + + + + + + + + + + + + + +
Nullable booleanNon-nullable boolean
Current values + {{value: NullableSelectedValue == null ? "null" : NullableSelectedValue}} + + {{value: NonNullableSelectedValue.ToString()}} +
DataSource + + + +
Hardcoded items + + + + + + + + + + +
+ + diff --git a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs index 6b287d9ae9..eb5392fe03 100644 --- a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs +++ b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs @@ -56,6 +56,7 @@ public partial class SamplesRouteUrls public const string ControlSamples_CheckBox_InRepeater = "ControlSamples/CheckBox/InRepeater"; public const string ControlSamples_ClaimView_ClaimViewTest = "ControlSamples/ClaimView/ClaimViewTest"; public const string ControlSamples_ComboBox_BindingCTValidation_StringToEnum = "ControlSamples/ComboBox/BindingCTValidation_StringToEnum"; + public const string ControlSamples_ComboBox_BooleanProperty = "ControlSamples/ComboBox/BooleanProperty"; public const string ControlSamples_ComboBox_Default = "ControlSamples/ComboBox/Default"; public const string ControlSamples_ComboBox_DelaySync = "ControlSamples/ComboBox/DelaySync"; public const string ControlSamples_ComboBox_DelaySync2 = "ControlSamples/ComboBox/DelaySync2"; diff --git a/src/Samples/Tests/Tests/Control/ComboBoxTests.cs b/src/Samples/Tests/Tests/Control/ComboBoxTests.cs index 8ff279f445..b9f08e5c25 100644 --- a/src/Samples/Tests/Tests/Control/ComboBoxTests.cs +++ b/src/Samples/Tests/Tests/Control/ComboBoxTests.cs @@ -1,9 +1,11 @@ using DotVVM.Samples.Tests.Base; using DotVVM.Testing.Abstractions; +using OpenQA.Selenium.Support.UI; using Riganti.Selenium.Core; using Riganti.Selenium.DotVVM; using Xunit; using Xunit.Abstractions; +using Xunit.Sdk; namespace DotVVM.Samples.Tests.Control { @@ -296,5 +298,30 @@ public void Control_ComboBox_BindingCTValidation_StringToEnum() }); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Control_ComboBox_BooleanProperty(bool nullable) + { + RunInAllBrowsers(browser => { + browser.NavigateToUrl(SamplesRouteUrls.ControlSamples_ComboBox_BooleanProperty); + var suffix = nullable ? "-n" : "-nn"; + var values = nullable ? new bool?[] {true, false, null, false, true} : new bool?[] { true, false, false, true }; + + foreach (var selectedBox in new [] { "cb1", "cb2" }) + { + foreach (var v in values) + { + var index = v switch { true => 0, false => 1, null => 2 }; + browser.Single(selectedBox + suffix, SelectByDataUi).Select(index); + AssertUI.InnerTextEquals(browser.Single("value" + suffix, SelectByDataUi), v?.ToString() ?? "null"); + Assert.Equal(new SelectElement(browser.Single("cb1" + suffix, SelectByDataUi).WebElement).SelectedOption.Text, v?.ToString().ToLowerInvariant() ?? ""); + Assert.Equal(new SelectElement(browser.Single("cb2" + suffix, SelectByDataUi).WebElement).SelectedOption.Text, v?.ToString().ToUpperInvariant() ?? "NULL"); + } + } + }); + } + + } } From b1c821c8802dbc92d91fa4b67560d307b4210733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Wed, 8 May 2024 14:37:04 +0200 Subject: [PATCH 2/2] AutoUI: set enum selector to the enum serialized string instead of enum value --- .../FormEditors/EnumComboBoxFormEditorProvider.cs | 9 +++++---- src/Framework/Framework/Binding/ValueOrBinding.cs | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/AutoUI/Core/PropertyHandlers/FormEditors/EnumComboBoxFormEditorProvider.cs b/src/AutoUI/Core/PropertyHandlers/FormEditors/EnumComboBoxFormEditorProvider.cs index 7b4d6d1f1b..c8d86ff239 100644 --- a/src/AutoUI/Core/PropertyHandlers/FormEditors/EnumComboBoxFormEditorProvider.cs +++ b/src/AutoUI/Core/PropertyHandlers/FormEditors/EnumComboBoxFormEditorProvider.cs @@ -32,10 +32,11 @@ public override DotvvmControl CreateControl(PropertyDisplayMetadata property, Au LocalizableString.CreateNullable(displayAttribute?.Name, displayAttribute?.ResourceType) ?? LocalizableString.Constant(name.Humanize()); var title = LocalizableString.CreateNullable(displayAttribute?.Description, displayAttribute?.ResourceType); - return (name, displayName, title); - }) - .Select(e => new SelectorItem(e.displayName.ToBinding(context.BindingService), new(Enum.Parse(enumType, e.name))) - .AddAttribute("title", e.title?.ToBinding(context.BindingService))); + var enumJsString = ReflectionUtils.ToEnumString(enumType, name); + + return new SelectorItem(displayName.ToBinding(context.BindingService), new(enumJsString)) + .AddAttribute("title", title?.ToBinding(context.BindingService)); + }); var control = new ComboBox() .SetCapability(props.Html) diff --git a/src/Framework/Framework/Binding/ValueOrBinding.cs b/src/Framework/Framework/Binding/ValueOrBinding.cs index 84d36e99d3..7c6a073fe9 100644 --- a/src/Framework/Framework/Binding/ValueOrBinding.cs +++ b/src/Framework/Framework/Binding/ValueOrBinding.cs @@ -160,7 +160,7 @@ public TResult ProcessValueBinding(DotvvmBindableObject control, Func If this contains a `resource` binding, it is evaluated and its value placed in property. `value`, and all other bindings are untouched and remain in the property. + /// If this contains a `resource` binding, it is evaluated and its value placed in property. `value`, and all other bindings are untouched and remain in the property. public ValueOrBinding EvaluateResourceBinding(DotvvmBindableObject control) { if (binding is null or IValueBinding or not IStaticValueBinding) return this;