Skip to content

Commit

Permalink
{Combobox] Fix loop when changing value (#3300)
Browse files Browse the repository at this point in the history
* Fix #3297
ListComponentBase:
- Add a backing field for InternalValue
- Refine checks in OnParameterSet
- Use check on initialized parameters like FluentInputBase

FluentCobobox:
- Use check on initialized parameters like FluentInputBase
- Invoke SelectedOptionChanged if selectedOption is set to null

* Fix test
  • Loading branch information
vnbaaij authored Feb 5, 2025
1 parent e6e151c commit cd7c33e
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 115 deletions.
77 changes: 76 additions & 1 deletion examples/Demo/Shared/Pages/Lab/IssueTester.razor
Original file line number Diff line number Diff line change
@@ -1 +1,76 @@

@page "/FluentComboBox"

@using System.Linq.Expressions

<Form Model="modelTest" EnableFluentValidation=true>

Check warning on line 5 in examples/Demo/Shared/Pages/Lab/IssueTester.razor

View workflow job for this annotation

GitHub Actions / Build and Deploy Demo site

Found markup element with unexpected name 'Form'. If this is intended to be a component, add a @using directive for its namespace.

Check warning on line 5 in examples/Demo/Shared/Pages/Lab/IssueTester.razor

View workflow job for this annotation

GitHub Actions / Build and Deploy Demo site

Found markup element with unexpected name 'Form'. If this is intended to be a component, add a @using directive for its namespace.

Check warning on line 5 in examples/Demo/Shared/Pages/Lab/IssueTester.razor

View workflow job for this annotation

GitHub Actions / Build and Deploy Demo site

Found markup element with unexpected name 'Form'. If this is intended to be a component, add a @using directive for its namespace.

Check warning on line 5 in examples/Demo/Shared/Pages/Lab/IssueTester.razor

View workflow job for this annotation

GitHub Actions / Build and deploy Demo site

Found markup element with unexpected name 'Form'. If this is intended to be a component, add a @using directive for its namespace.

Check warning on line 5 in examples/Demo/Shared/Pages/Lab/IssueTester.razor

View workflow job for this annotation

GitHub Actions / Build and deploy Demo site

Found markup element with unexpected name 'Form'. If this is intended to be a component, add a @using directive for its namespace.

Check warning on line 5 in examples/Demo/Shared/Pages/Lab/IssueTester.razor

View workflow job for this annotation

GitHub Actions / Build and deploy Demo site

Found markup element with unexpected name 'Form'. If this is intended to be a component, add a @using directive for its namespace.
<FluentCombobox
SelectedOptionChanged="@((selectedOption) => OnSelectedOptionChangedAsync(selectedOption))"
ValueChanged="@((value) => OnValueChangedAsync(value))"
ValueExpression="@(() => modelTest.SelectedValue)"
TOption="OptionItem"
Items="@Items"
OptionValue="@(i => i.Value)"
OptionText="@(i => i.Name)"
OptionDisabled="@(i => i.Disabled)"
OptionSelected="@(i => i.Value == modelTest.SelectedValue)"
Autocomplete=ComboboxAutocomplete.Both
Placeholder="Testing binding" />

<p>@modelTest.SelectedValue</p>
<p>@typedText</p>
</Form>

@code {
private string? typedText;
private OptionItem[] Items { get; set; } = [
new OptionItem { Name = "Test 1", Value = "TEST1", Disabled = false },
new OptionItem { Name = "Test 2", Value = "TEST2", Disabled = false },
new OptionItem { Name = "Test 3", Value = "TEST3", Disabled = false }
];

public class ModelTest
{
public string? SelectedValue { get; set; }
}

private ModelTest modelTest = new();

private async Task OnSelectedOptionChangedAsync(OptionItem? selectedOption)
{
if (selectedOption is not null)
{
Console.WriteLine($"OnSelectedOptionChangedAsync enter modelTest.SelectedValue:{modelTest.SelectedValue}, selectedOption.Value:{selectedOption.Value}.");
if (modelTest.SelectedValue != selectedOption.Value)
{
Console.WriteLine($"OnSelectedOptionChangedAsync before modelTest.SelectedValue:{modelTest.SelectedValue}, selectedOption.Value:{selectedOption.Value}.");
modelTest.SelectedValue = selectedOption.Value;
Console.WriteLine($"OnSelectedOptionChangedAsync after modelTest.SelectedValue:{modelTest.SelectedValue}, selectedOption.Value:{selectedOption.Value}.");
}
}

await Task.CompletedTask;
}

private async Task OnValueChangedAsync(string? text)
{
Console.WriteLine($"OnValueChangedAsync enter modelTest.SelectedValue:{modelTest.SelectedValue}, text:{text}.");
typedText = text;
if (modelTest.SelectedValue != default
&& (string.IsNullOrEmpty(text) || !Items.Select(i => i.Name).Contains(text)))
{
Console.WriteLine($"OnValueChangedAsync before value:{modelTest.SelectedValue}");
modelTest.SelectedValue = default;
Console.WriteLine($"OnValueChangedAsync after value:{modelTest.SelectedValue}");
}

await Task.CompletedTask;
}

public class OptionItem
{
public string? Name { get; set; }
public string? Value { get; set; }
public bool Hidden { get; set; }
public bool Disabled { get; set; }
}
}
83 changes: 45 additions & 38 deletions src/Core/Components/List/FluentCombobox.razor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace Microsoft.FluentUI.AspNetCore.Components;
public partial class FluentCombobox<TOption> : ListComponentBase<TOption>, IAsyncDisposable where TOption : notnull
{
private const string JAVASCRIPT_FILE = "./_content/Microsoft.FluentUI.AspNetCore.Components/Components/List/FluentCombobox.razor.js";
private bool _hasInitializedParameters;

/// <summary />
[Inject]
Expand Down Expand Up @@ -83,56 +84,61 @@ public override async Task SetParametersAsync(ParameterView parameters)
{
parameters.SetParameterProperties(this);

var isSetSelectedOption = false;
TOption? newSelectedOption = default;

foreach (var parameter in parameters)
if (!_hasInitializedParameters)
{
switch (parameter.Name)

var isSetSelectedOption = false;
TOption? newSelectedOption = default;

foreach (var parameter in parameters)
{
case nameof(SelectedOption):
isSetSelectedOption = true;
newSelectedOption = (TOption?)parameter.Value;
break;
default:
break;
switch (parameter.Name)
{
case nameof(SelectedOption):
isSetSelectedOption = true;
newSelectedOption = (TOption?)parameter.Value;
break;
default:
break;
}
}
}

if (isSetSelectedOption && !Equals(_currentSelectedOption, newSelectedOption))
{
if (Items != null)
if (isSetSelectedOption && !Equals(_currentSelectedOption, newSelectedOption))
{
if (Items.Contains(newSelectedOption))
if (Items != null)
{
_currentSelectedOption = newSelectedOption;
if (Items.Contains(newSelectedOption))
{
_currentSelectedOption = newSelectedOption;
}
else if (OptionSelected != null && newSelectedOption != null && OptionSelected(newSelectedOption))
{
// The selected option might not be part of the Items list. But we can use OptionSelected to compare the current option.
_currentSelectedOption = newSelectedOption;
}
else
{
// If the selected option is not in the list of items, reset the selected option
_currentSelectedOption = SelectedOption = default;
await SelectedOptionChanged.InvokeAsync(SelectedOption);
}
}
else if (OptionSelected != null && newSelectedOption != null && OptionSelected(newSelectedOption))
else
{
// The selected option might not be part of the Items list. But we can use OptionSelected to compare the current option.
// If Items is null, we don't know if the selected option is in the list of items, so we just set it
_currentSelectedOption = newSelectedOption;
}
else

// Sync Value from selected option.
// If it is null, we set it to the default value so the attribute is not deleted & the webcomponents don't throw an exception
var value = GetOptionValue(_currentSelectedOption) ?? string.Empty;
if (Value != value)
{
// If the selected option is not in the list of items, reset the selected option
_currentSelectedOption = SelectedOption = default;
await SelectedOptionChanged.InvokeAsync(SelectedOption);
Value = value;
await ValueChanged.InvokeAsync(Value);
}
}
else
{
// If Items is null, we don't know if the selected option is in the list of items, so we just set it
_currentSelectedOption = newSelectedOption;
}

// Sync Value from selected option.
// If it is null, we set it to the default value so the attribute is not deleted & the webcomponents don't throw an exception
var value = GetOptionValue(_currentSelectedOption) ?? string.Empty;
if (Value != value)
{
Value = value;
await ValueChanged.InvokeAsync(Value);
}
_hasInitializedParameters = true;
}

await base.SetParametersAsync(ParameterView.Empty);
Expand All @@ -144,11 +150,12 @@ protected override async Task ChangeHandlerAsync(ChangeEventArgs e)
if (e.Value is not null && Items is not null)
{
var value = e.Value.ToString();
TOption? item = Items.FirstOrDefault(i => GetOptionText(i) == value);
var item = Items.FirstOrDefault(i => GetOptionText(i) == value);

if (item is null)
{
SelectedOption = default;
await SelectedOptionChanged.InvokeAsync(SelectedOption);
}
else
{
Expand Down
Loading

0 comments on commit cd7c33e

Please sign in to comment.