microsoft / fluentui-blazor

Microsoft Fluent UI Blazor components library. For use with ASP.NET Core Blazor applications
https://www.fluentui-blazor.net
MIT License
3.74k stars 361 forks source link

fix: Setting Loading in FluentButton bound to FluentInputFile breaks it #2754

Open Leon99 opened 2 days ago

Leon99 commented 2 days ago

🐛 Bug Report

After setting Loading in FluentButton bound to FluentInputFile, it won't open the file browse dialog anymore.

💻 Repro or Code Sample

                    <FluentButton
                        Id="form-browse-button"
                        Loading="_formUploading">
                        Upload...
                    </FluentButton>
                <FluentInputFile
                    Accept="application/pdf"
                    AnchorId="form-browse-button"
                    DragDropZoneVisible="false"
                    Id="form-file-uploader"
                    Mode="InputFileMode.SaveToTemporaryFolder"
                    OnCompleted="OnFormUploadCompleted"
                    OnProgressChange="OnFormUploading"/>
@code {
    bool _formUploading;
    void OnFormUploading()
    {
        _formUploading = true;
    }
    async Task OnFormUploadCompleted(IEnumerable<FluentInputFileEventArgs> files)
    {
        _formUploading = false;
    }
}

🌍 Your Environment

vnbaaij commented 1 day ago

I can't find what is causing this. As an alternative I would use the Disabled property instead. Tested that and can confirm it works.

Will leave this open for a bit but it is not going to be treated as a high priority issue.

Leon99 commented 1 day ago

Thanks @vnbaaij, Disabled plus a separate ProgressRing is what I ended up doing as a workaround. Would be good to, at least, make a note of this limitation in the documentation, to make sure others don't spend as much time as I did trying to localize the issue upon testing a number of changes. I'd say it's not an unexpected use case, since file uploading always requires a progress indicator and using Loading on a button seems to be the easiest way to do it.

vnbaaij commented 1 day ago

Thanks @vnbaaij, Disabled plus a separate ProgressRing is what I ended up doing as a workaround. Would be good to, at least, make a note of this limitation in the documentation, to make sure others don't spend as much time as I did trying to localize the issue upon testing a number of changes.

Yes, I'll make a note of that. This is supper 'low hanging fruit' where you can help the community yourself (an us) by creating a simple PR for it.

I'd say it's not an unexpected use case, since file uploading always requires a progress indicator and using Loading on a button seems to be the easiest way to do it.

It is not an unexpected use case indeed, but you are the first person that has reported this in the ~2.5 years we've had this component.

I'm adding the 'community:good-first-issue' and 'community:contribution' labels with the hope that someone is willing to dive into this deeper.