deakjahn / flutter_dropzone

A drag-and-drop Flutter plugin (Web). Only web and only from outside into Flutter.
https://pub.dev/packages/flutter_dropzone
85 stars 36 forks source link

can now get string drops #54

Closed sgehrman closed 5 months ago

deakjahn commented 2 years ago

Let's try to sort it out because it seems to complicated to evaluate now. :-))

Part of it is due to the braindead formatting decisions of dartfmt, I can see that. It's stupid and abominable but enforced, I know. So, I'll upload the current version with formatting to get it out of the equation, and let's pick it up then.

Also, I needed to remove the .lock file, it's not required at all. Please, remove it manually from your fork, too.

Then, we really don't want to have breaking change version numbers unless absolutely necessary, so no way to jump that high for sure (2 to 4). First of all, let's decide what the real purpose of the modification is, whether it really means any change in the interface or not. If we do need to change the interface, that is a convoluted process requiring two or three separate steps, it can't be done in a single PR, anyway.

deakjahn commented 2 years ago

@sgehrman Oops, I expected GitHub to be smarter and to realize that the baseline became closer to your version with me applying the same formatting changes (expecting those to cancel out). But obviously it lost track somehow. :-)

deakjahn commented 2 years ago

So, what I could suggest is the following:

deakjahn commented 2 years ago

I now practically have this (not yet uploaded, I didn't want to make even bigger of a mess than it already is):

drop_handler(event) {
  event.preventDefault();

  var files = [];
  var strings = [];
  if (event.dataTransfer.items) {
    for (var i = 0; i < event.dataTransfer.items.length; i++) {
      var item = event.dataTransfer.items[i];
      switch (item.kind) {
        case "file":
          if (this.dropMIME == null || this.dropMIME.includes(item.type)) {
            var file = item.getAsFile();
            if (this.onDrop != null) this.onDrop(event, file);
            files.push(file);
          }
          break;

        case "string":
          const that = this;
          item.getAsString(function (text) {
            if (that.onDrop != null) that.onDrop(event, text);
            strings.push(text);
          });
          break;

        default:
          if (this.onError != null) this.onError("Wrong type: ${item.kind}");
          break;
      }
    }
  } else {
    for (var i = 0; i < ev.dataTransfer.files.length; i++)
      var file = event.dataTransfer.files[i];
      if (this.onDrop != null) this.onDrop(event, file);
      files.push(file);
  }

  if (this.onDropMultiple != null) {
    if (files.length > 0) this.onDropMultiple(event, files);
    if (strings.length > 0) this.onDropMultiple(event, strings);
  }
}

I think I grabbed your intention correctly. The only real difference is that I'd stick to the if (this.onDrop != null) format, it does work without the full condition, of course, but this seems to be more in line with the intention and clear coding practices.

deakjahn commented 2 years ago

@sgehrman Shall we proceed with it somehow?

deakjahn commented 2 years ago

@sgehrman Now I really lost track of where we are... :-)

The new lints are OK, the SDK version is not important because it works with the older ones as well. I most definitely don't want to reinclude the .lock files in Git.

Git still shows lots of conflicts to be resolved, so this won't help us. Now I uploaded the suggested changes to the JS file. Could you just check those and see if those are what you intended to include? I have the feeling that that would be an easier way out than to try to salvage this PR with what Git made out of it. :-)

deakjahn commented 5 months ago

Probably solved since, if not, please, start a new PR.