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

InputText and other input components do not use SetUpdatesAttributeName() for two-way binding #40097

Closed
1 task done
hakenr opened this issue Feb 9, 2022 · 3 comments
Closed
1 task done
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed feature-blazor-builtin-components Features related to the built in components we ship or could ship in the future help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone

Comments

@hakenr
Copy link
Member

hakenr commented Feb 9, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Hi @SteveSandersonMS,
is there any reason why the InputText implementation (and implementation of other input components) do not use the SetUpdatesAttributeName("value") method when it replicates the <input @bind-value="..." /> in BuildRenderTree()?

The changes in DOM get not synchronized to the .NET model then.

As a result when someone wants to derive his own component from InputText e.g. to apply some text formatting and you enter the same text input twice, the first callback causes the UI to replace the value with the formatted value but the second callback leaves the input intact as it does not detect any change in the .NET model to be synchronized to DOM (no matter you call the StateHasChanged() or not).

Expected Behavior

.NET model of InputText and other input components reflect DOM changes (two-way binding) from their underlying input elements.

Steps To Reproduce

public class MyInputText : InputText
{
	protected override bool TryParseValueFromString(string? value, out string? result, [NotNullWhen(false)] out string? validationErrorMessage)
	{
		if (value == "24h")
		{
			result = "24:00:00";
		}
		else
		{
			result = value;
		}
		validationErrorMessage = null;
		return true;
	}
}
<EditForm Model="model">
    <MyInputText @bind-Value="@model.Value" />
</EditForm>
@model.Value

@code
{
    private Model model = new Model();
    private class Model
    {
        public string Value { get; set; }
    }
}

2022-02-09_16-49-57

This fixes the issue:

public class MyInputText : InputText
{
	protected override bool TryParseValueFromString(string? value, out string? result, [NotNullWhen(false)] out string? validationErrorMessage)
	{
		if (value == "24h")
		{
			result = "24:00:00";
		}
		else
		{
			result = value;
		}
		validationErrorMessage = null;
		return true;
	}

	protected override void BuildRenderTree(RenderTreeBuilder builder)
	{
		builder.OpenElement(0, "input");
		builder.AddMultipleAttributes(1, base.AdditionalAttributes);
		builder.AddAttribute(2, "class", base.CssClass);
		builder.AddAttribute(3, "value", BindConverter.FormatValue(base.CurrentValue));
		builder.AddAttribute(4, "onchange", EventCallback.Factory.CreateBinder<string>((object)this, (Action<string>)delegate (string? __value)
		{
			base.CurrentValueAsString = __value;
		}, base.CurrentValueAsString, (CultureInfo?)null));
		builder.SetUpdatesAttributeName("value");              // <-------- THIS FIXES THE ISSUE
		builder.CloseElement();
	}
}

Exceptions (if any)

No response

.NET Version

6.0.101

Anything else?

If it helps, I can prepare a PR for this.

Related issue #17281

@TanayParikh TanayParikh added area-blazor Includes: Blazor, Razor Components feature-blazor-builtin-components Features related to the built in components we ship or could ship in the future labels Feb 9, 2022
@SteveSandersonMS
Copy link
Member

@hakenr Yes, I think you're right. Thanks for providing such a clear issue description and for checking into the solution.

If you are willing to provide a PR for this (along with some E2E test cases showing that it makes a difference), I'd be happy to work with you to get it merged soon.

@mkArtakMSFT mkArtakMSFT added the bug This issue describes a behavior which is not expected - a bug. label Feb 9, 2022
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Feb 9, 2022
@ghost
Copy link

ghost commented Feb 9, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@SteveSandersonMS
Copy link
Member

Fixed in #46434

@SteveSandersonMS SteveSandersonMS modified the milestones: Backlog, 8.0-preview2 Feb 9, 2023
@SteveSandersonMS SteveSandersonMS added the Done This issue has been fixed label Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed feature-blazor-builtin-components Features related to the built in components we ship or could ship in the future help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@SteveSandersonMS @hakenr @TanayParikh @mkArtakMSFT and others