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

fix: SelectedOption/Value not set correctly in FluentCombobox when selected by code #1485

Closed
joriverm opened this issue Feb 9, 2024 · 10 comments · Fixed by #1506
Closed

Comments

@joriverm
Copy link
Contributor

joriverm commented Feb 9, 2024

🐛 Bug Report

When a Combobox has its SelectedOption bound by variable, and set from code behind, the fluent-option gets the selected attribute, but aria-selected is not set correctly, the class is not set to selected and the fluent-combobox's current-value is not updated either.

i saw issue #983 and PR #1149, which sounded like they were related, but this seems to go further than just the properties.
im currently investigating this, but haven't found the problem yet.

💻 Repro or Code Sample

i changed the combobox' storybook "Option Template" case to the following :

@inject DataSource Data

<FluentButton OnClick="@(() => Person = Data.People.FirstOrDefault())">Select first person</FluentButton>
<FluentCombobox Items="@Data.People"
                OptionValue="@(i => i?.PersonId.ToString())"
                @bind-SelectedOption=@Person>
    <OptionTemplate>
        <FluentIcon Value="@(new Icons.Regular.Size16.Person())" Slot="end" OnClick="@(() => DemoLogger.WriteLine($"Icon for {@context?.LastName} selected"))" />
        @context?.FirstName (@context?.LastName)
    </OptionTemplate>
</FluentCombobox>
<p>
    Selected: @Person?.FirstName (@Person?.LastName)
</p>

@code {
    public Person? Person { get; set; } = default!;
}

🤔 Expected Behavior

Selection in combobox to change when the button is pressed

😯 Current Behavior

When a Combobox has its SelectedOption bound by variable, and set from code behind, the fluent-option gets the selected attribute, but nothing else. leaving the control to look like it still has the previous selection

💁 Possible Solution

still investigating to find the bug and fix it. idk if @pk9r327 has an idea?
after some looking around, it seems the overwriting of SetParametersAsync changes the behaviour and doesn't do the selection like it should?

🔦 Context

we have a form that is bound to a model. the combobox is part of a field of said model and can be set/changed by another control.
think food category combobox that gets set to 'drink' when selecting 'milk' in the food drop down

🌍 Your Environment

  • OS & Device: Windows 10, x64 laptop
  • Browser verified in Firefox, Chrome & edge
  • .NET and FAST Version .net 8 & FAST 4.4.1
@vnbaaij
Copy link
Collaborator

vnbaaij commented Feb 9, 2024

Looking into it, but...

you do know a combobox allows people to change the value selected from a list manually, right? So setting food category to 'drink drank drank' by the user when selecting 'milk' in the food drop down is possible and valid and can not be prevented

@joriverm
Copy link
Contributor Author

joriverm commented Feb 9, 2024

Looking into it, but...

you do know a combobox allows people to change the value selected from a list manually, right? So setting food category to 'drink drank drank' by the user when selecting 'milk' in the food drop down is possible and valid and can not be prevented

Hi @vnbaaij ,
thanks for the quick reply!

i have a gut feeling it has to do with ParametersView.Empty being passed to the base, which does the actual selection.
i don't know how those set properties work, but from what i saw when playing with it, it looks to be it. passing the parameters loses the optionstemplate though so something was off or missing in my way of thinking? :)

we are aware of this, and is actually the wanted behavior. the value we are setting in the combobox is the default of the loaded model. the user can change it after its been filled in, incase there is a deviance from the default. :D

a better way to think of it would be when we talk about medicine. medicine A is selected, which has 2 ways to be administered. one is default for medicine A, but can be changed depending on the patient's status :)

@vnbaaij
Copy link
Collaborator

vnbaaij commented Feb 9, 2024

Ok, good to have the use case clear.

It is indeed the SetParameters causing this. It's different from the other list based components. Still digging...

@vnbaaij vnbaaij linked a pull request Feb 12, 2024 that will close this issue
@vnbaaij
Copy link
Collaborator

vnbaaij commented Feb 12, 2024

Hi,

Please check the fix provided in the PR linked above. After we merge this PR in, a preview package containing this fix will be available on NuGet the following day. I will let you know here once that is done.
Once it is available, please test the preview package and let us know the result. If everyone is happy it will be part of our next release. Thanks

@joriverm
Copy link
Contributor Author

Hi Vincent,

Ive looked at the code and i suspect it will work. however, i will check the fix tomorrow while im in the office to verify it!
i had somehow thought something else needed to be fixed besides the setting of the parameters in the override, but guess i was wrong. good to know next time i come across something like this, then i can suggest a PR :)

anyway, thanks for the quick response and i will be in touch tomorrow

@joriverm
Copy link
Contributor Author

@vnbaaij,

i have sent the new version to my coworkers to verify in the application we are making and have verified that it works as expected.
i had some local issues but were due to browser cache, so after a force reload it was a-ok!

thanks a lot for the quick response!

@vnbaaij
Copy link
Collaborator

vnbaaij commented Feb 13, 2024

The PR has been merged (auto closed this issue as well). A preview package for 4.4.2 containing this PR will be released this night at 3AM (auto build pipeline) so will be on NuGet tomorrow morning.

@haefele
Copy link

haefele commented Apr 19, 2024

I am experiencing the same issue, but with a @bind-Value instead of the @bind-SelectedOption.

Just looking through the code a bit, it seems like ListComponentBase.cs is trying to handle this case correctly in it's SetParametersAsync method.
But as FluentCombobox overrides that method, implements its own, and doesn't call the base, it doesn't work correctly.

The logic in SetParametersAsync in the base ListComponentBase and the inheritor FluentCombobox seems extremely similar to me.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Apr 19, 2024

Did you take a look at https://www.fluentui-blazor.net/Combobox#pre-selected-option? It is working correctly and showing what seems to be your use case.

@haefele
Copy link

haefele commented Apr 19, 2024

Ups, my mistake. After trying to create a simple example to show the error, I realized it's a bit of a different setup.
I created a new issue #1900 for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants