Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Drag ends if mouse moves outside window while resizing a panel #2117

Closed
peterflynn opened this issue Nov 15, 2012 · 19 comments
Closed

Drag ends if mouse moves outside window while resizing a panel #2117

peterflynn opened this issue Nov 15, 2012 · 19 comments
Assignees

Comments

@peterflynn
Copy link
Member

  1. Make the Brackets window not fullscreen
  2. Grab the sidebar near the bottom of the window & start dragging horizontally
  3. While dragging horizontally, let the mouse drift downward until it's outside the window

Result:
Panel stops resizing. You have to mouseup, move the mouse back up, and start dragging again.

@peterflynn
Copy link
Member Author

This is happening because of the $resizeCont.mouseleave(endResize); in Resizer. If we attach the mousemove/mouseup listener to document I think that line will be unneeded -- you should get mousemove/up on document even when the mouse is outside the window bounds (only while dragging, of course).

@peterflynn
Copy link
Member Author

@jbalsas, fyi -- just noticed this while testing your pull request. If you want to roll this into the existing pull feel free, but we could also wait until later to tackle this...

@jbalsas
Copy link
Contributor

jbalsas commented Nov 15, 2012

Yes... we added that mouseleave() back in #1661 (comment) to fix that releasing outside the window would keep resizing later...

I'll check your suggestion while I work on the review changes for #2092 and push it there if it's simple enough

@redmunds
Copy link
Contributor

@peterflynn That was added by @jbalsas after trying to fix some weird bugs in initial code review, so that line of code cannot simply be removed.

I think problem was that when moving mouse outside Brackets window, the mouse cursor changes to indicate that drag is no longer in effect while Brackets still has mouse capture, which is worse than it is now. So, we need to investigate a solution for both problems.

@pthiess
Copy link
Contributor

pthiess commented Nov 15, 2012

Reviewed - assigning to Randy to continue to work with Chema.

@ghost ghost assigned redmunds Nov 15, 2012
@jbalsas
Copy link
Contributor

jbalsas commented Nov 15, 2012

I've just done a quick check, and document.mouseup/mousemove does indeed allow us to resize even when the mouse is outside of Brackets.

The problem is, as @redmunds points out, that the cursor outside of Brackets is either a regular pointer or the native window resizer (horizontal or vertical) which is even weirder. I've also noticed that sometimes, the native window resizer cursor manages to stay even when we move back to Brackets again.

How would you prefer to handle this?

@peterflynn
Copy link
Member Author

The visual glitch seems preferable to the behavior bug to me, but I'll ask around the core team and see if others disagree...

@redmunds
Copy link
Contributor

I think it's more than a "visual glitch". This is what happened to me:

I was dragging to size panel with a "vertical drag" cursor and the mouse went outside of Brackets. The mouse cursor changed back a normal cursor so I released the mouse thinking that the drag was done. After moving mouse back over Brackets, the panel started going wacky -- that's when I realized that mouse still had the capture. I've seen bugs like this before, so I thought to click the mouse to get out of this state, but it could be a mess for someone that doesn't think of that.

@peterflynn
Copy link
Member Author

@redmunds: that's the bug that would occur if you merely removed the $resizeCont.mouseleave() without any other changes. I agree we shouldn't do that. I'm suggesting we also move the mouseup/mousemove handlers up to document, which avoids the bug you're describing (because document is special-cased by browsers: during mouse capture, it continues to get events even while the cursor is outside the window).

@jbalsas
Copy link
Contributor

jbalsas commented Nov 16, 2012

Back in the day we were actually using mouseup/mousemove on $mainView which had the same issue as $resizeCont...

As @peterflynn points out, listening for the events on document does make resize behave consistently in any case and fixes the wacky-resizing issue @redmunds was referring to although the cursor does weird things.

I can put it together #2092 so you can check for yourselves. I guess we should be able to set the cursor system-wide in the shell to fix the visual glitch. Most of the apps I've checked on Mac seem to be doing so...

@peterflynn
Copy link
Member Author

I'm not seeing the cursor glitch on my Win XP matchine: CEF is maintaining the cursor we set even as you drag outside the window bounds. (Which I think you might get for free via OS mouse capture anyway...). Is the cursor glitch a Mac-only bug?

@jbalsas
Copy link
Contributor

jbalsas commented Nov 16, 2012

Yes, it looks like this is mac-only. I've tested on Win7 and the cursor works just fine.

With that in mind, we could solve this issue within #2092 and create a new mac-only issue for the mouse glitch alone.

@redmunds
Copy link
Contributor

How about fix it for Windows-only (in #2092 if you like), keep it like it is on Mac for now, and use this bug for the Mac-only issue?

@redmunds
Copy link
Contributor

WAIT! There's still a problem on Windows. If you release mouse outside of Brackets Window, then mouse cursor changes to normal, but when you move mouse back over Brackets, it still has the capture. This seems like a slight variation of the same problem.

@peterflynn
Copy link
Member Author

@redmunds: I'm not seeing anything bad on Win with my mockup of the changes (basically replacing $resizeCont with $(window.document) everywhere except the $body.append() call and the remove() call). When you say it still has capture, what do you mean? The panel continues to resize even though the mouse is up? Definitely not seeing that behavior on my XP machine...

@redmunds
Copy link
Contributor

@peterflynn Maybe I misunderstood the proposed changes, but it sounds like the plan was to simply remove the call to $resizeCont.mouseleave() on Windows becasue this is a Mac-only bug. So, that's what I did and was referring to.

When you say it still has capture, what do you mean? The panel continues to resize even though the mouse is up?

Yes, that what I meant.

@peterflynn
Copy link
Member Author

Ok yeah, I think we got our wires crossed then :-) See my comment yesterday -- the proposal isn't just to remove that one line, because that'll cause the bug you've been mentioning. But if we move the mouse listeners onto document then it's safe to remove that line, because document gets mousemove/mouseup events during a drag even when the cursor is outside the window.

I'd like to see this fix get rolled into @jbalsas's patch for both Mac and Win -- the mouse cursor glitch repros in current builds as well, so it's not a regression caused by this fix, and IMHO not having a fix for that yet shouldn't hold us back from fixing the actual behavioral bug sooner.

@jbalsas
Copy link
Contributor

jbalsas commented Nov 16, 2012

Glad to see we got this sorted out :)

I did the tests in the win machine with the overall ${window.document} replace and didn't see any weird behavior. I'm on Mac now, and I must say the mouse glitch is kind of annoying but now you can't think the resize operation has ended as you still see the panel being resized so it's pretty much harmless.

I'll finish putting everything together for the pull request in a moment so we can test the whole thing.

peterflynn added a commit that referenced this issue Nov 21, 2012
Clean up panel resize events; fix bugs with double-clicking to expand/collapse (#2079); fix bug with dragging mouse outside window bounds (##2117)
@peterflynn
Copy link
Member Author

Closing now that Chema's fix has been merged. Randy, did you want to file a follow-up bug about the mouse cursor being wrong if you drag past the window border too slowly?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants