From 4d4304495ea1f8b522c6ce754daf88c81f4672c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg?= Date: Wed, 22 Nov 2023 13:51:34 +0100 Subject: [PATCH 1/4] Added repro for a bug --- .../Common/DotVVM.Samples.Common.csproj | 3 ++ src/Samples/Common/DotvvmStartup.cs | 2 + .../CommandAsPropertyPageViewModel.cs | 37 +++++++++++++++++++ .../MarkupControl/CommandAsProperty.cs | 24 ++++++++++++ .../CommandAsProperty.dotcontrol | 10 +++++ .../CommandAsPropertyPage.dothtml | 19 ++++++++++ .../MarkupControl/CommandAsPropertyWrapper.cs | 24 ++++++++++++ .../CommandAsPropertyWrapper.dotcontrol | 10 +++++ 8 files changed, 129 insertions(+) create mode 100644 src/Samples/Common/ViewModels/FeatureSamples/MarkupControl/CommandAsPropertyPageViewModel.cs create mode 100644 src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs create mode 100644 src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol create mode 100644 src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml create mode 100644 src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.cs create mode 100644 src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol diff --git a/src/Samples/Common/DotVVM.Samples.Common.csproj b/src/Samples/Common/DotVVM.Samples.Common.csproj index 09852a5a42..fc40ba614e 100644 --- a/src/Samples/Common/DotVVM.Samples.Common.csproj +++ b/src/Samples/Common/DotVVM.Samples.Common.csproj @@ -118,6 +118,9 @@ + + + diff --git a/src/Samples/Common/DotvvmStartup.cs b/src/Samples/Common/DotvvmStartup.cs index 04dac2a63f..11bde4c862 100644 --- a/src/Samples/Common/DotvvmStartup.cs +++ b/src/Samples/Common/DotvvmStartup.cs @@ -267,6 +267,8 @@ private static void AddControls(DotvvmConfiguration config) config.Markup.AddCodeControls("cc", typeof(Loader)); config.Markup.AddMarkupControl("sample", "EmbeddedResourceControls_Button", "embedded://EmbeddedResourceControls/Button.dotcontrol"); config.Markup.AddMarkupControl("cc", "NodeControl", "Views/ControlSamples/HierarchyRepeater/NodeControl.dotcontrol"); + config.Markup.AddMarkupControl("cc", "CommandAsProperty", "Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol"); + config.Markup.AddMarkupControl("cc", "CommandAsPropertyWrapper", "Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol"); config.Markup.AutoDiscoverControls(new DefaultControlRegistrationStrategy(config, "sample", "Views/")); diff --git a/src/Samples/Common/ViewModels/FeatureSamples/MarkupControl/CommandAsPropertyPageViewModel.cs b/src/Samples/Common/ViewModels/FeatureSamples/MarkupControl/CommandAsPropertyPageViewModel.cs new file mode 100644 index 0000000000..21f0a92112 --- /dev/null +++ b/src/Samples/Common/ViewModels/FeatureSamples/MarkupControl/CommandAsPropertyPageViewModel.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using DotVVM.Framework.ViewModel; +using DotVVM.Framework.Hosting; + +namespace DotVVM.Samples.Common.ViewModels.FeatureSamples.MarkupControl +{ + public class CommandAsPropertyPageViewModel : DotvvmViewModelBase + { + + public List Items { get; set; } = new() { + new ItemModel() { Name = "One", IsTrue = true }, + new ItemModel() { Name = "Two", IsTrue = false }, + new ItemModel() { Name = "Three", IsTrue = true } + }; + + public ItemModel SelectedItem { get; set; } + + public Task MyFunction(string name, bool isTrue) + { + SelectedItem = new ItemModel() { Name = name, IsTrue = isTrue }; + return Task.CompletedTask; + } + + + public class ItemModel + { + public string Name { get; set; } + public bool IsTrue { get; set; } + + } + } +} + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs new file mode 100644 index 0000000000..b5fcbe49b3 --- /dev/null +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs @@ -0,0 +1,24 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using DotVVM.Framework.Binding; +using DotVVM.Framework.Controls; + +namespace DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl +{ + public class CommandAsProperty : DotvvmMarkupControl + { + + public Func Click + { + get => (Func)GetValue(ClickProperty)!; + set => SetValue(ClickProperty, value); + } + public static readonly DotvvmProperty ClickProperty + = DotvvmProperty.Register, CommandAsProperty>(c => c.Click, null); + + } +} + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol new file mode 100644 index 0000000000..18f66e6db9 --- /dev/null +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol @@ -0,0 +1,10 @@ +@viewModel System.String, mscorlib +@baseType DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl.CommandAsProperty, DotVVM.Samples.Common + +<%-- When uncommented, it produces an error --%> +<%-- In Release mode, the error that is really hard to debug (maybe it was caused because the control was loaded from embedded resource). --%> +<%----%> + + +<%--The following with ToString works somehow and I am not sure by what magic--%> + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml new file mode 100644 index 0000000000..14096e0037 --- /dev/null +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml @@ -0,0 +1,19 @@ +@viewModel DotVVM.Samples.Common.ViewModels.FeatureSamples.MarkupControl.CommandAsPropertyPageViewModel, DotVVM.Samples.Common + + + + + + + + + + + + +

