Fix column reordering when mouse leaves the browser window. #105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KamranAsif
reviewed
Jan 11, 2017
| this._eventOutToken = EventListener.listen( | ||
| this._domNode, | ||
| 'mouseout', | ||
| this.onMouseLeave |
There was a problem hiding this comment.
Maybe rename onMouseLeave to onMouseEnd?
KamranAsif
reviewed
Jan 11, 2017
| @@ -141,6 +156,13 @@ class DOMMouseMoveTracker { | |||
| } | |||
| this._onMoveEnd(); | |||
There was a problem hiding this comment.
Lets explicitly return false here
sisyphean2
commented
Jan 11, 2017
| @@ -77,11 +88,15 @@ class DOMMouseMoveTracker { | |||
| * These releases all of the listeners on document.body. | |||
| */ | |||
| releaseMouseMoves() { | |||
| if (this._eventMoveToken && this._eventUpToken) { | |||
| if (this._eventMoveToken && this._eventUpToken && this._eventLeaveToken) { | |||
There was a problem hiding this comment.
I need to add this.eventOutToken to this list.
KamranAsif
changed the title
KamranAsif
reviewed
Jan 12, 2017
| this.props.onColumnReorderEndCallback({ | ||
| columnBefore, columnAfter, reorderColumn | ||
| }); | ||
| } | ||
|
|
||
| var onHorizontalScroll = this.props.onHorizontalScroll; |
There was a problem hiding this comment.
I don't think we want to run this onHorizontalScroll code here.
Can we just return early if cancelReorder is true?
There was a problem hiding this comment.
Good call. I only made sure that we want the stuff above the cancelReorder check, I didn't look below it to see if we want to skip anything.
I'll change the contents of the if block to return early and put the Callback after the if block.
|
LGTM after one comment is addressed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add an onMouseLeave function that handles the mouse leaving the browser window. This function closes all listener functionality related to column reordering and cancels the reorder.
Motivation and Context
#104
How Has This Been Tested?
I tested this manually by reordering columns multiple times, dropping the columns both inside and outside the browser window.
Screenshots (if appropriate):
Types of changes
Checklist: