drag-drop-touch-js/dragdroptouch

Contributing changes

Pomax opened this issue · 8 comments

Pomax commented

Heya,

I was trying to use this library and ended up forking it and "massaging" it a little, possibly more than you'd like, maybe exactly as much as you'd wish you had time to do, so: I have a fork up over on https://github.com/Pomax/dragdroptouch that I wouldn't mind filing as an upstream contribution if you think that's a good idea.

Some of the major changes include:

  • it has a build system now (using esbuild), compiling a drag-drop-touch.esm.js and drag-drop-touch.esm.min.js.
  • on that note, it compiles an ESM file, because it's 2024 and I want to be part of the solution =D
  • the demo page got leaned up, maybe too much for your liking, but as a random drive-by user I have no interesting in wijmo, so the only example that still exists is the box example because if that works, anything works.
  • it looks for an "autoload" parameter (when loaded on its own), which just immediately builds a singleton instance so that "things just work(tm)" rather than someone needing to still call new DragDropTouch().
  • I removed the "supportsPassive" check because it's 2024 and everything does =)
  • I replaced a few loops with different/more modern loops
  • There is a piece of code that had // Added by NDP OK? TODO and reading over it, it wasn't OK in that it first returned if some condition was false, then followed that up with an if statement that could never be true. So that got changed.

Let me know what you'd like me to do here (happy to just keep it a fork, but even happier to get everything landed upstream of course =D).

(there is one wonky bit, where touch-dragging the box with an image doesn't do quite the right thing, but that's something that should be relatively "easy" to fix as part of the upstreaming)

Bernardo-Castilho commented
  • ❤️1
Pomax commented

Cheers! I'll probably tag you on some things for opinions still though, because getting completely sidelined is no fun =D

Bernardo-Castilho commented
Pomax commented

Not to create the impression I've just walked away here, I'm actually wrapping up a bunch of "from the ground up rewrite" work over on https://github.com/Pomax/custom-file-tree including adding integration testing using playwright, which would be fantastic to add here as well, and is the project I was working on that made me find this library in the first place =D

(and of course, v2 takes much longer to finish up than expected)

Bernardo-Castilho commented
  • ❤️1
Pomax commented

Man that always takes longer than you want (decades of dev work and I still can't timebox properly)... anyway, v3 of <file-tree> released, with the interesting part for this library being the testing aspect: using playwright (much nicer than Selenium or Cypress: it uses Chrome, Firefox, and Webkit, without any webdriver nonsense) I've added drag and drop tests for <file-tree> over in https://github.com/Pomax/custom-file-tree/blob/main/tests/move.spec.js and I'm pretty sure that can be used here as well. And of course ideally that means "write tests, create a github action that runs on PRs, good to go".

Pomax commented

All changes were merged in, this issue can be closed =)