Introduce custom design of FileInputField (#244)

[adamkudrna]

Closes #244.

[mbohal]

If we touch a prop explicitly, it should be listed in the props table.

Also onChange(event); must be called in the handleFileChange(). This will fix the functionality test.

[adamkudrna]

Also onChange(event); must be called in the handleFileChange(). This will fix the functionality test.

I don't understand this part. How can I put onChange(event) inside the handleFileChange function? 🤔

[mbohal]

The component accepts the onChange prop which is a function. This is just a normal prop, that is just a function that can be called or not.

The problem is that as it is now this function never gets called. I guess it should have been passed via transferProps, but it is not because you set onChange attribute on the <input /> explicitely and that takes preference.

So we need to call the onChange function originating from prop manually in the handler.

I'm not so sure if I explained it clearly. Feel free to call me if not.

[adamkudrna]

I think I get what you mean. How about now?

[bedrich-schindler]

Due tu undergoing transformation from Jest to Playwright, I would not spend time on Jest tests if not necessary. If we agree that it is not solvable within minutes, I would leave comment that it will be fixed in #603 that I created for this purpose.

[bedrich-schindler]

We have example in README.md where multiple prop is used. However, if I select multiple files, only the first name is shown. With @mbohal, we agreed that for now, we would suggest:

• For 1 file: show file name
• For >1 files: show something like " files selected"

In future, we can improve it e.g. by Popover that could show list of selected files. But there could be problem with keyboard interaction, so we would stick to simple solution and can talk about further improvements later.

---

Technically, we would change selectedFileName to array selectedFileNames. From peformance point of view, it would be easier to hold single string that would contain name or string " files selected". But it would not work in case that user switches languages, so that's why we recommend using array of names for it.

[bedrich-schindler]

This is not related to this PR, but @mbohal, if I see this and my another suggestion to display number of selected files, I think we need more robust approach for translations. For English it is OK, but I guess there can be languages that can have such language rules for which this approach would not work.

[adamkudrna]

My suspicion is this function is evaluated many times per second (which corresponds to how often the dragover event is being fired). Is it OK?

[bedrich-schindler]

This is how onDragOver is supposed to behave.

I think that setIsDragging is handleDragOver is not necessary. We should be able to solve it without it just using onDragEnter and onDragLeave. We only have to solve problem that onDragLeave is called when dragging over child element.

This can be solved with change in FileInputField.module.scss:

// 2. Prevent pointer events on all children of the root element to not to trigger drag events on children.

@layer components.file-input-field {
// Foundation
.root {
    @include foundation.root();
    
    * {
        pointer-events: none; // 2.
    }
}

Then setIsDragging(true); can be removed from handleDragOver.

CAUTION: handleDragOver must be present here to prevent default behaviour. See https://developer.mozilla.org/en-US/docs/Web/API/HTML_Drag_and_Drop_API/File_drag_and_drop#prevent_the_browsers_default_drag_behavior

[adamkudrna]

I don't know why but dragleave is fired when this element is hovered during dragging. It doesn't happen in Spirit. That's why I double-check the isDragging state is on by calling the hook (repeatedly) on the dragover event. I'm certainly open to any improvements.

[adamkudrna]

Spirit – vanilla JS implementation:

• live demo
• source

Spirit – React implementation:

• live demo
• source

[bedrich-schindler]

This is due to pointer events, see my comment: https://developer.mozilla.org/en-US/docs/Web/API/HTML_Drag_and_Drop_API/File_drag_and_drop#prevent_the_browsers_default_drag_behavior

[bedrich-schindler]

During our call with @mbohal, we agreed on several things:

• Change of onDragOver is described here: Introduce custom design of FileInputField (#244) #601 (comment)
• Due to complexity, we do not want to use synthetic events but rather custom prop that would cover input change and file drop at one place, something like onFilesChanged and document it. This is missing in the documentation yet. Due to its complexity, this can't be omitted.
• We do not want to make drag props available as we do not see benefits now and while do not want to use synthetic event. onChange, it would follow the same approach.
• We are not sure about transferProps. Currently, input is hidden and logic is provided through button and droppable area. While we do not want to provide traditional synthetic event onChange and input is hidden, are transfer props placed on correct element? Is input where all aria attributes should be placed? We frankly do not know yet.