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

New features: moveOnStartChange, event updates, and more #31

Closed
wants to merge 8 commits into from
Closed

New features: moveOnStartChange, event updates, and more #31

wants to merge 8 commits into from

Conversation

STRML
Copy link
Collaborator

@STRML STRML commented Dec 28, 2014

This is kind of a large PR so please let me know if you'd like it split up.

This is some of the work I did on Draggable to help it play nice with my new grid project.

The changes are:

  • JSHint fixes
  • Add moveOnStartChange. This flag tells the Draggable to update its left and top when the start param changes. This is used in React-Grid-Layout to move the Draggables when the layout updates.
  • Added the DOM Node to the fired event. I use this in RGL for a few reasons and it's not reliably available via e.target, and e.currentTarget is set incorrectly for some reason. I thought it would be helpful to have.
  • Added a class while dragging the element.
  • Added a class to the body while dragging to disable user-select. Without it, it's easy to get blue highlights all over your page.

Let me know what you think - if possible it would be great to merge all of the changes rather than split them up.

Thanks

It is not always reliable to use the event itself to find the draggable base.
In testing in Chrome, event.currentTarget was always window.
event.target was always the clicked element, which may not always be
the draggable instance if it has children.

The element passed to the event ensures that you always have access to
the base element if you need it.
This allows changes to the `start` param to move the element,
if desired.
@mzabriskie
Copy link
Collaborator

So I just refactored this component on a separate branch. I am trying to decide what is the best way to handle merging all this. Let me pick through your PR and see what makes sense.

@STRML
Copy link
Collaborator Author

STRML commented Jan 6, 2015

Sure. I'm running a fork in the react-grid-layout project, would be nice
to get rid of the github dep though; maybe will fork and rename if we
can't get it merged. I think they're useful additions though. Sorry
they're not more split up but I was working fast.
On 1/5/15 7:27 PM, Matt Zabriskie wrote:

So I just refactored this component on a separate branch. I am trying
to decide what is the best way to handle merging all this. Let me pick
through your PR and see what makes sense.


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

STRML added a commit that referenced this pull request May 1, 2015
Fixes:
* #48 (draggable start at current position),
* #34 (move element from javascript),
* #46 (spaces to tabs),
* Parts of #31 ('moveOnStartChange', add node to drag callbacks,
     fix pendingState in drag callbacks)
* Linting errors
* Documentation
STRML added a commit that referenced this pull request May 2, 2015
@STRML
Copy link
Collaborator Author

STRML commented May 2, 2015

All of these updates are finally merged.

@STRML STRML closed this May 2, 2015
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.

None yet

2 participants