Skip to content
New issue

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

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

Already on GitHub? # to your account

Adding drag and drop of mails using the SortableJs library #543

Closed

Conversation

NasserNgandu
Copy link
Contributor

Pullrequest

Issues

  • None

Checklist

  • None

How2Test

  • None

Todo

  • None

@NasserNgandu
Copy link
Contributor Author

Here is the link of the task: https://avan.tech/item44205-Cypht-Add-drag-and-drop.

@dumblob
Copy link
Member

dumblob commented Dec 30, 2021

Thanks for taking stab at this!

Before anyone will dive into to the source I wonder whether ~3.7k SLOC library is worth including?

Are there other real (not just hypothetical, not "for future") benefits of the (quite huge I'd say) JS library for Cypht?

@marclaporte
Copy link
Member

Related feature request: #216

@marclaporte
Copy link
Member

@dumblob There is more analysis at #216

I feel the decision is "good enough" and not worth investing more time. But if you or someone come with a better option, we can look into swapping.

Thanks!

@dumblob
Copy link
Member

dumblob commented Jan 15, 2022

@marclaporte I see - thanks for the analysis of "liveness" of the biggest contenders.

On the other hand I'd still prefer evaluating the following 3 criteria (in this order) in addition to project liveness:

  1. the user feel on a smartphone/tablet of the given library (yes, touch support is different and some implementations really feel very weird & unintuitive and some do feel much better)
  2. the simplicity and "smartness" of the API (i.e. how much it'll influence Cypht future design & decisions, how easy is it to learn, how orthogonal to other Cypht JS code it is, etc.)
  3. whether it dictates anything peculiar about the underlying html structure or not (if it does, then it goes the "React" route of constrained bulky widgets which is something Cypht IMHO doesn't want to go)

On the other hand, if the library you want to choose now will not influence Cypht design and APIs and thus will be easily exchangeable, then I'd take the first such library which also suits your devs as that's what counts the most at the beginning. We can later see if we could optimize (do better) in the sense of point (1) above or at least in the sense of the library size.

@marclaporte
Copy link
Member

With the currently proposed integration, 4 lines of code are modified. So the lib is easily interchangeable later. I do expect us to add drag and drop in a few other places like folder management but I don't expect it to influence Cypht.

I tested https://sortablejs.github.io/Sortable/ in touch mode, and it works well. Only issue I noticed in the "Handle" mode: the handle is small and not easy to catch.

SortableJS has MultiDrag, which makes a lot of sense for email:
https://github.com/SortableJS/Sortable/wiki/Dragging-Multiple-Items-in-Sortable

@jasonmunro
Copy link
Member

I'm a bit confused by this PR - What exactly is the point of this? To re-order the list on the client side? The order is not sticky (and will most likely break new message arrival placement in the list) so I really don't understand the use case. @marclaporte can you help me out here?

@marclaporte
Copy link
Member

This is the goal: #216

So pick 3 messages and drag and drop to a folder.

@NasserNgandu
Copy link
Contributor Author

I'm sorry, I misunderstood the task. The code just changes the order of the emails. I'll fix that ASAP.

@jasonmunro
Copy link
Member

I'm sorry, I misunderstood the task. The code just changes the order of the emails. I'll fix that ASAP.

I think this is going to be maybe a bit tricky. Let me know if you get stuck on anything I'm happy to help!

@marclaporte marclaporte changed the title Adding drag and drop of mails using the SortableJs librery Adding drag and drop of mails using the SortableJs library Jan 20, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants