dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.6k stars 25.29k forks source link

Provide guidance about how to write components that expose a bindable #13539

Closed mkArtakMSFT closed 5 years ago

mkArtakMSFT commented 5 years ago

There is a confusion from community where people expect that this is already supported. Here is an example of such confusion: https://github.com/aspnet/AspNetCore/issues/12690

chrdlx commented 5 years ago

Yes, I tried to change @bind for the (@onchange + value) combo, the issue now is that I lost the automatic type convertion provided by @bind.

I have a @bind="val" where valis decimal. And if I change my computer's decimal and thousands separator, it works perfectly.

Instead, if I go the (@onchange + value) route, I have to implement that convertion system myself... or am I missing something?

Regards!

guardrex commented 5 years ago

@chrdlx

For the docs update, I hope to be free this afternoon to take a closer look at this. 😅🏃🏃🏃🏃😅

I recommend that you ask further questions about the scenario on the engineering side ... on https://github.com/aspnet/AspNetCore/issues/12690. It will be more discoverable than over here. I see that the issue is closed, but it's still best to ask over there. They'll either provide a quick answer or re-open for additional discussion.

guardrex commented 5 years ago

@chrdlx I was playing with some 🙈 RexHacks!:tm: 🙉. The following seems to offer the behavior that you're seeking. I set the browser's language to Italian 🍕, and I was able to enter comma decimal values and get decimals assigned.

Can you paste your latest approach in here ... the value+onchange approach that wasn't allowing your browser's language setting to work properly?

Parent component

@page "/"

<p>ParentYear in Parent: @ParentYear</p>

<CascadingValue Value="@ParentYear">
    <ChildComponent T="@decimal" OnChange="@SetParentFromChild" />
</CascadingValue>

@code {
    [Parameter]
    private decimal ParentYear { get; set; }

    private void SetParentFromChild(UIChangeEventArgs e)
    {
        ParentYear = Convert.ToDecimal(e.Value);
    }
}

Child component

@typeparam T

<p>ParentYear in Child: @ParentYear</p>

<input type="number" step="any" value="@ParentYear" @onchange="OnChange" />

@code {
    [CascadingParameter]
    private T ParentYear { get; set; }

    [Parameter]
    private EventCallback<UIChangeEventArgs> OnChange { get; set; }
}
guardrex commented 5 years ago

@danroth27 If the <input> value is provided to the parent via UIChangeEventArgs and onchange, the value can't simultaneously be bound to a child property because it complains that there are two onchange events defined ... that makes sense.

Because value can't bind, the value can be brought in as a cascading parameter.

It works, but this feels like another one of my RexHacks!:tm: 😬

If that's correct, we sort'a kind'a mostly do document this today, but it requires the reader to get two sections of content pulled together in their mind in order to pull it off.

I propose to add a similar example :point_up: to the doc where cascading parameters are described (or possibly a new section just under the Cascading values and parameters section) ... and probably with a cross-link from the Event handling > EventCallback section.

If I've gone right into La La Land here :boy::gun:, I'll place your example and explanation into the topic at the right spot.

chrdlx commented 5 years ago

Thanks @guardrex !! My issue with the "value+onchange" way, is that you have to provide the convert functionality yourself as you did in your example. Also you added a CascadingValue which adds further complexity if you have a full form with several fields.

In that case I prefer to go full Cretaceous and use 100% strings and use Convert as needed, which is not great, and will duplicate whatever properties I have that are not string... but it works!!

Regards!

guardrex commented 5 years ago

Moving @JeroMiya's remark from https://github.com/aspnet/AspNetCore.Docs/issues/13578 to this issue (paragraph breaks are added) ...

There's a section on data binding showing the @bind-PropertyName syntax, along with the EventCallback<T> PropertyNameChanged convention, but it doesn't show a complete sample with two-way binding from parent to child and back.

Modifying the one-way sample provided to add an <input @bind=... > element to the child component, bound to the bindable property resulted in a one-way bind-once binding from parent to child, with data flowing to child once on initial render but not back to the parent when the child value changed.

I'm assuming either I did something wrong, or the child component has to explicitly invoke the PropertyNameChanged event callback on property change (which incidentally seems like a lot of extra boilerplate for something that should just work with cascading @binds), but if so, it's not clear from the current documentation.

guardrex commented 5 years ago

I'll attempt to react on this issue by surfacing the guidance from engineering at https://github.com/aspnet/AspNetCore/issues/12695#issuecomment-516587177.

danroth27 commented 5 years ago

I believe the main issue we wanted to capture here is that you can't cascade the data binding. When implementing data binding for a component you have to render the bindable value where you want it and explicitly call the EventCallback when the value changes.

