Skip to content

fix($drag): offset dragged element relative to first positioned ancestor #64

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

Closed
wants to merge 12 commits into from

Conversation

adam77
Copy link
Contributor

@adam77 adam77 commented Apr 16, 2015

fixes #63
Notes:

At some point the DOM utils stuff could be merged with $dndPosition perhaps.

@coveralls
Copy link

Coverage Status

Coverage increased (+23.94%) to 100.0% when pulling f8b138d on adam77:offset-fix into 97e2cae on caitp:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+23.94%) to 100.0% when pulling f8b138d on adam77:offset-fix into 97e2cae on caitp:master.

@adam77
Copy link
Contributor Author

adam77 commented Apr 16, 2015

heh, can't argue with those coverage stats.

@caitp
Copy link
Owner

caitp commented Apr 16, 2015

definitely, that's awesome. Let me take a look at this

* Provides read-only equivalent of jQuery's offset function:
* http://api.jquery.com/offset/
*/
offset: function (element) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was previously putting helpers like this in src/utils.js --- if moving them here and refactoring, please delete the old dead code and make sure this new code is being used consistently

@coveralls
Copy link

Coverage Status

Coverage increased (+1.39%) to 77.45% when pulling 515c10f on adam77:offset-fix into 97e2cae on caitp:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.39%) to 77.45% when pulling 515c10f on adam77:offset-fix into 97e2cae on caitp:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.39%) to 77.45% when pulling 8ad1df4 on adam77:offset-fix into 97e2cae on caitp:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.39%) to 77.45% when pulling 8ad1df4 on adam77:offset-fix into 97e2cae on caitp:master.

@caitp
Copy link
Owner

caitp commented Apr 17, 2015

something got messed up with the coveralls account --- could you rebase and re-push? I just want to see the accurate coverage info before landing

@coveralls
Copy link

Coverage Status

Coverage increased (+23.94%) to 100.0% when pulling 32fda40 on adam77:offset-fix into 97e2cae on caitp:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+23.94%) to 100.0% when pulling 32fda40 on adam77:offset-fix into 97e2cae on caitp:master.

@caitp
Copy link
Owner

caitp commented Apr 17, 2015

the coverage looks okay (non-regression at least), although for whatever reason coveralls is not reporting the new file on their UI (they seem to be suffering from some new bugs lately!)

@adam77
Copy link
Contributor Author

adam77 commented Apr 17, 2015

i noticed the new travis file has node v0.1?

@caitp
Copy link
Owner

caitp commented Apr 17, 2015

it's the same as 0.10, it's just formatted differently

@caitp
Copy link
Owner

caitp commented Apr 17, 2015

The demo still works nicely, so as far as I'm concerned it's safe to merge --- it would be good to add to the demo app with a little showcase of your fix. LGTM, landing

@caitp caitp closed this in bd583a8 Apr 17, 2015
@adam77 adam77 deleted the offset-fix branch April 17, 2015 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Draggables within positioned containers
3 participants