Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Sortable: Improved compatibility with flexbox #407

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CBenni
Copy link

@CBenni CBenni commented Nov 14, 2015

Added detection for flexbox into _isFloating, which improves the detection of where an item is moved to.

See CBenni/jquery-ui@3526e7f
In reference to jquery-ui bug id #9344 (http://bugs.jqueryui.com/ticket/9344), which is reproducable within angular-ui/ui-sortable as well.

Added detection for flexbox into _isFloating, which improves the detection of where an item is moved to.

See CBenni/jquery-ui@3526e7f
In reference to jquery-ui bug id #9344 (http://bugs.jqueryui.com/ticket/9344), which is reproducable within angular-ui/ui-sortable as well.
@thgreasi
Copy link
Contributor

👍 Great Job!
I don't think it needs any extra reviewing, maybe an example to the README and a test case.
I will be happy to merge this as is, as soon as jquery/jquery-ui#1641 is merged!

Thanks for your contribution 👍

@CBenni
Copy link
Author

CBenni commented Nov 14, 2015

As you can probably see (in the jquery-ui PR), I am having a hard time writing a test case for this. I have a small demo that demonstrates the issue, but I have no idea if, and how, one would write a test case for that specific issue, since its simply the UI being jittery/unreliable.

If you can help me with that, ill gladly add a test case.

@thgreasi
Copy link
Contributor

I can try to write a test case for you.
Can you provide me an example codepen? Ideally a fork of one of those that
are available in README.
On the other hand, I'm not sure whether phantomjs supports flex....

On Sat, Nov 14, 2015, 18:32 CBenni notifications@github.com wrote:

As you can probably see (in the jquery-ui PR), I am having a hard time
writing a test case for this. I have a small demo that demonstrates the
issue, but I have no idea if, and how, one would write a test case for that
specific issue, since its simply the UI being jittery/unreliable.

If you can help me with that, ill gladly add a test case.


Reply to this email directly or view it on GitHub
#407 (comment)
.

Thodoris Greasidis
Computer, Networking &
Communications Engineer

@thgreasi
Copy link
Contributor

Hi there. Have you been able to create an example available online to test
this?

On Sat, Nov 14, 2015, 23:45 Thodoris Greasidis thgreasi@gmail.com wrote:

I can try to write a test case for you.
Can you provide me an example codepen? Ideally a fork of one of those that
are available in README.
On the other hand, I'm not sure whether phantomjs supports flex....

On Sat, Nov 14, 2015, 18:32 CBenni notifications@github.com wrote:

As you can probably see (in the jquery-ui PR), I am having a hard time
writing a test case for this. I have a small demo that demonstrates the
issue, but I have no idea if, and how, one would write a test case for that
specific issue, since its simply the UI being jittery/unreliable.

If you can help me with that, ill gladly add a test case.


Reply to this email directly or view it on GitHub
#407 (comment)
.

Thodoris Greasidis
Computer, Networking &
Communications Engineer

Thodoris Greasidis
Computer, Networking &
Communications Engineer

@CBenni
Copy link
Author

CBenni commented Nov 24, 2015

Oh, sorry, I was very busy and forgot about this. See this plunk: http://plnkr.co/edit/UpybaNssJxQMVX30ks0B?p=preview

Trying to move certain objects is very jerky and uncontrollable (try moving 8 to where 9 is)
The behaviour without tolerance: "pointer" is even worse.

The example mimics the use-case I made this change for.

@thgreasi
Copy link
Contributor

Thanks, I will check it out!

@thgreasi
Copy link
Contributor

Thanks for coming back to this repo. I hope I will find some time during
the holidays to check this out.

Thodoris Greasidis
Computer, Networking &
Communications Engineer

@krema
Copy link

krema commented Feb 25, 2016

Hi, any updates? 😃

@thgreasi
Copy link
Contributor

The jquery-ui PR it's still open, but if this gets reported as useful by
more people here, then we can merge it anyway. My only consideration is
whether we should also add a chech about the direction that the flex
elements are rendered.
Any thoughts?

Thodoris Greasidis
Computer, Networking &
Communications Engineer

@CBenni
Copy link
Author

CBenni commented Feb 29, 2016

The problem about this "fix" is that it is very hard to see if it actually fixes all the possible situations. I dont know if it breaks any situation that I had not considered, and even with this fix, the position detection is not always flawless, its a very finnicky thing to mess with. I would wait for the jquery-ui team to approve the PR on their end, honestly.

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

Successfully merging this pull request may close these issues.

3 participants