widgets icon indicating copy to clipboard operation
widgets copied to clipboard

Uploader widget

Open tomdye opened this issue 6 years ago • 8 comments

Please add all comments and discussions surround an uploader widget to this issue

tomdye avatar Mar 05 '20 15:03 tomdye

The native <input type="file"> is fairly limited in its behavior. For example, each time the user selects files, newly selected files are not added to an existing list but rather overwrite the existing file list. As such, while any uploader widget would use <input type="file"> under the hood, to be useful it would need to maintain its own internal File[] list that allows files to be added and removed in response to user actions.

At its most basic, <Uploader> would render a button that allows users to select files to upload, a list of selected files, as well as "delete" buttons for each file to remove them from the list. This basic implementation would accept a list of properties that are common to other form controls, as well as a few specific to <input type="file">:

interface UploaderProperties {
  // generic form properties
  disabled?: boolean;
  label?: DNode;
  name?: string;
  required?: boolean;
  onChange?: (selectedFiles: File[]): void;
  
  // specific to file uploading
  /**
   * File-type extensions or MIME types
   */
  accept?: string | string[];
  
  /**
   * Can multiple files be uploaded? Defaults to true.
   */
  multiple?: boolean;
}

A child renderer function could be provided to control the list of selected files. To expose "remove file" functionality to the child renderer, we have two options: add a files property to <Uploader>, or pass a remove function to the child renderer. Since security concerns prohibit browsers from specifying initial files for upload, adding a files property seems misleading and therefore I'm inclined to prefer a remove function:

<Uploader>
  {(files: File[], remove: (files: File | File[]) => void) => (
    <ul>
      {files.map((file) => (
        <li key={file.name}>
          {file.name}
          
          <button onclick={() => remove(file)}>
            {messages.removeFile}
          </button>
        </li>
      )}
    </ul>
  )}

In the future, it would be possible to add a drag-and-drop area (dnd={true}) and remove callbacks to allow custom functionality like upload-on-select:

<Uploader
  multiple={true}
  onChange={(files: File[]) => showProgressAndUpload(files.filter(notUploaded)}
  onRemove={(files: File[]) => showLoaderAndRemove(files)}
/>

mwistrand avatar Mar 20 '20 16:03 mwistrand

UI mock for discussion: https://codesandbox.io/s/admiring-forest-nx0sg?file=/src/main.tsx

The mock has 4 file rows:

  1. Upload in progress (only relevant if we provide XHR uploading)
  2. Upload error (only relevant if we provide XHR uploading)
  3. File selected for upload
  4. File selected for upload
  • How do we handle the <input type="file"> element? I'm assuming we want to hide it and use DOM that we can style better. If we do so we need some way of triggering click on it:
    • Call domNode.click() - don't think this is the Dojo way?
    • Wrap the element in a <label> - any problems/objections to this approach?
  • Do we want to pass files to a child renderer, or just render them ourselves and limit customization to whatever can be done with CSS?
    • I think if we go with a child renderer we should at least provide a default renderer so the widget is closer to being full-featured and just working out of the box
  • Do we want to provide any sort of upload process functionality or integration? e.g. accept an endpoint as a parameter and XHR/fetch to the endpoint?
    • If so we probably want to provide a non-immediate option
interface UploaderChildren {
    buttonLabel?: string; // Choose files…
    dndLabel?: string; // Or drop files here
    renderFiles?(props: { files: File[] }): RenderResult;
}

interface UploaderProperties {
    // generic form properties
    disabled?: boolean;
    name?: string;
    required?: boolean;
    onValue?: (selectedFiles: File[]): void;
  
    // specific to file uploading
    /**
     * File-type extensions or MIME types
     */
    accept?: string | string[];
  
    allowDnd?: boolean;

    /**
     * Can multiple files be uploaded? Defaults to true.
     */
    multiple?: boolean;
}

msssk avatar Aug 26 '20 00:08 msssk

@msssk good questions / points so far. I think that we may well want multiple file uploaders to support different scenarios. For example FileUploader / MultiFileUploader. We also need to ensure that these widgets work in a reactive manner and are capable of being used both controlled / partially controlled.

FileUploader

The basic uploader would use a label as you suggested to wrap the underlying input element and provide both a dojo and a material themed appearance. The basic file uploader would have an onValue callback as you have specified which returns the uploaded file and would offer up the dnd / required / name etc properties you specified. It would not however handle multiple files and would accept only a label child. The dndLabel would most likely be a basic i18n message overridable via the i18nprocess. I believe the uploader may also benefit from validation around filesize / type etc.

MultiFileUploader

The multi file uploader could well have an appearance similar to that of your code pen example but would be opinionated about how it renders it's children rather than offering a child renderer for them. It would use the FileUploader widget to upload the actual files and would allow both controlled and partially controlled interfaces for handling the uploaded files. ie.

interface MultiFileUploaderProperties {
   value?: File[];
   initialValue?: File[];
   onValue(files: File[]) => void;
   onAdd(file: File) => void;
   onRemove(file: File) => void;
}

When the value property is set, the parent would be responsible for managing the value and adding / removing File entries, wheras if only the initialValue property was set, the multi file uploader would handle the adding / removing of files itself and would simply call back to the onValue property and inform the parent of the current file array.

The multi file uploader would also have the same kind of properties as the single file uploader such as required / dnd etc. It would probably also benefit from some validation such as max / min number of files?

This is far from extensive, but just my current thoughts on the above and the codepen example.

What do you think?

tomdye avatar Aug 26 '20 15:08 tomdye

edit to the above, the multi probably only needs onValue and not onAdd / onRemove

tomdye avatar Aug 26 '20 16:08 tomdye

I've created a draft PR (#1540) for this with a basic implementation and a few examples - Basic, Disabled, Multiple.

What would be the value of two separate widgets for single and multi file handling? One challenge I am seeing with that is the need to specify the multiple attribute on the native <input> element. If we don't specify that attribute then we remove a significant part of long-existing UX that allows the user to select multiple files at once from the OS file dialog. If we have a separate MultiFileUploader widget but FileUploader does not allow setting multiple, then we can't use FileUploader internally and basically have to reimplement all of FileUploader within MultiFileUploader.

  • The "Choose files" button is currently implemented as a <label> element wrapping the hidden <input type="file"> element. The label is styled as a button - this seems less than ideal, but I don't know what else to do unless we want to add middleware that allows you to invoke methods on a node so we can call fileInputNode.click()
  • I'm not sure of the best way to pass info to the child renderer - I do think it would be nice if we could pass the theme and i18n messages to the child renderer so it can use them

I think the next best things to work on are validation and DnD. After that maybe look into the widget itself handling XHR uploads? That would allow us to provide a renderer that handles progress display and errors.

msssk avatar Aug 27 '20 05:08 msssk

The idea between the two different widgets would be to provide both a basic and a more complete widget with a file list etc. You could certainly still specify multiple as a property on the basic file uploader to achieve this. The more advanced one would provide the file list / add / remove file functionality as you proposed in your example.

  • re. the styling - looking at the Ant design uploader (https://ant.design/components/upload/) a button seems appropriate. We should probably be able to compose the Button styles across from button.m.css in order to keep this consistent.
  • re. child renderer, I'm not sure what we need to render as children aside from a label as per other input widgets.

tomdye avatar Aug 27 '20 10:08 tomdye

I've split the widgets into two - a base FileUploadInput that is stateless and simply notifies its controlling widget of any file uploads, and a more full-featured FileUploader that controls a FileUploadInput and displays a list of added files.

File uploads and DnD are necessitating some DOM interactions not currently supported by Dojo:

  • In order to provide keyboard a11y we need to be able to call domNode.click() on the hidden input element. Is it ok to use vdom.node directly in FileUploadInput or should this be moved into middleware?
  • File DnD requires using DragEvent. @dojo/framework/core/middleware/drag provides more widely compatible DnD using pointer events, but this does not provide the DataTransfer API for handling files. I've created dnd middleware in the widgets repo
    • Should this be in widgets or framework?
    • How should the event listeners be removed?
    • Feedback on the API and code would be welcome

msssk avatar Aug 28 '20 14:08 msssk

We should really get this over the line

tomdye avatar Mar 22 '21 14:03 tomdye