Add settings.setupFormData

[choonkeat]

1. Added settings.setupFormData

Currently settings.extraParams appends more fields to the end, after the file itself (which would be very far down the http post payload). pickier server like S3 direct file upload requires the other fields be in front in order to validate (and reject the upload) - so i need to augment the FormData at the start

But simply copying values over isn't enough, since some extra params are extracted from the file itself, e.g. Content-Type in S3 direct file upload. So the solution is to have parameters augmented via a function instead, and passing the file along.

Optional: with settings.setupFormData, we can deprecate settings.extraParams

2. Added data(inlineattachment) so we can access the inlineAttachment instance.

Ideally I'd use settings.onFileUploadResponse to customize how I'd handle the S3 direct upload response and return false.

Unfortunately, we cannot access most things that the default inlineAttachment.prototype.onFileUploadResponse has access to e.g. this.settings.urlText, this.filenameTag, this.editor

One approach is to obtain the inlineAttachment instance and attach a custom onFileUploadResponse function directly on it.

[Rovak]

I like the extra settings.setupFormData which enables more control of the form data. I don't want to deprecate settings.extraParams yet because it provides a simpler api to add additional parameters.

The second data(inlineattachment) can be solved in another way, in PR #42 there was a change where all functions will have their this context change to the inlineattachment instance, so the parameters you need can be accessed from within the function. This change was not applied to v2.

I favor changing the this context to the inlineattachment, instead of exposing the instance through the jQuery data. That way all plugins will be able to access the inlineattachment instance.

If you could revert the second change, then i will merge it and create another PR which fixes the this context.

[choonkeat]

Dropped data(inlineattachment). Please :eyes:

[Rovak]

LGTM, thanks!

I also merged #47 which fixes 2, please try it out if it works for you

[choonkeat]

Works great. Thanks!

Btw I added code to detect drag drop of file.type.match(/^text/) files, and made this.editor.setValue with the file content (instead of uploading the file). This is done by overwriting inlineAttachment.prototype.isFileAllowed. Not sure if you'd want this in the lib itself, and if you do - how would you like to insert it?

inlineAttachment.prototype.isFileAllowed = function(file) {
  if (this.settings.allowedTypes.indexOf(file.type) >= 0) {
    return true;
  } else if (file.type.match(/^text/)) {
    if (file.getAsString) {
      // copy paste text, ignore
    } else if (window.FileReader) {
      var that = this;
      var reader = new FileReader();
      reader.onload = function() {
        var current_value = that.editor.getValue();
        if (current_value) {
          that.editor.setValue(current_value + "\n\n" + this.result);
        } else {
          that.editor.setValue(this.result);
        }
      }
      reader.readAsText(file)
    }
  }
};

[Rovak]

I think the best place would be onFileReceived

var options = {
  onFileReceived: function(file) {
    if (file.type.match(/^text/) && !file.getAsString) {
      var that = this;
      var reader = new FileReader();
      reader.onload = function() {
        var current_value = that.editor.getValue();
        if (current_value) {
          that.editor.setValue(current_value + "\n\n" + this.result);
        } else {
          that.editor.setValue(this.result);
        }
      }
      reader.readAsText(file);
      return false; // return false so default behavior is overriden
    }
  }
};

[choonkeat]

oops, thanks for the cleanup! unfortunately this code sits after isFileAllowed need to get pass allowedTypes.indexOf(file.type). it gets a little impractical to list possible text/*file types text/plain, text/html, text/markdown, text/javascript, ...

[Rovak]

hmm, i guess it would make sense to add an extra hook which is called before any validations, something like beforeFileReceived

[choonkeat]

Or maybe regexp/customizable isFileAllowed is sufficiently appropriate?

[Rovak]

It doesn't feel right to do file handling in a hook that is meant to check if a file is allowed or not. I made a new issue #50 to think of an API to allow this. Or i will add this feature to the plugin.

[choonkeat]

doesn't feel right to do file handling in a hook that is meant to check if a file is allowed or not

agreed