Merge pull request #16863 from atom/fb-mdt-fix-dock-dragndrop

Fix dock dragging and dropping

src/dock.js

@@ -153,7 +153,10 @@ module.exports = class Dock {
     this.state = nextState
     this.render(this.state)
 
-    const {visible} = this.state
+    const {hovered, visible} = this.state
+    if (hovered !== prevState.hovered) {
+      this.emitter.emit('did-change-hovered', hovered)
+    }
     if (visible !== prevState.visible) {
       this.emitter.emit('did-change-visible', visible)
     }
@@ -296,7 +299,7 @@ module.exports = class Dock {
   }
 
   handleDrag (event) {
-    if (!this.pointWithinHoverArea({x: event.pageX, y: event.pageY}, false)) {
+    if (!this.pointWithinHoverArea({x: event.pageX, y: event.pageY}, true)) {
       this.draggedOut()
     }
   }
@@ -313,9 +316,13 @@ module.exports = class Dock {
 
   // Determine whether the cursor is within the dock hover area. This isn't as simple as just using
   // mouseenter/leave because we want to be a little more forgiving. For example, if the cursor is
-  // over the footer, we want to show the bottom dock's toggle button.
-  pointWithinHoverArea (point, includeButtonWidth = this.state.hovered) {
+  // over the footer, we want to show the bottom dock's toggle button. Also note that our criteria
+  // for detecting entry are different than detecting exit but, in order for us to avoid jitter, the
+  // area considered when detecting exit MUST fully encompass the area considered when detecting
+  // entry.
+  pointWithinHoverArea (point, detectingExit) {
     const dockBounds = this.innerElement.getBoundingClientRect()
+
     // Copy the bounds object since we can't mutate it.
     const bounds = {
       top: dockBounds.top,
@@ -324,39 +331,67 @@ module.exports = class Dock {
       left: dockBounds.left
     }
 
-    // Include all panels that are closer to the edge than the dock in our calculations.
+    // To provide a minimum target, expand the area toward the center a bit.
+    switch (this.location) {
+      case 'right':
+        bounds.left = Math.min(bounds.left, bounds.right - 2)
+        break
+      case 'bottom':
+        bounds.top = Math.min(bounds.top, bounds.bottom - 1)
+        break
+      case 'left':
+        bounds.right = Math.max(bounds.right, bounds.left + 2)
+        break
+    }
+
+    // Further expand the area to include all panels that are closer to the edge than the dock.
     switch (this.location) {
       case 'right':
-        if (!this.isVisible()) bounds.left = bounds.right - 2
         bounds.right = Number.POSITIVE_INFINITY
         break
       case 'bottom':
-        if (!this.isVisible()) bounds.top = bounds.bottom - 1
         bounds.bottom = Number.POSITIVE_INFINITY
         break
       case 'left':
-        if (!this.isVisible()) bounds.right = bounds.left + 2
         bounds.left = Number.NEGATIVE_INFINITY
         break
     }
 
-    // The area used when detecting "leave" events is actually larger than when detecting entrances.
-    if (includeButtonWidth) {
+    // If we're in this area, we know we're within the hover area without having to take further
+    // measurements.
+    if (rectContainsPoint(bounds, point)) return true
+
+    // If we're within the toggle button, we're definitely in the hover area. Unfortunately, we
+    // can't do this measurement conditionally (e.g. only if the toggle button is visible) because
+    // our knowledge of the toggle's button is incomplete due to CSS animations. (We may think the
+    // toggle button isn't visible when in actuality it is, but is animating to its hidden state.)
+    //
+    // Since `point` is always the current mouse position, one possible optimization would be to
+    // remove it as an argument and determine whether we're inside the toggle button using
+    // mouseenter/leave events on it. This class would still need to keep track of the mouse
+    // position (via a mousemove listener) for the other measurements, though.
+    const toggleButtonBounds = this.toggleButton.getBounds()
+    if (rectContainsPoint(toggleButtonBounds, point)) return true
+
+    // The area used when detecting exit is actually larger than when detecting entrances. Expand
+    // our bounds and recheck them.
+    if (detectingExit) {
       const hoverMargin = 20
-      const {width, height} = this.toggleButton.getBounds()
       switch (this.location) {
         case 'right':
-          bounds.left -= width + hoverMargin
+          bounds.left = Math.min(bounds.left, toggleButtonBounds.left) - hoverMargin
           break
         case 'bottom':
-          bounds.top -= height + hoverMargin
+          bounds.top = Math.min(bounds.top, toggleButtonBounds.top) - hoverMargin
           break
         case 'left':
-          bounds.right += width + hoverMargin
+          bounds.right = Math.max(bounds.right, toggleButtonBounds.right) + hoverMargin
           break
       }
+      if (rectContainsPoint(bounds, point)) return true
     }
-    return rectContainsPoint(bounds, point)
+
+    return false
   }
 
   getInitialSize () {
@@ -577,6 +612,16 @@ module.exports = class Dock {
     return this.paneContainer.onDidDestroyPaneItem(callback)
   }
 
+  // Extended: Invoke the given callback when the hovered state of the dock changes.
+  //
+  // * `callback` {Function} to be called when the hovered state changes.
+  //   * `hovered` {Boolean} Is the dock now hovered?
+  //
+  // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidChangeHovered (callback) {
+    return this.emitter.on('did-change-hovered', callback)
+  }
+
   /*
   Section: Pane Items
   */
@@ -726,10 +771,7 @@ class DockToggleButton {
   }
 
   getBounds () {
-    if (this.bounds == null) {
-      this.bounds = this.element.getBoundingClientRect()
-    }
-    return this.bounds
+    return this.innerElement.getBoundingClientRect()
   }
 
   update (newProps) {

src/workspace-element.js

@@ -92,7 +92,13 @@ class WorkspaceElement extends HTMLElement {
         window.removeEventListener('dragstart', this.handleDragStart)
         window.removeEventListener('dragend', this.handleDragEnd, true)
         window.removeEventListener('drop', this.handleDrop, true)
-      })
+      }),
+      ...[this.model.getLeftDock(), this.model.getRightDock(), this.model.getBottomDock()]
+        .map(dock => dock.onDidChangeHovered(hovered => {
+          if (hovered) this.hoveredDock = dock
+          else if (dock === this.hoveredDock) this.hoveredDock = null
+          this.checkCleanupDockHoverEvents()
+        }))
     )
     this.initializeContent()
     this.observeScrollbarStyle()
@@ -186,19 +192,13 @@ class WorkspaceElement extends HTMLElement {
   }
 
   updateHoveredDock (mousePosition) {
-    this.hoveredDock = null
-    for (let location in this.model.paneContainers) {
-      if (location !== 'center') {
-        const dock = this.model.paneContainers[location]
-        if (!this.hoveredDock && dock.pointWithinHoverArea(mousePosition)) {
-          this.hoveredDock = dock
-          dock.setHovered(true)
-        } else {
-          dock.setHovered(false)
-        }
-      }
-    }
-    this.checkCleanupDockHoverEvents()
+    // If we haven't left the currently hovered dock, don't change anything.
+    if (this.hoveredDock && this.hoveredDock.pointWithinHoverArea(mousePosition, true)) return
+
+    const docks = [this.model.getLeftDock(), this.model.getRightDock(), this.model.getBottomDock()]
+    const nextHoveredDock =
+      docks.find(dock => dock !== this.hoveredDock && dock.pointWithinHoverArea(mousePosition))
+    docks.forEach(dock => { dock.setHovered(dock === nextHoveredDock) })
   }
 
   checkCleanupDockHoverEvents () {