MudBlazor / MudBlazor

Blazor Component Library based on Material design with an emphasis on ease of use. Mainly written in C# with Javascript kept to a bare minimum it empowers .NET developers to easily debug it if needed.
http://mudblazor.com
MIT License
7.91k stars 1.26k forks source link

MudFileUpload throws an exception when selecting more files than specified by MaximumFileCount parameter #7063

Open olizarevichroman opened 1 year ago

olizarevichroman commented 1 year ago

Bug type

Component

Component name

MudFileUpload

What happened?

InvalidOperationException is thrown by the underlying InputFileChangeEventArgs.GetMultipleFiles when user select more files that specified by MudFileUpload.MaximumFileCount

Expected behavior

Exception should not be thrown, validation error should be displayed or at least consumer provided callback should be called

Reproduction link

https://try.mudblazor.com/snippet/GucxEAcQoeBTcSMA

Reproduction steps

  1. Click to button and select more files than specified by MaximumFileCount parameter.
  2. Exception will be thrown.

Relevant log output

Uncaught (in promise) Error: System.InvalidOperationException: The maximum number of files accepted is 2, but 3 were supplied.
   at Microsoft.AspNetCore.Components.Forms.InputFileChangeEventArgs.GetMultipleFiles(Int32 maximumFileCount)
   at MudBlazor.MudFileUpload`1.<OnChange>d__60[[System.Collections.Generic.IReadOnlyList`1[[Microsoft.AspNetCore.Components.Forms.IBrowserFile, Microsoft.AspNetCore.Components.Web, Version=7.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()
   at Microsoft.AspNetCore.Components.ComponentBase.CallStateHasChangedOnAsyncCompletion(Task task)
    at Object.endInvokeDotNetFromJS (blazor.webassembly.js:1:3549)
    at Object.Xt [as endInvokeDotNetFromJS] (blazor.webassembly.js:1:63231)
    at Object.Gt [as invokeJSFromDotNet] (blazor.webassembly.js:1:62728)
    at Object.Ii (dotnet.7.0.5.iqbxf5i7cn.js:5:71974)
    at _mono_wasm_invoke_js_blazor (dotnet.7.0.5.iqbxf5i7cn.js:14:103886)
    at dotnet.wasm:0x1d507
    at dotnet.wasm:0x1c935
    at dotnet.wasm:0xe025
    at dotnet.wasm:0xce95
    at dotnet.wasm:0x1a21ff

Version (bug)

6.4.1

Version (working)

No response

What browsers are you seeing the problem on?

Chrome

On what operating system are you experiencing the issue?

Mac OSX

Pull Request

Code of Conduct

ScarletKuro commented 1 year ago

I think it has to be inside a form if you want validation, but I'm not sure. Can you look @Mr-Technician?

Mr-Technician commented 1 year ago

@ScarletKuro This is unrelated to form validation. There is a call to InputFileChangeEventArgs.GetMultipleFiles that can throw the exception mentioned in the issue. I suppose catching it and adding the exception to the list of errors might make the most sense. Alternatively we could remove the limit but it remains as a performance safeguard.

ScarletKuro commented 1 year ago

In my opinion, the most logical approach would be the possibility to catch it using an ErrorBoundary, document this feature, and let the user decide how to handle it. However, for unknown reasons, this error doesn't propagate up to the ErrorBoundary.

olizarevichroman commented 1 year ago

@ScarletKuro This is unrelated to form validation. There is a call to InputFileChangeEventArgs.GetMultipleFiles that can throw the exception mentioned in the issue. I suppose catching it and adding the exception to the list of errors might make the most sense. Alternatively we could remove the limit but it remains as a performance safeguard.

What is the intension of having MudFileUpload.MaximumFileCount? As I understand it should be used as input validation as well as MudNumericField.Min/MudNumericField.Max e.g.

If not then it's not clear why do we need MudFileUpload.Validation and why MudFileUpload is treated as from component.

Also I would say if we provide 3 files via MudFileUpload.Files having MudFileUpload.MaximumFileCount == 2 then we also should have validation error.

@ScarletKuro @Mr-Technician I can provide PR for this issue if needed

ScarletKuro commented 1 year ago

What is the intension of having MudFileUpload.MaximumFileCount? As I understand it should be used as input validation as well as MudNumericField.Min/MudNumericField.Max e.g.

My guess it's to override the Microsoft's default-value for GetMultipleFiles https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.components.forms.inputfilechangeeventargs.getmultiplefiles?view=aspnetcore-7.0 But @Mr-Technician knows better as he made this this component.

Mr-Technician commented 1 year ago

@ScarletKuro is correct, MudFileUpload.MaximumFileCount is an override for the default parameter of GetMultipleFiles as linked above. This was my best solution at the time to allow users to increase the limit without removing it entirely and removing the performance safeguard it provides. This being said, if an ErrorBoundary could catch the exception that would certainly help improve the UX.

A couple of other points:

Min/Max values serve a different purpose in the components that use them. They are used to prevent the input of out-of-range numeric values either by typing or through the spin buttons. There is no way of preventing a user from attempting to attach more or fewer than the allowed number of files.

Standard behavior for other inputs is that when validation fails, the invalid value is still bound to the property and the error is shown. In this case, the Value can't be set because the underlying args object threw an exception instead of returning anything. This also means that other validation rules (say, one that requires at least one file) would inform the user that no file was included at all.