Support Drag and Drop of Folders as well as files, fixes #668 #659

humphd · 2017-03-22T22:49:58Z

I saw this tweet yesterday about CodePen's ability to let you drag and drop folders as well as files:

https://twitter.com/CodePen/status/843908650997600256?s=03

When I wrote our drag and drop stuff, browsers only supported files. So I did some research, and sure enough, Edge, Chrome, and Firefox all support it (hilariously, all via a webkit* prefix method). The drag and drop and file api specs are a bit of a dog's breakfast, however, I was able to make it all work.

With this patch, I've reworked our code to support both the new webkit way (which supports folders and files) and also the old way, which only works with folders. Both ways still support auto-expanding of .zip and .tar files contained within.

I could use some help really testing this on various browsers and files/folders. cc @Pomax and @flukeout.

Blackweda · 2017-03-23T00:25:47Z

Your build failed around the eslint:src section.. my PR also failed around that sort of thing. How do we fix that??

humphd · 2017-03-23T00:38:17Z

@Blackweda you just have to fix the lint errors. My code has some, it seems. I'll fix and push a patch. You can test it locally first to make sure lint passes.

humphd · 2017-03-25T23:02:07Z

I also added another commit that fixes https://github.com/mozilla/thimble.mozilla.org/issues/1886, allowing you to drag files/folders onto an existing dir vs. always importing to the project root. My code also auto-opens subdirs as you hover over them, in case they are closed.

humphd · 2017-03-25T23:07:11Z

@Pomax this PR is mostly dedicated to you. I should get you to test it and see if it works well for your use cases. Some things to try/note:

Only some browsers support dragging folders in addition to files. This patch supports whatever the browser allows, using 1 of 2 import strategies.
Dragging and dropping .zip and .tar files still works as before (auto-expanding them)
I've made it possible to drag-and-drop deep into the filetree. If you hover over a dir, it will open it, allowing you to navigate the tree, then drop, and it will use the path you drop on as the root for importing vs. the project root.

I don't have access to Windows and IE/Edge, so I don't know what it does in these browsers. But on Mac, it's working in Safari, Chrome, Firefox, and Opera.

I'm fairly confident that I can do 2 more patches to follow this that will support:

dragging files out of the browser onto your desktop (only Chrome supports this at the moment, but it's very cool, and what Gmail uses)
dragging files around in the file tree to move them.

I've now gotten the hang of the filetree code :)

Pomax · 2017-03-27T22:33:34Z

I tried this with a folder on my desktop called "test" containing C5th86IUoAA-T9w.jpg, a 400KBish JPG, gauss.txt, a 1.5KB plain text file, and test.txt, an 8 byte plain text file. When I drag this into the file tree in both Chrome 56 and Firefox 52 (using Windows 10 Pro x64) the directory gets built, but only the JPG file shows as dir content. The .txt files do not show.

Pomax · 2017-03-27T22:34:38Z

Edge 14 does not like folder drag-and-drop, it would appear. I can't drop a folder onto the filetree in it.

Pomax · 2017-03-27T22:37:25Z

do we filter on extension @humphd? if I rename my .txt files to .js, then they do end up in the filetree...

humphd · 2017-03-27T23:48:05Z

That *.txt vs. *.js issue is new to me, and certainly looks like a bug. I've filed it separate to this: #668.

Folder drag and drop is not going to work in IE/Edge (yet), no, so that's known. Dragging a bunch of files should work, though, in all browsers.

Pomax · 2017-03-27T23:54:23Z

barring that weird extension filtering, this actually seems to work pretty well. I tried dragging the folder into itself a few times. Works pretty well too - does highlight a question though: do we want a file-overwrite warning when a drop operation will overwrite existing files?

Pomax · 2017-03-28T00:02:10Z

if the overwrite issue is known and we have issues outstanding, then this is R+ from me.

humphd · 2017-03-29T21:03:32Z

Thanks for the reviews. I'm going to wait on a few other PRs to land that touch this code, and then I'll circle back, rebase, and try to get this in.

… dropping deep into the filetree.

humphd · 2017-05-08T01:39:04Z

@gideonthomas I've rebased this, and fixed a few things I noticed in testing. Do you want to take a look before I land?

gideonthomas · 2017-05-08T16:48:28Z

sure

gideonthomas

Mainly just nits in the code!

I tested this out and it works well! One thing I noticed was that in Thimble, while dnd'ing folders/files/archives works perfectly if you do it directly even if there is a file in the folder that exceeds that max file size limit. However, if we use the "Upload Files" dialog and drag a folder (or archive or just the file) containing a file that is more than the max file size, it shows the error and then the editor just hangs. I feel like that is a known issue, but I'm not sure. If not, let's file a follow-up to fix in Thimble since we need to fix the strings in the "Upload Files" dialog as well (since we can upload folders now too)

gideonthomas · 2017-05-08T17:10:00Z

src/filesystem/impls/filer/lib/LegacyFileImport.js

+            });
+    };
+
+    exports.create = function(byteLimit) {


rename to options since it's not just byteLimit?

gideonthomas · 2017-05-08T17:11:28Z

src/filesystem/impls/filer/lib/WebKitFileImport.js

+        }
+    };
+
+    exports.create = function(byteLimit) {


same here...options vs. byteLimit?

gideonthomas · 2017-05-08T17:14:01Z

src/project/FileTreeView.js

@@ -538,7 +551,8 @@ define(function (require, exports, module) {
                    className: this.getClasses("jstree-leaf"),
                    onClick: this.handleClick,
                    onMouseDown: this.handleMouseDown,
-                    onDoubleClick: this.handleDoubleClick
+                    onDoubleClick: this.handleDoubleClick,
+                    onDragEnter: this.handleDragEnter


it's interesting that we don't need to bind to this here even though we use this inside handleDragEnter. Maybe it does .call(this, ...)?

gideonthomas · 2017-05-08T17:16:48Z

src/utils/DragAndDrop.js

+        var files = event.dataTransfer.files;
+        var types = event.dataTransfer.types;
+
+        if ((!files || !files.length) && types) { // We only want to check if a string of text was dragged into the editor


if( !(files && files.length) && types )

This was referenced Mar 23, 2017

dragging files/dirs around in the file tree with (deeply) nested relocation #449

Open

Fixed issue #1770 filed in thimble repo #638

Closed

humphd force-pushed the dnd-folders branch from f5faa59 to c238c04 Compare March 25, 2017 23:00

humphd requested a review from Pomax March 25, 2017 23:05

humphd mentioned this pull request Mar 27, 2017

Dragging some file types into browser fails silently (e.g., .txt) #668

Closed

Pomax removed their request for review April 7, 2017 22:57

sdalmeida mentioned this pull request Apr 12, 2017

Fixed mozilla/thimble.mozilla.org#1887: File overwrites without notifying user #712

Closed

humphd added the Launch label Apr 13, 2017

humphd added 5 commits May 7, 2017 20:48

Add support for drag-and-drop folders in supported browsers

3d56728

Fix UploadFiles drag and drop to use new code too

07fd0b1

Fix bug in how processFiles is done from Upload dialog

23dc926

Stop using es6 stuff, keep eslint happy

2b7ac27

Fix https://github.com/mozilla/thimble.mozilla.org/issues/1886: allow…

ee30c12

… dropping deep into the filetree.

humphd force-pushed the dnd-folders branch from c238c04 to ee30c12 Compare May 8, 2017 01:31

humphd assigned gideonthomas May 8, 2017

humphd requested a review from gideonthomas May 8, 2017 01:39

gideonthomas approved these changes May 8, 2017

View reviewed changes

humphd added 2 commits May 8, 2017 15:14

Review fixes, corrected some issues found in testing with Upload dialog

f108c40

Honour drop path when doing unzip/untar

1d1f9a8

humphd changed the title ~~Support Drag and Drop of Folders as well as files~~ Support Drag and Drop of Folders as well as files, fixes #668 May 8, 2017

humphd merged commit 906bf7b into mozilla:master May 8, 2017

humphd mentioned this pull request May 8, 2017

Drag files out of the browser #704

Merged

snyk-bot mentioned this pull request Jan 22, 2020

[Snyk] Fix for 1 vulnerabilities mukuljainx/brackets#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Drag and Drop of Folders as well as files, fixes #668 #659

Support Drag and Drop of Folders as well as files, fixes #668 #659

humphd commented Mar 22, 2017

Blackweda commented Mar 23, 2017 •

edited

Loading

humphd commented Mar 23, 2017

humphd commented Mar 25, 2017

humphd commented Mar 25, 2017

Pomax commented Mar 27, 2017

Pomax commented Mar 27, 2017

Pomax commented Mar 27, 2017

humphd commented Mar 27, 2017

Pomax commented Mar 27, 2017

Pomax commented Mar 28, 2017

humphd commented Mar 29, 2017

humphd commented May 8, 2017

gideonthomas commented May 8, 2017

gideonthomas left a comment

gideonthomas May 8, 2017

gideonthomas May 8, 2017

gideonthomas May 8, 2017

gideonthomas May 8, 2017

Support Drag and Drop of Folders as well as files, fixes #668 #659

Support Drag and Drop of Folders as well as files, fixes #668 #659

Conversation

humphd commented Mar 22, 2017

Blackweda commented Mar 23, 2017 • edited Loading

humphd commented Mar 23, 2017

humphd commented Mar 25, 2017

humphd commented Mar 25, 2017

Pomax commented Mar 27, 2017

Pomax commented Mar 27, 2017

Pomax commented Mar 27, 2017

humphd commented Mar 27, 2017

Pomax commented Mar 27, 2017

Pomax commented Mar 28, 2017

humphd commented Mar 29, 2017

humphd commented May 8, 2017

gideonthomas commented May 8, 2017

gideonthomas left a comment

Choose a reason for hiding this comment

gideonthomas May 8, 2017

Choose a reason for hiding this comment

gideonthomas May 8, 2017

Choose a reason for hiding this comment

gideonthomas May 8, 2017

Choose a reason for hiding this comment

gideonthomas May 8, 2017

Choose a reason for hiding this comment

Blackweda commented Mar 23, 2017 •

edited

Loading