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.

Sign up for GitHub

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

Jump to bottom

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

STRML added 8 commits December 28, 2014 16:00
@STRML
Add body class to disable user-select while drag is active.
8fcebf8
@STRML
Lint fixes
8759b6f
@STRML
Add draggable element to fired drag events.
f044c56 
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.
@STRML
Add moveOnStartChange option.
47d1822 
This allows changes to the `start` param to move the element,
if desired.
@STRML
Add class while dragging.
02ac9ec
@STRML
Add min/max constraints
6306c2e
@STRML
Use pendingState instead of state in UI events; state has not been se…
f1d94f1 
…t yet
@STRML
Only catch left clicks when catching mouse events
ccee798
@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 STRML mentioned this pull request Apr 30, 2015
Closed
STRML added a commit that referenced this pull request May 1, 2015
@STRML
Many fixes for 0.5.0 release. See below.
611c895 
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
Add bounds options.
ca2058a 
Fixes #36, #35, #31.
@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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@STRML @mzabriskie