Merge pull request #14736 from atom/fb-mdt-observe-dock-visibility

Add methods for observing dock visibility

spec/dock-spec.js

@@ -9,12 +9,15 @@ describe('Dock', () => {
     it('opens the dock and activates its active pane', () => {
       jasmine.attachToDOM(atom.workspace.getElement())
       const dock = atom.workspace.getLeftDock()
+      const didChangeVisibleSpy = jasmine.createSpy()
+      dock.onDidChangeVisible(didChangeVisibleSpy)
 
       expect(dock.isVisible()).toBe(false)
       expect(document.activeElement).toBe(atom.workspace.getCenter().getActivePane().getElement())
       dock.activate()
       expect(dock.isVisible()).toBe(true)
       expect(document.activeElement).toBe(dock.getActivePane().getElement())
+      expect(didChangeVisibleSpy).toHaveBeenCalledWith(true)
     })
   })
 
@@ -22,17 +25,24 @@ describe('Dock', () => {
     it('transfers focus back to the active center pane if the dock had focus', () => {
       jasmine.attachToDOM(atom.workspace.getElement())
       const dock = atom.workspace.getLeftDock()
+      const didChangeVisibleSpy = jasmine.createSpy()
+      dock.onDidChangeVisible(didChangeVisibleSpy)
+
       dock.activate()
       expect(document.activeElement).toBe(dock.getActivePane().getElement())
+      expect(didChangeVisibleSpy.mostRecentCall.args[0]).toBe(true)
 
       dock.hide()
       expect(document.activeElement).toBe(atom.workspace.getCenter().getActivePane().getElement())
+      expect(didChangeVisibleSpy.mostRecentCall.args[0]).toBe(false)
 
       dock.activate()
       expect(document.activeElement).toBe(dock.getActivePane().getElement())
+      expect(didChangeVisibleSpy.mostRecentCall.args[0]).toBe(true)
 
       dock.toggle()
       expect(document.activeElement).toBe(atom.workspace.getCenter().getActivePane().getElement())
+      expect(didChangeVisibleSpy.mostRecentCall.args[0]).toBe(false)
 
       // Don't change focus if the dock was not focused in the first place
       const modalElement = document.createElement('div')
@@ -43,9 +53,11 @@ describe('Dock', () => {
 
       dock.show()
       expect(document.activeElement).toBe(modalElement)
+      expect(didChangeVisibleSpy.mostRecentCall.args[0]).toBe(true)
 
       dock.hide()
       expect(document.activeElement).toBe(modalElement)
+      expect(didChangeVisibleSpy.mostRecentCall.args[0]).toBe(false)
     })
   })
 

src/dock.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const _ = require('underscore-plus')
-const {CompositeDisposable} = require('event-kit')
+const {CompositeDisposable, Emitter} = require('event-kit')
 const PaneContainer = require('./pane-container')
 const TextEditor = require('./text-editor')
 const Grim = require('grim')
@@ -35,7 +35,8 @@ module.exports = class Dock {
     this.notificationManager = params.notificationManager
     this.viewRegistry = params.viewRegistry
     this.didActivate = params.didActivate
-    this.didHide = params.didHide
+
+    this.emitter = new Emitter()
 
     this.paneContainer = new PaneContainer({
       location: this.location,
@@ -53,6 +54,7 @@ module.exports = class Dock {
     }
 
     this.subscriptions = new CompositeDisposable(
+      this.emitter,
       this.paneContainer.onDidActivatePane(() => {
         this.show()
         this.didActivate(this)
@@ -135,14 +137,12 @@ module.exports = class Dock {
   setState (newState) {
     const prevState = this.state
     const nextState = Object.assign({}, prevState, newState)
-    let didHide = false
 
     // Update the `shouldAnimate` state. This needs to be written to the DOM before updating the
     // class that changes the animated property. Normally we'd have to defer the class change a
     // frame to ensure the property is animated (or not) appropriately, however we luck out in this
     // case because the drag start always happens before the item is dragged into the toggle button.
     if (nextState.visible !== prevState.visible) {
-      didHide = !nextState.visible
       // Never animate toggling visiblity...
       nextState.shouldAnimate = false
     } else if (!nextState.visible && nextState.draggingItem && !prevState.draggingItem) {
@@ -152,7 +152,11 @@ module.exports = class Dock {
 
     this.state = nextState
     this.render(this.state)
-    if (didHide) this.didHide(this)
+
+    const {visible} = this.state
+    if (visible !== prevState.visible) {
+      this.emitter.emit('did-change-visible', visible)
+    }
   }
 
   render (state) {
@@ -379,12 +383,31 @@ module.exports = class Dock {
     })
   }
 
-  // PaneContainer-delegating methods
-
   /*
   Section: Event Subscription
   */
 
+  // Essential: Invoke the given callback when the visibility of the dock changes.
+  //
+  // * `callback` {Function} to be called when the visibility changes.
+  //   * `visible` {Boolean} Is the dock now visible?
+  //
+  // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  onDidChangeVisible (callback) {
+    return this.emitter.on('did-change-visible', callback)
+  }
+
+  // Essential: Invoke the given callback with the current and all future visibilities of the dock.
+  //
+  // * `callback` {Function} to be called when the visibility changes.
+  //   * `visible` {Boolean} Is the dock now visible?
+  //
+  // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe.
+  observeVisible (callback) {
+    callback(this.isVisible())
+    return this.onDidChangeVisible(callback)
+  }
+
   // Essential: Invoke the given callback with all current and future panes items
   // in the dock.
   //

src/workspace.js

@@ -184,7 +184,6 @@ module.exports = class Workspace extends Model {
     this.didChangeActivePaneOnPaneContainer = this.didChangeActivePaneOnPaneContainer.bind(this)
     this.didChangeActivePaneItemOnPaneContainer = this.didChangeActivePaneItemOnPaneContainer.bind(this)
     this.didActivatePaneContainer = this.didActivatePaneContainer.bind(this)
-    this.didHideDock = this.didHideDock.bind(this)
 
     this.enablePersistence = params.enablePersistence
     this.packageManager = params.packageManager
@@ -270,7 +269,6 @@ module.exports = class Workspace extends Model {
       deserializerManager: this.deserializerManager,
       notificationManager: this.notificationManager,
       viewRegistry: this.viewRegistry,
-      didHide: this.didHideDock,
       didActivate: this.didActivatePaneContainer,
       didChangeActivePane: this.didChangeActivePaneOnPaneContainer,
       didChangeActivePaneItem: this.didChangeActivePaneItemOnPaneContainer,
@@ -321,6 +319,7 @@ module.exports = class Workspace extends Model {
     this.subscribeToFontSize()
     this.subscribeToAddedItems()
     this.subscribeToMovedItems()
+    this.subscribeToDockToggling()
   }
 
   consumeServices ({serviceHub}) {
@@ -484,14 +483,6 @@ module.exports = class Workspace extends Model {
     }
   }
 
-  didHideDock (dock) {
-    const {activeElement} = document
-    const dockElement = dock.getElement()
-    if (dockElement === activeElement || dockElement.contains(activeElement)) {
-      this.getCenter().activate()
-    }
-  }
-
   setDraggingItem (draggingItem) {
     _.values(this.paneContainers).forEach(dock => {
       dock.setDraggingItem(draggingItem)
@@ -513,6 +504,20 @@ module.exports = class Workspace extends Model {
     })
   }
 
+  subscribeToDockToggling () {
+    const docks = [this.getLeftDock(), this.getRightDock(), this.getBottomDock()]
+    docks.forEach(dock => {
+      dock.onDidChangeVisible(visible => {
+        if (visible) return
+        const {activeElement} = document
+        const dockElement = dock.getElement()
+        if (dockElement === activeElement || dockElement.contains(activeElement)) {
+          this.getCenter().activate()
+        }
+      })
+    })
+  }
+
   subscribeToMovedItems () {
     for (const paneContainer of this.getPaneContainers()) {
       paneContainer.observePanes(pane => {