chrdlx commented 5 years ago

@guardrex I think I've come up with a more jurassic hax than yours! 👇

@typeparam T

<input  @bind="@InnerValue" />

@code {
    private T _innerValue;
    public T InnerValue
    {
        get { return _innerValue; }
        set
        {
            //Provides parent notification
            _innerValue = value;
            ValueChanged.InvokeAsync(value);
        }
    }

    [Parameter]
    public T Value
    {
        //Provides parent's ability to change the value
        get { return _innerValue; }
        set { _innerValue = value; }
    }

    [Parameter]
    public EventCallback<T> ValueChanged { get; set; }
}

This is what I was looking for, so I can use this component inside a parent and it provides:

1) The out of the box features of @bind like culture / custom formatting. 2) Notification to the parent when the value changes.

I don't know why I didn't come up with this solution earlier, maybe I was trying to make the framework do what I needed instead of doing it myself :eyes:

guardrex commented 5 years ago

Great scenario. I'm not exactly sure when I'll get this, but it should be fairly soon. I have a few more Pre8 issues (globalization and localization) that I need to get through first. This should come up for coverage fairly soon tho.

JeroMiya commented 5 years ago

@chrdlx I see how your InnerValue changes propagate to the parent component, but not how the parent Value changes propagate to the child component's InnerValue. You could write a setter that sets the InnerValue property but that would result in an infinite loop unless you separated InnerValue into a property and backing store, right?

chrdlx commented 5 years ago

@JeroMiya You're right, I updated the code to provide the parent's ability to update the input.. Check it out, luckily didn't dead lock 😄 But I sweated a bit when I read your post. Try it, let me know.. It seems to work fine.

guardrex commented 5 years ago

This seems like a lot of work. I thought I read a remark from engineering that the InputNumber component could resolve this ... or were they just saying 'look at how we set that component up for an example of how we do this'?

VR-Architect commented 5 years ago

Here is some sample code I wrote to use a complex object with two-way binding from parent-child. It also shows TypeConverting for the custom class for passing in the bind.

    using System.ComponentModel;
    [TypeConverter(typeof(MyClassConverter))]
    public class MyClass
    {
        public string Property1 { get; set; }
        public int Property2 { get; set; }
        public MyClass(string p1, int p2)
        {
            Property1 = p1;
            Property2 = p2;
        }
    }
   using System;
   using System.ComponentModel;
   using System.Globalization;

    [TypeConverter(typeof(MyClassConverter))]
    public class MyClassConverter : TypeConverter
    {
        public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
        {
            if (sourceType == typeof(string))
            {
                return true;
            }
            return base.CanConvertFrom(context, sourceType);
        }
        public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value)
        {
            if (value is string)
            {
                return new MyClass("", Int32.Parse(value.ToString()));
            }
            return base.ConvertFrom(context, culture, value);
        }
        public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)
        {
            if (destinationType == typeof(string)) { return "___"; }
            return base.ConvertTo(context, culture, value, destinationType);
        }
    }
@page "/"

<div style="border: double; padding: 5px;">
    <h1>Parent Component</h1>
    <p>Parent Property1: @ParentMyClass.Property1</p>
    <p>Parent Property2: @ParentMyClass.Property2</p>
    <button class="btn btn-primary" @onclick="ChangeTheData">
        Change the data from Parent
    </button>
</div>
<br />
<div style="border: double; padding: 5px;">
    <Child @bind-myClass="ParentMyClass" @onchange="RefreshParent" />
</div>

@code{
    private MyClass ParentMyClass { get; set; } = new MyClass("Initial data before changed", 2000);
    private void RefreshParent(UIChangeEventArgs args)
    {
        StateHasChanged();

        // If you need access to which property changed in the child and its value
        string changedProperty = args.Type;
        var changedValue = args.Value;

    }
    private void ChangeTheData()
    {
        ParentMyClass.Property1 = "I got changed by the parent";
        ParentMyClass.Property2 = 1776;
    }
}
<h2>Child Component</h2>
<p>Child Property1: @myClass.Property1</p>
<input type="text" @onchange="Property1Changed" value="@myClass.Property1" />
<p>Child Property2: @myClass.Property2</p>
<input type="number" @onchange="Property2Changed" value="@myClass.Property2" />