Selected item: {{value: Name}}, {{value: IsTrue}}

+ + + + + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.cs b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.cs new file mode 100644 index 0000000000..2772994a13 --- /dev/null +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.cs @@ -0,0 +1,24 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using DotVVM.Framework.Binding; +using DotVVM.Framework.Controls; + +namespace DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl +{ + public class CommandAsPropertyWrapper : DotvvmMarkupControl + { + + public Func Click + { + get { return (Func)GetValue(ClickProperty); } + set { SetValue(ClickProperty, value); } + } + public static readonly DotvvmProperty ClickProperty + = DotvvmProperty.Register, CommandAsPropertyWrapper>(c => c.Click, null); + + } +} + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol new file mode 100644 index 0000000000..8452273875 --- /dev/null +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol @@ -0,0 +1,10 @@ +@viewModel DotVVM.Samples.Common.ViewModels.FeatureSamples.MarkupControl.CommandAsPropertyPageViewModel, DotVVM.Samples.Common +@baseType DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl.CommandAsPropertyWrapper, DotVVM.Samples.Common + + + <%-- + The correct syntax would be Click="{value: _control.Click(_parent.Name, _parent.IsTrue)}" + Btw is value binding the right thing, or shall we use resource binding here? + --%> + + From 8121e7aae4be5352d0f019262ac4bcf6d9228ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Sun, 26 Nov 2023 10:02:08 +0100 Subject: [PATCH 2/4] Improve error message when binding a command with arguments into normal button --- .../Javascript/ParametrizedCode.cs | 14 ++++++-- .../Framework/Controls/KnockoutHelper.cs | 33 ++++++++++++------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs b/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs index 95a0941035..7138d78314 100644 --- a/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs +++ b/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs @@ -50,9 +50,14 @@ public ParametrizedCode(string code, OperatorPrecedence precedence = default) // TODO(exyi): add WriteTo(StringBuilder) /// - /// Converts this to string and assigns all parameters using `parameterAssignment`. If there is any missing, exception is thrown. + /// Converts this to string and assigns all parameters using `parameterAssignment`. /// + /// Thrown when some parameter is not assigned and has no default value. public string ToString(Func parameterAssignment) => ToString(parameterAssignment, out var _); + /// + /// Converts this to string and assigns all parameters using `parameterAssignment`. + /// + /// Thrown when some parameter is not assigned and has no default value. public string ToString(Func parameterAssignment, out bool allIsDefault) { allIsDefault = true; @@ -205,7 +210,7 @@ private CodeParameterAssignment[] FindAssignment(Func EnumerateAllParameters() } } + public record MissingAssignmentException(CodeParameterInfo Parameter, ParametrizedCode FullCode): RecordExceptions.RecordException + { + public override string Message => $"Assignment of parameter '{Parameter.Parameter}' was not found."; + } + /// /// Builder class with reasonably fast Add operation. Use Build method to convert it to immutable ParametrizedCode /// diff --git a/src/Framework/Framework/Controls/KnockoutHelper.cs b/src/Framework/Framework/Controls/KnockoutHelper.cs index e0438393ea..5a40e00242 100644 --- a/src/Framework/Framework/Controls/KnockoutHelper.cs +++ b/src/Framework/Framework/Controls/KnockoutHelper.cs @@ -9,6 +9,7 @@ using DotVVM.Framework.Compilation.Javascript.Ast; using DotVVM.Framework.Configuration; using DotVVM.Framework.Utils; +using FastExpressionCompiler; using Newtonsoft.Json; namespace DotVVM.Framework.Controls @@ -263,18 +264,26 @@ options.KoContext is object ? string SubstituteArguments(ParametrizedCode parametrizedCode) { - return parametrizedCode.ToString(p => - p == JavascriptTranslator.CurrentElementParameter ? options.ElementAccessor : - p == CommandBindingExpression.CurrentPathParameter ? CodeParameterAssignment.FromIdentifier(getContextPath(control)) : - p == CommandBindingExpression.ControlUniqueIdParameter ? uniqueControlId?.GetParametrizedJsExpression(control) ?? CodeParameterAssignment.FromLiteral("") : - p == JavascriptTranslator.KnockoutContextParameter ? knockoutContext : - p == JavascriptTranslator.KnockoutViewModelParameter ? viewModel : - p == CommandBindingExpression.OptionalKnockoutContextParameter ? optionalKnockoutContext : - p == CommandBindingExpression.CommandArgumentsParameter ? options.CommandArgs ?? default : - p == CommandBindingExpression.PostbackHandlersParameter ? CodeParameterAssignment.FromIdentifier(getHandlerScript()) : - p == CommandBindingExpression.AbortSignalParameter ? abortSignal : - default - ); + try + { + return parametrizedCode.ToString(p => + p == JavascriptTranslator.CurrentElementParameter ? options.ElementAccessor : + p == CommandBindingExpression.CurrentPathParameter ? CodeParameterAssignment.FromIdentifier(getContextPath(control)) : + p == CommandBindingExpression.ControlUniqueIdParameter ? uniqueControlId?.GetParametrizedJsExpression(control) ?? CodeParameterAssignment.FromLiteral("") : + p == JavascriptTranslator.KnockoutContextParameter ? knockoutContext : + p == JavascriptTranslator.KnockoutViewModelParameter ? viewModel : + p == CommandBindingExpression.OptionalKnockoutContextParameter ? optionalKnockoutContext : + p == CommandBindingExpression.CommandArgumentsParameter ? options.CommandArgs ?? default : + p == CommandBindingExpression.PostbackHandlersParameter ? CodeParameterAssignment.FromIdentifier(getHandlerScript()) : + p == CommandBindingExpression.AbortSignalParameter ? abortSignal : + default + ); + } + catch (ParametrizedCode.MissingAssignmentException e) when (e.Parameter.Parameter == CommandBindingExpression.CommandArgumentsParameter) + { + var returnType = expression.GetProperty(ErrorHandlingMode.ReturnNull)?.Type; + throw new DotvvmControlException(control, $"The binding {expression} of type {returnType?.ToCode(stripNamespace: true) ?? "?"} requires arguments, but none were provided to the KnockoutHelper.GenerateClientPostback method.", innerException: e) { RelatedBinding = expression }; + } } } From 6d676be6605bc17bf3cefbfb56cda81cb6d5004e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg?= Date: Thu, 8 Feb 2024 17:41:59 +0100 Subject: [PATCH 3/4] Updates in samples + added UI tests --- .../MarkupControl/CommandAsProperty.cs | 6 ++--- .../CommandAsProperty.dotcontrol | 2 +- .../CommandAsPropertyPage.dothtml | 16 ++++++++++++- .../CommandAsPropertyWrapper.dotcontrol | 23 ++++++++++++++---- .../Abstractions/SamplesRouteUrls.designer.cs | 1 + .../Tests/Tests/Feature/MarkupControlTests.cs | 24 +++++++++++++++++++ 6 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs index b5fcbe49b3..bba02836e9 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.cs @@ -11,13 +11,13 @@ namespace DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl public class CommandAsProperty : DotvvmMarkupControl { - public Func Click + public Func Click { - get => (Func)GetValue(ClickProperty)!; + get => (Func)GetValue(ClickProperty)!; set => SetValue(ClickProperty, value); } public static readonly DotvvmProperty ClickProperty - = DotvvmProperty.Register, CommandAsProperty>(c => c.Click, null); + = DotvvmProperty.Register, CommandAsProperty>(c => c.Click, null); } } diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol index 18f66e6db9..4419a16506 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol @@ -7,4 +7,4 @@ <%--The following with ToString works somehow and I am not sure by what magic--%> - + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml index 14096e0037..71ba6b9cee 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml @@ -9,9 +9,23 @@ +

