File Input errors handling on drop

[fredericvilcot]

The implementation of the FileInput component has been launched, following specification declared in issue okp4/ui#300.

This discussion is opened in order to decide on the interest of an additional property of this component.

💡Idea

When the user adds one or more files using the drag and drop mechanism, it is possible that the deposited elements do not respect certain integrity rules declared in advance, such as the total size of an upload, the bad file format etc.

🚀 Solution

To allow the component to freely manage this error, we can imagine some solutions:

1. Add an optional onError property which will be a callback method called in the event of an error when dropping elements. This allows the parent to be warned about an eventual error state and thus react (or not) to propagate this error.

onError?: (error: FileInputError) => void

Pros / Cons:

• ✅ allows the parent to be warned about an error state and its whole context
• ❌ error context can be hard to be built and may not be appropriated to a dum component

2. Handle the error directly at the component level and display an error message. This internationalized message will be directly carried by the FileInput component, the parent has no control over it.

Pros / Cons:

• ✅ allows a maximal autonomy and genericity as the component is able to warn the user by itself.
• ❌ the parent has no control of the way warn is displayed and its content

Please, share your ideas about that, do not hesitate to add some propositions! 🙏

[fredericvilcot]

@JoeyDjr @ccamel @lolottetheclash @ErikssonJoakim

[lolottetheclash]

Since the parent passes the constraints to be respected by some properties like acceptedFormats or possibly maxFilesSize, I think that the child component FileUpload could detect and react according to the respect or not of the rules imposed by its parent by adjusting alone its way of communicating the error to the user (via red dashed + an error message for example)
I don't think that it is annoying that the parent doesn't have control over the error display if the message remains linked to the passed property (e.g. "the file doesn't respect the accepted formats", "the file weight exceeds the allowed limit" etc...)

[ccamel]

@fredericvilcot Please align vocabulary, because in the ticket #300 we talk about "File Input Component" and "DragAndDrop component", and here we talk about "FileUpload", so I'm a little bit confused about the scope and responsibility of the component we are talking about here.

[fredericvilcot]

@ccamel I removed all bad names to keep only FileInput as official component name

[JoeyDjr]

@fredericvilcot what did you have in mind concerning the type FileInputError ?
After discussions with @lolottetheclash and @ErikssonJoakim , we think that the FileInput component can provide a prop readonly errorMessages: { multiple: string; format: string } for example that contains all the error messages for each type of error that the component can handle. And then we have FileInputError is a object with only the message parameter (FileInputError: { message: string }) for the v1.
What do you think about that ?

[lolottetheclash]

@fredericvilcot & @JoeyDjr I think that the fileInput component should be able to react to the error by turning its own dotted lines red (as planned) but also remotely send the error to its parent so that its parent has control over the behavior in case of error.
InputFile can send back to its parent an Error.message which contains the I18n keys(or directly the translation) corresponding to either a format error or a single/multiple file error.
If an error is fed back to the parent, then the parent can display the message and add a behavior if it wishes.

[fredericvilcot]

@lolottetheclash
The main issue is, IMHO, that the FileInput component should directly handle the display of the message, not the parent.
The parent could be warned of the error, but if FileInput already displays error, I am not sure that there is a need to add an extra behavior to this...

So, If there is an error:

• onChanged callback is not called
• there is an error message in the drag and drop zone in addition of the red dashed line

I think this is enough for a v0, let me know what do you think

[lolottetheclash]

@fredericvilcot I originally chose the solution n°2 when you proposed two solutions above, but after our discussion, I was thinking we had to implement both solutions 1 & 2 to offer maximum possibilities to the user!

[fredericvilcot]

@lolottetheclash
Yes, I thought so too 😆,but, after thinking about it, I think only choice #2 is sufficient for a first version of the component

[JoeyDjr]

If the FileInputcomponent manage itself the error by displaying an short error message into the component, will this not be too much with the i18n ?
And the messages will be maybe too common, for example, if the parent would like to display the message "Only image are accepted".
In the solution with the errorMessages parameter, a parent can provide the message in this wqy :

<FileInput
            id="fileinput"
            label="Drop files here"
            description={
              <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center' }}>
                <Typography as="div" fontFamily="brand" fontSize="small" fontWeight="xlight">
                  Try to drop several files to handle an error of type `multiple` or try to files in
                  other format than `.png` to handle an error of type `format`.
                </Typography>
              </div>
            }
            multiple={false}
            acceptedFormats={['.png']}
            onDropped={file => {
              setError(null)
            }}
            onError={error => {
              setError(error)
            }}
            errorMessages={{
              multiple: 'Callback method `onError` called for type `multiple`.',
              format: 'Callback method `onError` called for type `format`.'
            }}
          />

What do you think @fredericvilcot @lolottetheclash

[JoeyDjr]

After discusssion with @fredericvilcot @lolottetheclash and @ccamel, we decided the component remain a dumb component.
The parent will give him two props error: boolean and errorMessage and will manage the error state of the component with theses props. The FileInput component will be in an error state and display an error message via the parent.

[fredericvilcot]

Thanks all for your contribution, I lock this discussion