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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Updating EditContext state in FluentSelect #2086

Closed
SteveSandersonMS opened this issue May 23, 2024 · 2 comments 路 Fixed by #2118
Closed

fix: Updating EditContext state in FluentSelect #2086

SteveSandersonMS opened this issue May 23, 2024 · 2 comments 路 Fixed by #2118
Labels
status:in-progress Work is in progress
Milestone

Comments

@SteveSandersonMS
Copy link

馃悰 Bug Report

FluentSelect never marks the bound field as modified in the edit context.

馃捇 Repro or Code Sample

@using Microsoft.FluentUI.AspNetCore.Components

<EditForm Model="@this">
    <p>Modified: @(context.IsModified())</p>

    <FluentSelect TOption="MyThing" @bind-SelectedOption="@currentThing"
        Items="@AllThings" OptionText="@(x => x.DisplayText)" />

    <FluentTextField @bind-Value="@someString" />
</EditForm>

@code {
    MyThing? currentThing;
    string? someString;
    MyThing[] AllThings = new[] { new MyThing(1, "One"), new MyThing(2, "Two") };

    record MyThing(int Id, string DisplayText);
}

馃 Expected Behavior

If you change the FluentSelect dropdown, then "Modified" should become true. Notice that this does happen correctly if you change the text in the FluentTextField box.

馃槸 Current Behavior

FluentSelect never tells the editcontext that the bound value was modified, so "Modified" remains false.

This breaks many scenarios around showing "save" only if the form is edited, or warning the user not to exit if the form has unsaved changes.

馃拋 Possible Solution

As a (rather unpleasant) workaround, the developer could add:

@bind-SelectedOption:after="@(() => { context.NotifyFieldChanged(FieldIdentifier.Create(() => currentThing)); })"

However this should not be necessary as form components should notify the containing EditContext when they mutate the bound field.

@microsoft-github-policy-service microsoft-github-policy-service bot added the triage New issue. Needs to be looked at label May 23, 2024
@SteveSandersonMS
Copy link
Author

I haven't checked all the other FluentUI form components, but they should all notify the EditContext in this way. Some of them already do by virtue of inheriting from Blazor's built-in components.

Sidenote: FluentSelect seems not to be listed in the docs sidebar, and I could only find this page by using the "search" feature.

@vnbaaij
Copy link
Collaborator

vnbaaij commented May 23, 2024

Hi Steve,

This is (indeed) caused by the list components (Listbox, Select, Combobox and AutoComplete) not inheriting form FluentInputBase (which copies Blazor's built-in mechanism for doing this). That is also the reason why we haven't listed them in the 'Form & Inputs' section yet and they are still listed in the general 'Components' section. The search pointed you to the right page.

We definitely want to change this and it is on our 'ToDo' list. I have started a branch for it locally and will continue working on it.

@vnbaaij vnbaaij added status:in-progress Work is in progress and removed triage New issue. Needs to be looked at labels May 23, 2024
@vnbaaij vnbaaij added this to the v4.8 milestone May 23, 2024
@vnbaaij vnbaaij modified the milestones: v4.8, v4.7.3 Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:in-progress Work is in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants