Log in as admin
Go to course -> forum
Add new discussion (note: For this test to happen the maximum number of attachments needs to be set to 1 so that dragging 2 files are one too many)
Select two files on desktop and drag them to file area
Color and text should change in file area while the mouse button is pressed
Drop the files on file area
Error message should be displayed, and one file will be attached.
Delete the attached file and now drag and drop one file
check for color and text on file area (it should be changed) and file should be attached.
delete the file
supplemental test: try to break thsi awesome dnd and report/comment about any issues.

# Currently Drag and drop show color change, it will be nice to change text stating "Drop the files in this area" (Like gmail)
# After maximum file upload is reached (for file manager), DND event should show error message that you can't upload more then x files

# Currently Drag and drop show colour change, it will be nice to change text stating "Drop the files in this area" (Like gmail)
# After maximum file upload is reached (for file manager), DND event should show error message that you can't upload more then x files

Note: I have had to add the 'uploadmessage' element into the FilePicker and FileManager code, as dynamically adding/removing the element as I dragged over the filepicker/filemanager was causing the overlay to occasionally get stuck on (I've not had this problem when showing/hiding the pre-existing overlay element).

Davo Smith
added a comment - 15/Jan/12 3:18 AM First item addressed here:
https://github.com/davosmith/moodle/tree/MDL-31114_dnd_ui_improvements
Diff:
https://github.com/davosmith/moodle/commit/13064ab44bfcbf7a6d6bcf75efb7e03752fe97a6
Note: I have had to add the 'uploadmessage' element into the FilePicker and FileManager code, as dynamically adding/removing the element as I dragged over the filepicker/filemanager was causing the overlay to occasionally get stuck on (I've not had this problem when showing/hiding the pre-existing overlay element).

Just added a second commit to this same branch, to show an 'alert' message when the user attempts to upload past the 'maxfiles' limit. I note that there seems to be a bug in Chrome that is preventing it form displaying the 'spinner' when the alert box pops up. I'm not sure there is any way around this, as stepping through the code after setting a breakpoint causes it to work properly.

Davo Smith
added a comment - 16/Jan/12 3:56 AM Just added a second commit to this same branch, to show an 'alert' message when the user attempts to upload past the 'maxfiles' limit. I note that there seems to be a bug in Chrome that is preventing it form displaying the 'spinner' when the alert box pops up. I'm not sure there is any way around this, as stepping through the code after setting a breakpoint causes it to work properly.
Diff here:
https://github.com/davosmith/moodle/commit/1dc2445a1190b9f8bc814119f99d002a285cfcc8

I've been looking at this now.
The changes look good, the only thing I noted was that we avoid using inline styles. In this case being JS output it doesn't matter too much just noting it.

What I'm not sure about is the change in text.
I'm in two minds about it, personally I think the change in colour is enough to make it clear that a drop action is possible. I can see the rational in a change like this I'm just not sure about the implementation.

Having the text displayed only when the user drags a file over the drop area is a little redundant. I think to be useful we would need to display the text as soon as the user drags a file over the document. (Looked at how Gmail was doing to get an idea about what Raj was asking)

Making the text look more like the underlying text would also help I feel, the shift from left aligned to center aligned is abrasive so far. Perhaps as well this won't be so bad in 1/ above is done.

Let us know your thought Davo and Raj when Davo's had a chance to reply have a look and see what you think.

Sam Hemelryk
added a comment - 19/Jan/12 3:49 PM Hi Davo,
I've been looking at this now.
The changes look good, the only thing I noted was that we avoid using inline styles. In this case being JS output it doesn't matter too much just noting it.
What I'm not sure about is the change in text.
I'm in two minds about it, personally I think the change in colour is enough to make it clear that a drop action is possible. I can see the rational in a change like this I'm just not sure about the implementation.
Having the text displayed only when the user drags a file over the drop area is a little redundant. I think to be useful we would need to display the text as soon as the user drags a file over the document. (Looked at how Gmail was doing to get an idea about what Raj was asking)
Making the text look more like the underlying text would also help I feel, the shift from left aligned to center aligned is abrasive so far. Perhaps as well this won't be so bad in 1/ above is done.
Let us know your thought Davo and Raj when Davo's had a chance to reply have a look and see what you think.
Cheers
Sam

Hello Sam and Davo,
I agree with Sam here. I was hoping to see the old text initially and when user drag and drop the file, with change in color, there should be change in text. It might be redundant, but gives good user experience. I personally like the way gmail shows it, and probably helps in accessibility as well.

Rajesh Taneja
added a comment - 19/Jan/12 4:06 PM Hello Sam and Davo,
I agree with Sam here. I was hoping to see the old text initially and when user drag and drop the file, with change in color, there should be change in text. It might be redundant, but gives good user experience. I personally like the way gmail shows it, and probably helps in accessibility as well.
On the other note, patch looks Great Davo.
Thanks for working on this

Oh, and the inline styles I also normally avoid, but very occasionally, I slip them in (especially when they are more functional than 'design' related - without the 'position:relative' on the filemanager/picker box, the overlay fills the entire page, which seemed a bit too important to leave in a stylesheet somewhere - I could be convinced otherwise...).

Davo Smith
added a comment - 19/Jan/12 7:50 PM - edited OK, so we're looking for:
1. When the user drags a file onto the page, change the styling of the box (and / or text)
2. When the user drags over the box, trigger a secondary change in the styling (and / or text)
If someone wants to specify exactly what should be in the box (in terms of styling / text) in each of these situations, then I'll have a go at implementing it:
a) No files uploaded already, drag (with files attached) enters page
b) No files uploaded already, drag (with files attached) enters upload box
c) Files already uploaded, drag (with files attached) enters page
d) Files already uploaded, drag (with files attached) enters upload box
Oh, and the inline styles I also normally avoid, but very occasionally, I slip them in (especially when they are more functional than 'design' related - without the 'position:relative' on the filemanager/picker box, the overlay fills the entire page, which seemed a bit too important to leave in a stylesheet somewhere - I could be convinced otherwise...).

Davo Smith
added a comment - 22/Jan/12 12:17 AM I've added another commit to the branch to show the 'drop here' message when you drag over the page and to highlight it when you drag over the correct element.
Once Sam/Rajesh have had a chance to look over this, I'll do a bit of cherry-picking / rebasing to tidy up the commit history, before putting this forward for integration.

# Log in as admin
# Go to course -> forum
# Add new discussion
# Select two files on desktop and drag them to file area
# Color and text should change in file area while the mouse button is pressed
# Drop the files on file area
# Error message should be displayed, and one file will be attached.
# Delete the attached file and now drag and drop one file
# check for color and text on file area (it should be changed) and file should be attached.
# delete the file

Davo Smith
added a comment - 14/Feb/12 4:28 PM Note there may be a clash with MDL-31321 , which has just been integrated, as that makes an indentation change to almost all of the javascript file.
If necessary, I'll reapply the changes from this issue, once this week's integrations are over.

Hi Davo,
i'm seeing conflicts with dndupload.js likely from another patch already integrated with changes to that file. (if MDL-31321 , i wish i had seen this issue first.) IF you can rebase (in a separate branch) against current integration.git that would be awesome.

Aparup Banerjee
added a comment - 14/Feb/12 5:23 PM Hi Davo,
i'm seeing conflicts with dndupload.js likely from another patch already integrated with changes to that file. (if MDL-31321 , i wish i had seen this issue first.) IF you can rebase (in a separate branch) against current integration.git that would be awesome.
review still in prog...

Davo Smith
added a comment - 14/Feb/12 6:20 PM I've recreated the patches on top of integration master
Seems to be working locally, so please feel free to attempt integration again.
(Branch name / diff URL are the same)

Aparup Banerjee
added a comment - 15/Feb/12 11:54 AM Thanks for that patch Davo it looks good (even with the so called nasty hack) and its tested fine for me as well (chrome/ubuntu).
Thats been integrated into master only for any further testing.
Tester: heres the challenge - try and break it the drag n drop in this test!

# Log in as admin
# Go to course -> forum
# Add new discussion
# Select two files on desktop and drag them to file area
# Color and text should change in file area while the mouse button is pressed
# Drop the files on file area
# Error message should be displayed, and one file will be attached.
# Delete the attached file and now drag and drop one file
# check for color and text on file area (it should be changed) and file should be attached.
# delete the file

# Log in as admin
# Go to course -> forum
# Add new discussion
# Select two files on desktop and drag them to file area
# Color and text should change in file area while the mouse button is pressed
# Drop the files on file area
# Error message should be displayed, and one file will be attached.
# Delete the attached file and now drag and drop one file
# check for color and text on file area (it should be changed) and file should be attached.
# delete the file

supplemental test: try to break thsi awesome dnd and report/comment about any issues.

In the help item next to "drag and drop available" it says "Note: this may not work with other web browsers" Other web browsers?

Can we have "drop files here to upload" visible by default? And centred to. Having is disappear once the user has uploaded a single file is fine but having it hidden until the user drags a file into the page makes little sense. We're telling them drag and drop is available but not telling them where until they've dragged in a file.

On a related note, the string "drag and drop available" above the file area should be removed. Telling the user that drag and drop is available somewhere on the page is unnecessarily mysterious. It would be better if the file area told the user that they could drag files there and if the area immediately above it, instead of saying "Maximum size for new files: 2MB - drag and drop available", said "Maximum size for new files: 2MB Maximum attachments: 2"

"Error message should be displayed, and one file will be attached."
Testing instructions are missing a small but important detail. For this to happen the maximum number of attachments needs to be set to 1.

If the user drags in more files than than the maximum number of attachments attaching a random subset of their files isn't very helpful. If I am a teacher and I have 2 documents (say, the exam location announcement and the practice exam) to distribute to my students but I'm only allowed 1 attachment its unlikely that I'll say "ah well, that'll have to do" when only 1 of those documents attach while the other vanishes. We're just forcing the user to go through and delete any files that did upload before starting over ie putting those files into a zip then uploading that. We should probably either accept all of the files, or if we can't, then display an error and reject the whole lot.

Should we hold this up for this or should I copy and paste my rant into a new MDL?

Andrew Davis
added a comment - 15/Feb/12 12:52 PM - edited I have some general comments.
In the help item next to "drag and drop available" it says "Note: this may not work with other web browsers" Other web browsers?
Can we have "drop files here to upload" visible by default? And centred to. Having is disappear once the user has uploaded a single file is fine but having it hidden until the user drags a file into the page makes little sense. We're telling them drag and drop is available but not telling them where until they've dragged in a file.
On a related note, the string "drag and drop available" above the file area should be removed. Telling the user that drag and drop is available somewhere on the page is unnecessarily mysterious. It would be better if the file area told the user that they could drag files there and if the area immediately above it, instead of saying "Maximum size for new files: 2MB - drag and drop available", said "Maximum size for new files: 2MB Maximum attachments: 2"
"Error message should be displayed, and one file will be attached."
Testing instructions are missing a small but important detail. For this to happen the maximum number of attachments needs to be set to 1.
If the user drags in more files than than the maximum number of attachments attaching a random subset of their files isn't very helpful. If I am a teacher and I have 2 documents (say, the exam location announcement and the practice exam) to distribute to my students but I'm only allowed 1 attachment its unlikely that I'll say "ah well, that'll have to do" when only 1 of those documents attach while the other vanishes. We're just forcing the user to go through and delete any files that did upload before starting over ie putting those files into a zip then uploading that. We should probably either accept all of the files, or if we can't, then display an error and reject the whole lot.
Should we hold this up for this or should I copy and paste my rant into a new MDL?
Otherwise, it does indeed work fine.

Aparup Banerjee
added a comment - 15/Feb/12 2:30 PM +1 for 'reject the whole lot' of files vs random/1st file acceptance - however i would be tempted to let this pass and stay in master for some more feedback (noting that Davo likes feedback alot)
i had 'For this to happen the maximum number of attachments needs to be set to 1.' by default but adding it to the test here.
my +1 for a separate MDL to refine this further.

# Log in as admin
# Go to course -> forum
# Add new discussion
# Select two files on desktop and drag them to file area
# Color and text should change in file area while the mouse button is pressed
# Drop the files on file area
# Error message should be displayed, and one file will be attached.
# Delete the attached file and now drag and drop one file
# check for color and text on file area (it should be changed) and file should be attached.
# delete the file

supplemental test: try to break thsi awesome dnd and report/comment about any issues.

# Log in as admin
# Go to course -> forum
# Add new discussion (note: For this test to happen the maximum number of attachments needs to be set to 1 so that dragging 2 files are one too many)
# Select two files on desktop and drag them to file area
# Color and text should change in file area while the mouse button is pressed
# Drop the files on file area
# Error message should be displayed, and one file will be attached.
# Delete the attached file and now drag and drop one file
# check for color and text on file area (it should be changed) and file should be attached.
# delete the file

supplemental test: try to break thsi awesome dnd and report/comment about any issues.

Rejecting all files when they would go over the limit is fine by me - I'll put together a patch that does that (very easy to do).

How to tell the user that drag and drop is available (and what that means) is a wider issue, that I'd like a bit more feedback / consensus on. There's already been a bit of back & forth about this (changes to the text / help message, etc). I'll see if I can get any further comments about this.

Davo Smith
added a comment - 15/Feb/12 3:55 PM Handling in a separate issue would be far easier for me, certainly.
Rejecting all files when they would go over the limit is fine by me - I'll put together a patch that does that (very easy to do).
How to tell the user that drag and drop is available (and what that means) is a wider issue, that I'd like a bit more feedback / consensus on. There's already been a bit of back & forth about this (changes to the text / help message, etc). I'll see if I can get any further comments about this.

Eloy Lafuente (stronk7)
added a comment - 17/Feb/12 10:07 AM It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks!
Closing as fixed, heading to zzzZZZzzz, niao