Pass as command

+
-

Selected item: {{value: Name}}, {{value: IsTrue}}

+ <%--

Pass as static command

+ +
--%> + + <%--

Pass as value

+ +
--%> + + <%--

Pass as resource

+ +
--%> + +

Selected item: {{value: Name}}, {{value: IsTrue}}

diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol index 8452273875..fc84905e3a 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol @@ -2,9 +2,22 @@ @baseType DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl.CommandAsPropertyWrapper, DotVVM.Samples.Common - <%-- - The correct syntax would be Click="{value: _control.Click(_parent.Name, _parent.IsTrue)}" - Btw is value binding the right thing, or shall we use resource binding here? - --%> - +
+
+

Passed as command

+ +
+
+

Passed as static command

+ +
+
+

Passed as value

+ +
+
+

Passed as resource

+ +
+
diff --git a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs index fe2388d721..84518eb640 100644 --- a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs +++ b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs @@ -281,6 +281,7 @@ public partial class SamplesRouteUrls public const string FeatureSamples_Localization_Localization_FormatString = "FeatureSamples/Localization/Localization_FormatString"; public const string FeatureSamples_Localization_Localization_NestedPage_Type = "FeatureSamples/Localization/Localization_NestedPage_Type"; public const string FeatureSamples_MarkupControl_ComboBoxDataSourceBoundToStaticCollection = "FeatureSamples/MarkupControl/ComboBoxDataSourceBoundToStaticCollection"; + public const string FeatureSamples_MarkupControl_CommandAsPropertyPage = "FeatureSamples/MarkupControl/CommandAsPropertyPage"; public const string FeatureSamples_MarkupControl_CommandBindingInDataContextWithControlProperty = "FeatureSamples/MarkupControl/CommandBindingInDataContextWithControlProperty"; public const string FeatureSamples_MarkupControl_CommandBindingInRepeater = "FeatureSamples/MarkupControl/CommandBindingInRepeater"; public const string FeatureSamples_MarkupControl_CommandPropertiesInMarkupControl = "FeatureSamples/MarkupControl/CommandPropertiesInMarkupControl"; diff --git a/src/Samples/Tests/Tests/Feature/MarkupControlTests.cs b/src/Samples/Tests/Tests/Feature/MarkupControlTests.cs index ef51f5007f..f034ca4ce6 100644 --- a/src/Samples/Tests/Tests/Feature/MarkupControlTests.cs +++ b/src/Samples/Tests/Tests/Feature/MarkupControlTests.cs @@ -409,5 +409,29 @@ public void Feature_MarkupControl_MarkupDeclaredProperties() browser.WaitFor(() => AssertUI.InnerTextEquals(browser.First("[data-ui=counter]"), "2"), 2000); }); } + + [Fact] + public void Feature_MarkupControl_CommandAsPropertyPage() + { + RunInAllBrowsers(browser => { + browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_MarkupControl_CommandAsPropertyPage); + + var lists = browser.FindElements("div[data-ui=button-list]"); + for (var j = 0; j < 4; j++) + { + for (var i = 0; i < lists.Count; i++) + { + lists[i].ElementAt("input[type=button]", j).Click(); + AssertUI.InnerTextEquals(browser.Single("p[data-ui=result]"), (i % 3) switch { + 0 => "Selected item: One, true", + 1 => "Selected item: Two, false", + _ => "Selected item: Three, true" + }); + + i++; + } + } + }); + } } } From 47ba7e202526503e459a43f064aa46b702ff1655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg?= Date: Fri, 23 Feb 2024 14:44:50 +0100 Subject: [PATCH 4/4] Refined UI tests --- .../MarkupControl/CommandAsProperty.dotcontrol | 8 ++------ .../MarkupControl/CommandAsPropertyPage.dothtml | 6 +++--- .../MarkupControl/CommandAsPropertyWrapper.dotcontrol | 2 ++ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol index 4419a16506..6aa0b43198 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsProperty.dotcontrol @@ -1,10 +1,6 @@ @viewModel System.String, mscorlib @baseType DotVVM.Samples.Common.Views.FeatureSamples.MarkupControl.CommandAsProperty, DotVVM.Samples.Common -<%-- When uncommented, it produces an error --%> -<%-- In Release mode, the error that is really hard to debug (maybe it was caused because the control was loaded from embedded resource). --%> -<%----%> - - -<%--The following with ToString works somehow and I am not sure by what magic--%> +<%-- both options are legit --%> + diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml index 71ba6b9cee..7ca44388c9 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyPage.dothtml @@ -13,9 +13,9 @@
- <%--

Pass as static command

- -
--%> +

Pass as static command

+ +
<%--

Pass as value

diff --git a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol index fc84905e3a..7aa8969c17 100644 --- a/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol +++ b/src/Samples/Common/Views/FeatureSamples/MarkupControl/CommandAsPropertyWrapper.dotcontrol @@ -11,6 +11,8 @@

Passed as static command

+ + <%-- remove after we fix the lambda conversion --%>

Passed as value