Draggable: fix spurious blur in dialogs on mousedown#1730
Draggable: fix spurious blur in dialogs on mousedown#1730AttackTheDarkness wants to merge 2 commits into
Conversation
I was running into a problem with a popup menu control in a dialog; clicks weren't working (but keyboard was working fine). It turned out that the menu was getting destroyed before the click event could fire. Tracked down the issue to the way draggable blurs focused controls; it was doing the blur before it ran through the logic to figure out if the drag was actually on the handle. I've moved the blur below these checks, so it'll only blur things if it actually needs to handle the drag. Otherwise, it asserts no opinion on what should and shouldn't be focused, which seems like the way things ought to be. Also, added a unit test to check for the expected behavior.
|
|
||
| testHelper.move( element, 1, 1 ); | ||
|
|
||
| assert.strictEqual( document.activeElement, focusElement.get( 0 ), "test element is focused after mousing down off the handle" ); |
There was a problem hiding this comment.
Why does the focus not move off of the test element after clicking outside of the handle?
There was a problem hiding this comment.
Because firing a mousedown event in an element won't force a blur (although a manual mouse interaction will).
The blur in question is manually triggered by the draggable element. This test is meant to make sure the draggable only does its manual blur if the mousedown occurs in the handle; otherwise it shouldn't do any special handling, since the event is otherwise ignored by the control.
There was a problem hiding this comment.
I don't like that the test is asserting the opposite of the expected behavior. When we finally move to WebDriver, this test would be correct, but for now, I'd prefer if we just mock $.ui.safeBlur() to verify the logic.
Would you mind updating the test to do that? If you need any help, just let us know.
There was a problem hiding this comment.
Fair enough... I don't like that part either :-P
Hooking into safeBlur sounds good. I'll get on that.
Should be less brittle this way, and clearer about what it's trying to test.
|
OK, good to go. |
I was running into a problem with a popup menu control in a dialog; clicks
weren't working (but keyboard was working fine). It turned out that the menu
was getting destroyed before the click event could fire.
Tracked down the issue to the way draggable blurs focused controls; it was
doing the blur before it ran through the logic to figure out if the drag was
actually on the handle. I've moved the blur below these checks, so it'll only
blur things if it actually needs to handle the drag. Otherwise, it asserts no
opinion on what should and shouldn't be focused, which seems like the way
things ought to be.
Also, added a unit test to check for the expected behavior.