@code {
    [Parameter] private MyClass myClass { get; set; }
    [Parameter] public EventCallback<UIChangeEventArgs> MyClassChanged { get; set; }
    [Parameter] public EventCallback<UIChangeEventArgs> OnChange { get; set; }

    private async Task Property1Changed(UIChangeEventArgs args)
    {
        myClass.Property1 = args.Value.ToString();
        args.Type = "Property1";
        await OnChange.InvokeAsync(args);
    }
    private async Task Property2Changed(UIChangeEventArgs args)
    {
        if (int.TryParse(args.Value.ToString(), out int iValue))
        {
            myClass.Property2 = iValue;
            args.Type = "Property2";
            await OnChange.InvokeAsync(args);
        }
    }
}
VR-Architect commented 5 years ago

BTW, if anyone has any influence over the devs. Having to do the type conversion seems like it should be out-of-box for us. I got stuck there for a while.

guardrex commented 5 years ago

has any influence over the devs

The :pray: server gods :pray: are listening! :ear:

Dan monitors Blazor feedback on both doc issues and engineering issues.

guardrex commented 5 years ago

type conversion seems like it should be out-of-box for us

That's why I asked about the InputNumber component approach. It was mentioned along the way in convos. I couldn't tell (and haven't had time to look yet) to determine if the remark was 'just use it' or 'this is how we do it ... you should do something similar.'

VR-Architect commented 5 years ago

Humm, I tried the form validation but they don't seem to be calling the onchange event. I need the onchange to work so the child can notify the parent to refresh state.

What would be awesome for the devs to do is auto-refresh a parent whenever a child changes a bound object's value.

Note, I don't want a submit button as I need the values to change when the user changes a field. I am making a property panel, so not normal to have such a button.

<EditForm Model="@myClass">
    <DataAnnotationsValidator />
    <ValidationSummary />

    <InputText id="p001" @bind-Value="@myClass.Property1" @onchange="Property1Changed" />
    <InputNumber Id="p002" @bind-Value="@myClass.Property2" @onchange="Property2Changed" />
</EditForm>
guardrex commented 5 years ago

auto-refresh a parent whenever a child changes a bound object's value.

Using my cascading param approach does that. When I get some time, I'll take a closer look at these approaches, and I want to try out the @chrdlx approach, too. I'll look at the reference that was made to InputNumber ... they may have meant to say 'this is how we do it', not 'use it directly.'

VR-Architect commented 5 years ago

EDITED: So I see the root issue now. We can't have a onchange event and a bind-value at the same time. If we use the bind-value in a child, the child is not notifying the parent of the changed stated. So we have to use the onchange event with the value attribute like I did above.

I would hope the devs would fix this as it is not intuitive. If I change a child's state, I expect the parent to get that change without extra code when using two-way binding. This seems like a core tenant of using components with two-way binding.

nstohler commented 5 years ago

I wanted to add a MatDialog (from https://github.com/SamProf/MatBlazor) to my page. Having it directly in my main page was no problem, but I wanted to refactor the whole dialog part out of there into its own component.

I had to use the following to keep the parent EditDlgOpen in sync with the child IsOpen and MatDialogs internal open state field. The child IsOpen setter change checks were vital, otherwise I would run into infinite loops!

I've tested this mechanism over three layers (parent-child-grandchild), with buttons to toggle the local state; the state change propagated through all levels, everything stayed in synch!

Parent: Items.razor:

<button @onclick="@(x => EditDlgOpen = !EditDlgOpen)">test dlg: @EditDlgOpen</button>
<MyEditor @bind-IsOpen="EditDlgOpen"></MyEditor>

@code 
{
    bool EditDlgOpen { get; set; } = false;
}

Child: MyEditor.razor

<h1>MyEditor</h1>

<button @onclick="@(x => IsOpen = !IsOpen)">toggle</button>

<MatDialog @bind-IsOpen="@IsOpen">
    <MatDialogTitle>Editor Title</MatDialogTitle>
    <MatDialogContent>
        <div>editor form here</div>
    </MatDialogContent>
    <MatDialogActions>
    </MatDialogActions>
</MatDialog>

@code
{
    private bool _isOpen;

    [Parameter]
    public bool IsOpen
    {
        get
        {
            return _isOpen;
        }
        set
        {
            if (_isOpen != value) // prevent infinite loops!
            {
                _isOpen = value;
                IsOpenChanged.InvokeAsync(value);
            }
        }
    }

    [Parameter]
    public EventCallback<bool> IsOpenChanged { get; set; }
}
danroth27 commented 5 years ago

@guardrex Is this now covered by the chained bind content?

guardrex commented 5 years ago

@danroth27 AFAICT, yes ... type conversion is still required from ChangeEventArgs (we show that in the content that we added), the validation we show is good, and the use of generics shouldn't be a problem. We can keep an :ear: open for 'How would this look with generics?' questions and then explicitly add an example later if needed. I'll mark the PR that we did with a "fixes" for this issue.