Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add drag and drop support #519

Closed
wants to merge 3 commits into from
Closed

Conversation

oparoz
Copy link
Contributor

@oparoz oparoz commented Feb 12, 2016

Replaced by #581

@oparoz oparoz added this to the 9.1-next milestone Feb 13, 2016
@oparoz oparoz self-assigned this Feb 13, 2016
@oparoz oparoz mentioned this pull request Feb 13, 2016
8 tasks
@oparoz oparoz force-pushed the official-drag-and-drop-support branch from 9666995 to 05c5fef Compare February 15, 2016 02:09
@jancborchardt
Copy link
Member

Nice! :) Only some details:

  • the dashed border around albums looks a bit off. What about dotted, and rather a color of rgba(0,0,0,.5)?
  • the message »target exists« is too technical. Should be »File already exists.« (or Folder if it’s a folder). Or could we have a replacement dialog like in Files? cc @butonic

@oparoz
Copy link
Contributor Author

oparoz commented Feb 15, 2016

the dashed border around albums looks a bit off.

I agree.

What about dotted, and rather a color of rgba(0,0,0,.5)?

Dotted didn't work in my previous tests, but I'll try with your colour suggestion.
Maybe There should just be a plain colour, but that might look strange with transparent pictures

the message »target exists« is too technical. Should be »File already exists.« (or Folder if it’s a folder). Or could we have a replacement dialog like in Files?

I've copied the behaviour and message from Files, so it's best to open an issue there and then I can sync with any changes made there. It's not possible to have a pre-move conflict dialogue as the content of the target is not known beforehand. An improvement in Files could be to have a post-move dialogue.

@oparoz
Copy link
Contributor Author

oparoz commented Feb 16, 2016

Tried dotted again and saw why it doesn't work. Each side is dotted instead of having one dotted line going all the way around the square. This makes the corners look ugly. By using a dashed line, the problem is almost non-existent.

@oparoz oparoz force-pushed the official-drag-and-drop-support branch from c173c0a to 8a90e77 Compare February 16, 2016 23:07
@oparoz
Copy link
Contributor Author

oparoz commented Feb 16, 2016

I've improved the look of the drop target. I think it looks good enough now.
Bonus: mobile support.

@oparoz oparoz force-pushed the official-drag-and-drop-support branch 2 times, most recently from a014380 to 093dedc Compare February 17, 2016 16:20
@oparoz
Copy link
Contributor Author

oparoz commented Feb 17, 2016

On mobile, you need to tap and hold to be able to move elements. It has to be done this way in order to not have a conflict with scrolling.

@oparoz oparoz force-pushed the official-drag-and-drop-support branch from 093dedc to ae954ce Compare February 17, 2016 17:31
@oparoz oparoz force-pushed the next-9.1 branch 2 times, most recently from ec94b9f to d982e0e Compare March 1, 2016 00:41
@oparoz
Copy link
Contributor Author

oparoz commented Mar 10, 2016

This could also use some testing: @tahaalibra @viraj96 @imjalpreet @mbtamuli

@tahaalibra
Copy link
Contributor

I tested and found various issues:

Public

  • the thumbnail view is gone and plain list is started as default
  • when i changed the view to thumbnail...i can drag and drop the photo..although it does not succeed, i think we should remove drag and drop from public view

Logged In

  • when i try moving a folder f1 to another folder f2. i get a js error
    "gallery.js?v=22125a1…:394 Uncaught TypeError: targetPath.charAt is not a function"
    also the loading animation on f2 does not go away(i thinks this is because of the js error)
    selection_002

NOTE: this is happening only when i try to move one album into another at the fist page of gallery(outside of all albums)

-when i move photos using breadcrumbs..i don't have any loading gif or any indicator..this can be improved i guess..

@oparoz
Copy link
Contributor Author

oparoz commented Mar 10, 2016

Public

the thumbnail view is gone and plain list is started as default

You need to copy the link from the Gallery side or install the ShareLinks app to get the Gallery link, otherwise you land on the Files side.

i can drag and drop the photo..although it does not succeed

Make sure you've given the "editing" permission on the link, but there is indeed a bug. I need to use the public endpoint for the MOVE operation.

Logged in

when i try moving a folder f1 to another folder f2. i get a js error "gallery.js?v=22125a1

I can't reproduce. I thought it was because of the short name, but I don't have that issue.
Try to see if you can figure out why. Maybe it's a browser issue?

when i move photos using breadcrumbs..i don't have any loading gif or any indicator..this can be improved i guess..

Yes, I think it would be a nice improvement to be added in the future

@tahaalibra
Copy link
Contributor

@oparoz

Public

  • I copied the link from the gallery side, but still public starts in list mode
    selection_003

Logged In

To reproduce the javascript error try this
On the files:

  • create a folder named(without quotes) "12345" and add 2 or 3 images
  • create another folder named(without quotes) "New Folder" and add 2 or 3 images
  • visit gallery from the menu
  • try move New Folder to 12345

i think this behavior is coming when i move to a folder that has a number name

@oparoz
Copy link
Contributor Author

oparoz commented Mar 10, 2016

Thanks for the screenshot. Please open a bug report regarding the generated link. You have "short links" enabled somehow and those are not properly converted to Gallery links.

Thanks for the test case. I was able to reproduce it. Strange bug :)

@tahaalibra
Copy link
Contributor

i guess the link generation bug is unique to official-drag-and-drop-support branch because it was not on the master or official-upload-support branch..should i still report it?

@oparoz
Copy link
Contributor Author

oparoz commented Mar 10, 2016

Ah, thanks for checking. It's because this hasn't been rebased...
I'll push a new version in a minute, with the fixes as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants