havit / Havit.Blazor

Free Bootstrap 5 components for ASP.NET Blazor + optional enterprise-level stack for Blazor development (gRPC code-first, layered architecture, localization, auth, ...)
https://havit.blazor.eu
MIT License
490 stars 67 forks source link

[HxInputNumber] Improperly requires ValueExpression when manually-binding #536

Closed robertmclaws closed 9 months ago

robertmclaws commented 1 year ago

If you are using a HxInputNumber inside a more control, and using a Parameter on that custom control to bind the inner value of the editor, the runtime throws an exception if you only wire up the Value and ValueChanged properties. I had to add a ValueExpression property, even though the expression was not used at all.

Might consider adding a default expression or something so that the code doesn't break.

hakenr commented 1 year ago

@jirikanda?

jirikanda commented 1 year ago

We derive out HxInputNumber from Microsoft.AspNetCore.Components.Forms.InputBase.

The condition on ValueExpression is required by this base class same way as the EditedContext is required. And here is the Microsoft check for the ValueExpression: https://github.com/dotnet/aspnetcore/blob/c42b0d67a086b8472c0d0e4d8579ff0fb2a49274/src/Components/Web/src/Forms/InputBase.cs#LL233C6-L233C6

ValueExpression is used for model validation and the validation result display.

When required EditedContext is missing we try to fake one. We can try the same for the ValueExpression but the consequences here are here enormous. When HxInputNumber is used with binding, validation messages works. When HxInputNumber was used without binding, the validation messages would not work. From my perspective, our right action here is "no action, no change".

If you know what you are doing, you can fake ValueExpression yourself 🤔.

@hakenr , @robertmclaws

robertmclaws commented 1 year ago

I have so many issues with this response.

When required EditedContext is missing we try to fake one.

This makes the control incredibly useful, because I don't need to wrap it in an EditForm to use it. Requiring one was a stupid decision on Microsoft's part, and you folks did the right thing by working around it.

When HxInputNumber was used without binding, the validation messages would not work.

They ALREADY don't work, because if for some reason you cannot use bindings, then you're not going to be able to use a ValueExpression for validation either. So that logic doesn't make any sense.

If you know what you are doing, you can fake ValueExpression yourself 🤔.

Why on earth would anyone want to have to go that deep into learning how it works in order to fake it? You folks already know how it works, your controls should have a built-in way to opt-in to behavior that fakes it the same way you are already faking an EditContext. Anything else is inviting the same kind of disaster by other means, because different developers will create different workarounds, and then build their apps to rely on those workarounds.

If you're going to make your controls work outside of an EditContext, then you can't half-ass it. If you're not using an EditContext, you're probably not using Validation.

There are other possible options besides "do nothing", and you folks are creative enough to come up with one. I believe in you!

hakenr commented 9 months ago

@robertmclaws I believe that ValueExpression cannot be artificially created "from inside" like EditContext, unless we introduce a new, unique identifier for the form field, which is not our intention. This concept is similar to how name or id attributes are utilized with input tags. The Blazor team has chosen ValueExpression as the field identifier, and it's quite straightforward to implement in wrapper components.

In addition to validation, ValueExpression is also now used for SSR forms (see ASP.NET Core pull request #49224, particularly the Editor<T> abstract class).

Simplified Usage and Configuration

Consider the example of MyInputMoney.razor:

<HxInputNumber Value="Amount" ValueChanged="AmountChanged" ValueExpression="AmountExpression" ... />
<HxSelect Value="Currency" ValueChanged="CurrencyChanged" ValueExpression="CurrencyExpression" ... />

@code {
  [Parameter] public decimal? Amount { get; set; }
  [Parameter] public EventCallback<decimal?> AmountChanged { get; set; }
  [Parameter] public Expression<Func<decimal?>> AmountExpression { get; set; }
  [Parameter] public int Currency { get; set; }
  [Parameter] public EventCallback<int> CurrencyChanged { get; set; }
  [Parameter] public Expression<Func<int>> CurrencyExpression { get; set; }
  ...
}

With this setup, you can easily utilize:

<MyInputMoney @bind-Amount="amount" @bind-Currency="currencyCode" />

@code {
  private decimal? amount { get; set; }
  private int currencyCode { get; set; }
}

This is not the only way to structure your edit-components, but it shows that using ValueExpression doesn’t significantly increase complexity.

If you still want to simulate the ValueExpression, it's straightforward to apply the syntax where necessary:

<HxInputNumber ValueExpression="() => this.Amount" ... />

(Messing with the ValueExpression leads to issues such as https://github.com/havit/Havit.Blazor/issues/673#issuecomment-1860879301, which we would like to avoid.)