flutter_dropzone icon indicating copy to clipboard operation
flutter_dropzone copied to clipboard

can now get string drops

Open sgehrman opened this issue 2 years ago • 6 comments

sgehrman avatar Mar 24 '22 09:03 sgehrman

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 avatar Mar 25 '22 13:03 deakjahn

@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 avatar Mar 25 '22 14:03 deakjahn

So, what I could suggest is the following:

  • Let's leave the formatting of the .JS file as is. The dartfmt rules only apply to Dart, not JavaScript, anyway. The Dart files are now reformatted.
  • I can't see any interface change, so we shouldn't touch the version number there. You can also leave the rest of the version numbers untouched, that's standard housekeeping during publication, anyway.
  • As much as I can tell, the only real intention was to handle item.kind == 'string'. Could you help me out when this actually happens? I can see it documented here: https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/kind but how can you drag-and-drop it? Then I'll try to recreate your changes in the JS file and maybe we can achieve the same results together. :-)

deakjahn avatar Mar 25 '22 14:03 deakjahn

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 avatar Mar 25 '22 14:03 deakjahn

@sgehrman Shall we proceed with it somehow?

deakjahn avatar Mar 28 '22 10:03 deakjahn

@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 avatar May 15 '22 11:05 deakjahn

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

deakjahn avatar Feb 14 '24 20:02 deakjahn