dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.2k stars 9.94k forks source link

Using InputRadioGroup in a component does not generate a unique name #53413

Closed chrisportelli closed 7 months ago

chrisportelli commented 8 months ago

Is there an existing issue for this?

Describe the bug

In Blazor Server for .NET 8, when using the <InputRadioGroup> within a component so that the latter can be re-used, it doesn't seem to generate a Guid-based "name" attribute when this is not provided. This is leading to an issue whereby when having this custom component multiple times on the same page/component, they are all considered to be within the same radio button group. This was previously working in .NET 6.

The below code references a repo created specifically for this issue. From what I can see if we create a component in .NET 8 to wrap a radio button group so that it is used in multiple places, as shown in the below definition:

@typeparam TValue

<InputRadioGroup @bind-Value="@Value">
    @foreach (var option in Options)
    {
        <label>
            <InputRadio Value="@option.Key" /> <span>@option.Value</span>
        </label>
    }
</InputRadioGroup>

@code {
    [Parameter]
    public bool Value { get; set; }

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

    [Parameter]
    public IDictionary<TValue, string> Options { get; set; }
}

When called multiple times as follows:

<CustomInputRadioGroup Options="@_firstOptions" @bind-Value="@_customModel.FirstOption" />
<CustomInputRadioGroup Options="@_secondOptions" @bind-Value="@_customModel.SecondOption" />

This leads to the following HTML where the "name" attribute is the same throughout, even though they are separate groups.

<label><input class="valid" type="radio" name="Value" value="True" /> <span>Option A</span></label>
<label><input class="valid" type="radio" name="Value" value="False" checked="a" /> <span>Option B</span></label>
<label><input class="valid" type="radio" name="Value" value="True" /> <span>Option C</span></label>
<label><input class="valid" type="radio" name="Value" value="False" checked="a" /> <span>Option D</span></label>

In .NET 6, this seems to generate a different name as can be seen here:

<label><input class="modified valid" type="radio" name="effa514c73c347ba8c191a5a9ecffa20" value="True" /> <span>Option A</span></label>
<label><input class="modified valid" type="radio" name="effa514c73c347ba8c191a5a9ecffa20" value="False" /> <span>Option B</span></label>
<label><input class="valid" type="radio" name="01068211277c4f6594fbd24974e17fc3" value="True" /> <span>Option C</span></label>
<label><input class="valid" type="radio" name="01068211277c4f6594fbd24974e17fc3" value="False" /> <span>Option D</span></label>

As a side note, on .NET 8 the issue doesn't seem to occur when using the <InputRadioGroup> component directly on the page. It only seems to happen when placed in another component.

Expected Behavior

Multiple radio button groups should generate a different "name" attribute.

Steps To Reproduce

A repo has been created in https://github.com/chrisportelli/BlazorRadioButtonGroupReproduction which contains a .NET 6 and a .NET 8 project. Each project's homepage makes use of two radio button groups (defined as a separate component) with two radio buttons for each group.

Exceptions (if any)

No response

.NET Version

8.0.101

Anything else?

Visual Studio 17.8.4 OS: Windows 10.0.22621 64-bit

gragra33 commented 8 months ago

It looks like aa bug in the InputRadioGroup.OnParametersSet method. The code was changed:

.Net6 (modified for readability):

protected override void OnParametersSet()
{
    this._context = new InputRadioContext(
        this.CascadedContext, 
        !string.IsNullOrEmpty(this.Name) ? this.Name : this._defaultGroupName,
        (object) this.CurrentValue,
        this.EditContext.FieldCssClass(this.FieldIdentifier),
        EventCallback.Factory.CreateBinder<string>((object) this,
        (Action<string>) (__value => this.CurrentValueAsString = __value),
        this.CurrentValueAsString)
    );

and .Net8 (modified for readability):

protected override void OnParametersSet()
{
      if (this._context == null)
        this._context = new InputRadioContext(
            this.CascadedContext,
            EventCallback.Factory.CreateBinder<string>(
                (object) this,
                (Action<string>) (__value => this.CurrentValueAsString = __value), this.CurrentValueAsString));

      else if (this._context.ParentContext != this.CascadedContext)
        throw new InvalidOperationException("An InputRadioGroup cannot change context after creation");

      this._context.GroupName = string.IsNullOrEmpty(this.Name)
          ? (string.IsNullOrEmpty(this.NameAttributeValue)
              ? this._defaultGroupName : this.NameAttributeValue)
          : this.Name;

      this._context.CurrentValue = (object) this.CurrentValue;
      InputRadioContext context = this._context;

      EditContext editContext = this.EditContext;

      string str = editContext != null ? editContext.FieldCssClass(this.FieldIdentifier) : (string) null;
      context.FieldClass = str;
}

In .Net 6 this.Name value is used however in .Net8 this.NameAttributeValue is used. So in .Net 8, the Name is set to the property name and not the property value. So the group name is set to Value and not a GUID.

This is a bug. I can see how it would slip though the tests.

javiercn commented 8 months ago

@chrisportelli thanks for contacting us.

@gragra33 very likely not a bug, but a requirement to support form mapping on SSR. In this case, it's very likely that CustomInputRadioGroup needs to extend Editor<T>. See https://learn.microsoft.com/en-us/aspnet/core/blazor/forms/binding?view=aspnetcore-8.0#nest-and-bind-forms

ghost commented 8 months ago

Hi @chrisportelli. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

gragra33 commented 8 months ago

@chrisportelli thanks for contacting us.

@gragra33 very likely not a bug, but a requirement to support form mapping on SSR. In this case, it's very likely that CustomInputRadioGroup needs to extend Editor<T>. See https://learn.microsoft.com/en-us/aspnet/core/blazor/forms/binding?view=aspnetcore-8.0#nest-and-bind-forms

So, a breaking change then?

ghost commented 7 months ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

clebron949 commented 2 months ago

Hello, I'm experiencing this same issue using dotnet8. Where the name attribute is the same for all input radio elements of the foreach loop. Is there a workaround for this issue?