Skip to content
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

BlazorInput two-way binding doesn't work #41

Closed
dinhduongha opened this issue Feb 27, 2019 · 21 comments
Closed

BlazorInput two-way binding doesn't work #41

dinhduongha opened this issue Feb 27, 2019 · 21 comments
Assignees
Labels
bug Something isn't working

Comments

@dinhduongha
Copy link

When using the BlazorInput tag you are unable to use two-way binding to the value.
Change text and press submit.

<BlazorInput InputType="InputType.Text" bind-Content="@Content" />@*Two-Way?*@
<input type="text" bind="@Content2" />
<BlazorButton Color="Color.Primary" OnClick="@Submit">Submit</BlazorButton>
@functions {
    private string Content { get; set; }
    private string Content2 { get; set; }
    async void Submit(UIMouseEventArgs e)
    {
        Console.WriteLine(Content);  // Not work
        Console.WriteLine(Content2); // Work
    }
}
@IvanJosipovic
Copy link
Contributor

Change bind-Content to bind-Value.

Have a look at, https://github.com/chanan/BlazorStrap/blob/master/src/BlazorStrap/BlazorInput.cshtml
[Parameter] private string Value { get; set; }

@simbesh
Copy link

simbesh commented Apr 13, 2019

@IvanJosipovic I cannot get binding to work also. is onchange=@onchange missing from the attributes list to call the method?
When I created my own component I added that and it worked.

@razfriman
Copy link
Contributor

razfriman commented Apr 24, 2019

Can confirm this is not functioning properly.
Here is my usage.

Tried various combinations of bind, value, etc.

One way binding works, but unfortunately until I can get two way data binding working I cannot start to use this library in my project :(

<FormGroup>
<BlazorLabel For="inputFirstName">First Name</BlazorLabel>
<BlazorInput Id="inputFirstName" InputType="InputType.Text" PlaceHolder="First Name" bind-Value="@user.FirstName" />
</FormGroup>
<FormGroup>
<BlazorLabel For="inputFirstName">First Name</BlazorLabel>
<BlazorInput Id="inputFirstName" InputType="InputType.Text" PlaceHolder="First Name" Value="@user.FirstName" />
</FormGroup>

@chanan
Copy link
Owner

chanan commented Apr 24, 2019

bind-value should work, I will take a look at this when I look at the other issue with form tonight.

@chanan chanan self-assigned this Apr 24, 2019
@chanan
Copy link
Owner

chanan commented Apr 24, 2019

The code looks right to me based on the docs: https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.0#data-binding Not sure what is wrong.

@chanan
Copy link
Owner

chanan commented Apr 24, 2019

Tried adding:

case EventCallback<UIChangeEventArgs> e:
                        builder.AddAttribute(1, param.Key, e);
                        break;

Didnt help either

@chanan
Copy link
Owner

chanan commented Apr 24, 2019

I tracked down the problem, but not sure how to solve yet. The issue is that DynamicElement isn't updating value, it most likely need to build an on change event.

@razfriman
Copy link
Contributor

Nice! I'll keep looking into it when I have some time.. So far I have reproduced it in my branch to the point where BlazorInput acts as 1-way, and normal input successfully works as two-way: feel free to check out my branch where I was exploring it:

https://github.com/razfriman/BlazorStrap/pull/1

@chanan
Copy link
Owner

chanan commented Apr 24, 2019

I got basic two way binding support working:

  1. Works on strings in BlazorInput, not sure checkboxs will work or select
  2. Seems to update when the form is submitted only

Creating a new build now, I also add /samples/form with a working example.

@razfriman
Copy link
Contributor

Have you had a look at this? This is the implmentation for Blazor's EditForm Input element variants? Might be a good example to follow?

https://github.com/aspnet/AspNetCore/blob/master/src/Components/Components/src/Forms/InputComponents/InputText.cs

@chanan
Copy link
Owner

chanan commented May 1, 2019

I am considering changing either just the form/input control or all controls to be actual C# classes instead of razor files (which have a backing C# class anyway). At the same time also take some cues from EditForm control. Any thoughts about this?

@chucker
Copy link
Contributor

chucker commented May 3, 2019

Any thoughts about this?

I mean… the linked page does say "Developers building their own input components should use Razor syntax." ;-)

@chucker
Copy link
Contributor

chucker commented Jun 13, 2019

I am considering changing either just the form/input control or all controls to be actual C# classes instead of razor files (which have a backing C# class anyway).

Can we separate that discussion into its own issue?

Creating a new build now, I also add /samples/form with a working example.

Not sure if I broke that, but this fails on load with:

blazor.webassembly.js:1 WASM:
System.ArgumentException: Object of type
'Microsoft.AspNetCore.Components.EventCallback`1[Microsoft.AspNetCore.Components.UIEventArgs]'
cannot be converted to type 'Microsoft.AspNetCore.Components.EventCallback'.

@chanan
Copy link
Owner

chanan commented Jun 13, 2019

About the binding - I wont want to do anything till 1.0 is final - lets talk about it again at that time?

About the error - I will take a look tonight, but I was also wondering if this is still needed: https://github.com/chanan/BlazorStrap/blob/master/src/BlazorStrap/DynamicElement.cs#L90-L131

@chucker
Copy link
Contributor

chucker commented Jun 13, 2019

I was also wondering if this is still needed:

Oh, interesting. They closed dotnet/aspnetcore#10357.

On the one hand, that's great — I considered that piece of code a hack. :)

On the other hand, I fear we found the reason for my error above.

@rpskelton2
Copy link

From looking at documentation and samples it appears the razor.g.cs file should contain onchange statements like below. I'm not any in mine and two way binding is not working on any of my samples.

builder.AddAttribute(5, "onchange", Microsoft.AspNetCore.Components.EventCallback.Factory.CreateBinder(this, __value => Title = __value, Title));

@chanan
Copy link
Owner

chanan commented Jun 17, 2019

Having some issues upgrading to preview6, I will take another look at this once that is done

@rpskelton2
Copy link

rpskelton2 commented Jun 18, 2019

It seems the syntax for binding has changed. Most examples found on line look like this and do not work:
input type="text" bind="@name" />

The new syntax with @Bind seems ok:
input type="text" @Bind="Name" />

This commit on May 30 changed the syntax:
dotnet/aspnetcore@dd07fa0#diff-438bb5dff82affac680fa750d6535658

@chucker
Copy link
Contributor

chucker commented Jun 19, 2019

Yes, that's correct. In preview 6, you're actually supposed to use @bind="@Name". They're hoping to eventually shorten that to @bind="Name". But bind without the @ is no longer correct — attributes without the @ are now HTML, and attributes with it C#.

@chanan
Copy link
Owner

chanan commented Jun 20, 2019

I updated the syntax in v1.0.0-preview6-10 please let me know what you think

@chanan chanan added the bug Something isn't working label Jul 4, 2019
@chanan
Copy link
Owner

chanan commented Jul 4, 2019

Current status:

Per current documentation (preview6) BlazorIntput has:

[Parameter] private string Value { get; set; }
[Parameter] private EventCallback<string> ValueChanged { get; set; }

Which seems to work more or less ok (I think there are still some issues). I am hesitant to change more for two reasons:

  1. more changes to Blazor might change how binding works. Specifically for BlazorStrap, Blazor will be adding splat operator which will likely mean we won't need BootstrapComponentBase anymore which may effect some of our binding issues
  2. I think we should either base forms/inputs off of EditForm or supply duplicates that use EditForm which will also likely solve some of our binding issues

Due to the above, I am going to wait a bit and see what Blazor brings before attempting to further solve these issues.

PS - Basic binding does work, please try it out and let me know how it goes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants