From 72fe5861018d1ef9ef9162c549f0e9565beec42f Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 10 Jan 2014 17:25:30 -0700 Subject: [PATCH] Remove the concept of focus from the model --- spec/focusable-spec.coffee | 36 ------------------- spec/pane-container-model-spec.coffee | 26 ++++---------- spec/pane-container-spec.coffee | 2 +- spec/pane-model-spec.coffee | 26 +------------- src/focus-context.coffee | 15 -------- src/focusable.coffee | 28 --------------- src/pane-axis-model.coffee | 5 +-- src/pane-axis.coffee | 12 ++++--- src/pane-container-model.coffee | 36 +++---------------- src/pane-container.coffee | 32 +++++++++++++---- src/pane-model.coffee | 27 +++++++-------- src/pane.coffee | 50 ++++++++++++--------------- src/space-pen-extensions.coffee | 3 ++ 13 files changed, 85 insertions(+), 213 deletions(-) delete mode 100644 spec/focusable-spec.coffee delete mode 100644 src/focus-context.coffee delete mode 100644 src/focusable.coffee diff --git a/spec/focusable-spec.coffee b/spec/focusable-spec.coffee deleted file mode 100644 index 0cb26b908..000000000 --- a/spec/focusable-spec.coffee +++ /dev/null @@ -1,36 +0,0 @@ -{Model} = require 'theorist' -Focusable = require '../src/focusable' -FocusContext = require '../src/focus-context' - -describe "Focusable mixin", -> - it "ensures that only a single model is focused for a given focus manager", -> - class Item extends Model - Focusable.includeInto(this) - - focusContext = new FocusContext - item1 = new Item({focusContext}) - item2 = new Item({focusContext}) - item3 = new Item({focusContext}) - - expect(focusContext.focusedObject).toBe null - expect(item1.focused).toBe false - expect(item2.focused).toBe false - expect(item3.focused).toBe false - - item1.focus() - expect(focusContext.focusedObject).toBe item1 - expect(item1.focused).toBe true - expect(item2.focused).toBe false - expect(item3.focused).toBe false - - item2.focus() - expect(focusContext.focusedObject).toBe item2 - expect(item1.focused).toBe false - expect(item2.focused).toBe true - expect(item3.focused).toBe false - - item2.blur() - expect(focusContext.focusedObject).toBe null - expect(item1.focused).toBe false - expect(item2.focused).toBe false - expect(item3.focused).toBe false diff --git a/spec/pane-container-model-spec.coffee b/spec/pane-container-model-spec.coffee index 22c0f8668..dd6e14e60 100644 --- a/spec/pane-container-model-spec.coffee +++ b/spec/pane-container-model-spec.coffee @@ -16,11 +16,10 @@ describe "PaneContainerModel", -> containerB = containerA.testSerialization() [pane1B, pane2B, pane3B] = containerB.getPanes() - expect(pane3B.focusContext).toBe containerB.focusContext expect(pane3B.focused).toBe true it "preserves the active pane across serialization, independent of focus", -> - pane3A.blur() + pane3A.makeActive() expect(containerA.activePane).toBe pane3A containerB = containerA.testSerialization() @@ -34,30 +33,30 @@ describe "PaneContainerModel", -> pane1 = new PaneModel container = new PaneContainerModel(root: pane1) - it "references the first pane if no pane has been focused", -> + it "references the first pane if no pane has been made active", -> expect(container.activePane).toBe pane1 expect(pane1.active).toBe true - it "references the most recently focused pane", -> + it "references the most pane on which ::makeActive was most recently called", -> pane2 = pane1.splitRight() + pane2.makeActive() expect(container.activePane).toBe pane2 expect(pane1.active).toBe false expect(pane2.active).toBe true - pane1.focus() + pane1.makeActive() expect(container.activePane).toBe pane1 expect(pane1.active).toBe true expect(pane2.active).toBe false - it "is reassigned to the next pane if the current active pane is unfocused and destroyed", -> + it "is reassigned to the next pane if the current active pane is destroyed", -> pane2 = pane1.splitRight() - pane2.blur() + pane2.makeActive() pane2.destroy() expect(container.activePane).toBe pane1 expect(pane1.active).toBe true pane1.destroy() expect(container.activePane).toBe null - # TODO: Remove this behavior when we have a proper workspace model we can explicitly focus describe "when the last pane is removed", -> [container, pane, surrenderedFocusHandler] = [] @@ -70,14 +69,3 @@ describe "PaneContainerModel", -> pane.destroy() expect(container.root).toBe null expect(container.activePane).toBe null - - describe "if the pane is focused", -> - it "emits a 'surrendered-focus' event", -> - pane.focus() - pane.destroy() - expect(surrenderedFocusHandler).toHaveBeenCalled() - - describe "if the pane is not focused", -> - it "does not emit an event", -> - expect(pane.focused).toBe false - expect(surrenderedFocusHandler).not.toHaveBeenCalled() diff --git a/spec/pane-container-spec.coffee b/spec/pane-container-spec.coffee index efd43f262..2dd46707f 100644 --- a/spec/pane-container-spec.coffee +++ b/spec/pane-container-spec.coffee @@ -42,7 +42,7 @@ describe "PaneContainer", -> describe ".focusPreviousPane()", -> it "focuses the pane preceding the focused pane or the last pane if no pane has focus", -> container.attachToDom() - pane3.focusout() + $(document.body).focus() # clear focus container.focusPreviousPane() expect(pane3.activeItem).toMatchSelector ':focus' diff --git a/spec/pane-model-spec.coffee b/spec/pane-model-spec.coffee index f17b141c9..77f1db022 100644 --- a/spec/pane-model-spec.coffee +++ b/spec/pane-model-spec.coffee @@ -2,7 +2,6 @@ PaneModel = require '../src/pane-model' PaneAxisModel = require '../src/pane-axis-model' PaneContainerModel = require '../src/pane-container-model' -FocusContext = require '../src/focus-context' describe "PaneModel", -> describe "split methods", -> @@ -80,25 +79,12 @@ describe "PaneModel", -> expect(column.orientation).toBe 'vertical' expect(column.children).toEqual [pane1, pane3, pane2] - it "focuses the new pane, even if the current pane isn't focused", -> + it "sets up the new pane to be focused", -> expect(pane1.focused).toBe false pane2 = pane1.splitRight() expect(pane2.focused).toBe true describe "::removeItemAtIndex(index)", -> - describe "when the removal of the item causes blur to be called on the pane model", -> - it "remains focused if it was before the item was removed", -> - pane = new PaneModel(items: ["A", "B", "C"]) - container = new PaneContainerModel(root: pane) - pane.on 'item-removed', -> pane.blur() - pane.focus() - pane.removeItemAtIndex(0) - expect(pane.focused).toBe true - - pane.blur() - pane.removeItemAtIndex(0) - expect(pane.focused).toBe false - describe "when the last item is removed", -> it "destroys the pane", -> pane = new PaneModel(items: ["A", "B"]) @@ -146,13 +132,3 @@ describe "PaneModel", -> expect(container.root.children).toEqual [pane1, pane2] pane2.destroy() expect(container.root).toBe pane1 - - describe "if the pane is focused", -> - it "shifts focus to the next pane", -> - pane2 = pane1.splitRight() - pane3 = pane2.splitRight() - pane2.focus() - expect(pane2.focused).toBe true - expect(pane3.focused).toBe false - pane2.destroy() - expect(pane3.focused).toBe true diff --git a/src/focus-context.coffee b/src/focus-context.coffee deleted file mode 100644 index c17514471..000000000 --- a/src/focus-context.coffee +++ /dev/null @@ -1,15 +0,0 @@ -{Model} = require 'theorist' - -module.exports = -class FocusContext extends Model - @property 'focusedObject', null - - blurSuppressionCounter: 0 - - isBlurSuppressed: -> - @blurSuppressionCounter > 0 - - suppressBlur: (fn) -> - @blurSuppressionCounter++ - fn() - @blurSuppressionCounter-- diff --git a/src/focusable.coffee b/src/focusable.coffee deleted file mode 100644 index c046e7cd2..000000000 --- a/src/focusable.coffee +++ /dev/null @@ -1,28 +0,0 @@ -Mixin = require 'mixto' - -module.exports = -class Focusable extends Mixin - @included: -> - @property 'focusContext' - @behavior 'focused', -> - @$focusContext - .flatMapLatest((context) -> context?.$focusedObject) - .map((focusedObject) => focusedObject is this) - .distinctUntilChanged() - - focus: -> - throw new Error("Object must be assigned a focusContext to be focus") unless @focusContext - unless @focused - @suppressBlur => - @focusContext.focusedObject = this - - blur: -> - throw new Error("Object must be assigned a focusContext to be blurred") unless @focusContext - if @focused and not @focusContext.isBlurSuppressed() - @focusContext.focusedObject = null - - suppressBlur: (fn) -> - if @focusContext? - @focusContext.suppressBlur(fn) - else - fn() diff --git a/src/pane-axis-model.coffee b/src/pane-axis-model.coffee index ce6a2b54b..f019856e6 100644 --- a/src/pane-axis-model.coffee +++ b/src/pane-axis-model.coffee @@ -12,8 +12,6 @@ class PaneAxisModel extends Model Serializable.includeInto(this) Delegator.includeInto(this) - @delegatesProperty 'focusContext', toProperty: 'container' - constructor: ({@container, @orientation, children}) -> @children = Sequence.fromArray(children ? []) @@ -67,5 +65,4 @@ class PaneAxisModel extends Model @children.splice(index + 1, 0, newChild) reparentLastChild: -> - @focusContext.suppressBlur => - @parent.replaceChild(this, @children[0]) + @parent.replaceChild(this, @children[0]) diff --git a/src/pane-axis.coffee b/src/pane-axis.coffee index 5d535da4a..79ed2cfb1 100644 --- a/src/pane-axis.coffee +++ b/src/pane-axis.coffee @@ -7,10 +7,8 @@ Pane = null module.exports = class PaneAxis extends View initialize: (@model) -> - @subscribe @model.children.onRemoval @onChildRemoved - @subscribe @model.children.onEach @onChildAdded - - @onChildAdded(child) for child in children ? [] + @onChildAdded(child) for child in @model.children + @subscribe @model.children, 'changed', @onChildrenChanged viewForModel: (model) -> viewClass = model.getViewClass() @@ -22,6 +20,12 @@ class PaneAxis extends View removeChild: (child) -> @model.removeChild(child.model) + onChildrenChanged: ({index, removedValues, insertedValues}) => + focusedElement = document.activeElement if @hasFocus() + @onChildRemoved(child, index) for child in removedValues + @onChildAdded(child, index + i) for child, i in insertedValues + focusedElement?.focus() if document.activeElement is document.body + onChildAdded: (child, index) => view = @viewForModel(child) @insertAt(index, view) diff --git a/src/pane-container-model.coffee b/src/pane-container-model.coffee index 2e7ebb3bd..b32f28135 100644 --- a/src/pane-container-model.coffee +++ b/src/pane-container-model.coffee @@ -1,7 +1,6 @@ {Model} = require 'theorist' Serializable = require 'serializable' {find} = require 'underscore-plus' -FocusContext = require './focus-context' PaneModel = require './pane-model' module.exports = @@ -11,7 +10,6 @@ class PaneContainerModel extends Model @properties root: null - focusContext: null activePane: null previousRoot: null @@ -21,13 +19,9 @@ class PaneContainerModel extends Model constructor: -> super - - @focusContext ?= new FocusContext - @subscribe @$root, @onRootChanged deserializeParams: (params) -> - @focusContext ?= params.focusContext ? new FocusContext params.root = atom.deserializers.deserialize(params.root, container: this) params @@ -41,31 +35,6 @@ class PaneContainerModel extends Model getPanes: -> @root?.getPanes() ? [] - getFocusedPane: -> - find @getPanes(), (pane) -> pane.focused - - focusNextPane: -> - panes = @getPanes() - if panes.length > 1 - currentIndex = panes.indexOf(@getFocusedPane()) - nextIndex = (currentIndex + 1) % panes.length - panes[nextIndex].focus() - true - else - @emit 'surrendered-focus' - false - - focusPreviousPane: -> - panes = @getPanes() - if panes.length > 1 - currentIndex = panes.indexOf(@getFocusedPane()) - previousIndex = currentIndex - 1 - previousIndex = panes.length - 1 if previousIndex < 0 - panes[previousIndex].focus() - true - else - false - makeNextPaneActive: -> panes = @getPanes() if panes.length > 1 @@ -78,7 +47,10 @@ class PaneContainerModel extends Model onRootChanged: (root) => @unsubscribe(@previousRoot) if @previousRoot? @previousRoot = root - return unless root? + + unless root? + @activePane = null + return root.parent = this root.container = this diff --git a/src/pane-container.coffee b/src/pane-container.coffee index bf7c1657d..eeb54f74b 100644 --- a/src/pane-container.coffee +++ b/src/pane-container.coffee @@ -17,8 +17,6 @@ class PaneContainer extends View @content: -> @div class: 'panes' - @delegatesMethods 'focusNextPane', 'focusPreviousPane', toProperty: 'model' - initialize: (params) -> if params instanceof PaneContainerModel @model = params @@ -27,7 +25,6 @@ class PaneContainer extends View @subscribe @model.$root, 'value', @onRootChanged @subscribe @model.$activePaneItem.changes, 'value', @onActivePaneItemChanged - @subscribe @model, 'surrendered-focus', @onSurrenderedFocus viewForModel: (model) -> if model? @@ -49,6 +46,8 @@ class PaneContainer extends View @model.root = root?.model onRootChanged: (root) => + focusedElement = document.activeElement if @hasFocus() + oldRoot = @getRoot() if oldRoot instanceof Pane and oldRoot.model.isDestroyed() @trigger 'pane:removed', [oldRoot] @@ -56,14 +55,11 @@ class PaneContainer extends View if root? view = @viewForModel(root) @append(view) - view.makeActive?() + focusedElement?.focus() onActivePaneItemChanged: (activeItem) => @trigger 'pane-container:active-pane-item-changed', [activeItem] - onSurrenderedFocus: => - atom?.workspaceView?.focus() - removeChild: (child) -> throw new Error("Removing non-existant child") unless @getRoot() is child @setRoot(null) @@ -117,3 +113,25 @@ class PaneContainer extends View removeEmptyPanes: -> for pane in @getPanes() when pane.getItems().length == 0 pane.remove() + + focusNextPane: -> + panes = @getPanes() + if panes.length > 1 + currentIndex = panes.indexOf(@getFocusedPane()) + nextIndex = (currentIndex + 1) % panes.length + panes[nextIndex].focus() + true + else + atom.workspaceView?.focus() + false + + focusPreviousPane: -> + panes = @getPanes() + if panes.length > 1 + currentIndex = panes.indexOf(@getFocusedPane()) + previousIndex = currentIndex - 1 + previousIndex = panes.length - 1 if previousIndex < 0 + panes[previousIndex].focus() + true + else + false diff --git a/src/pane-model.coffee b/src/pane-model.coffee index e29dd405d..d4f2e6662 100644 --- a/src/pane-model.coffee +++ b/src/pane-model.coffee @@ -3,18 +3,17 @@ {Model, Sequence} = require 'theorist' Serializable = require 'serializable' PaneAxisModel = require './pane-axis-model' -Focusable = require './focusable' Pane = null module.exports = class PaneModel extends Model atom.deserializers.add(this) Serializable.includeInto(this) - Focusable.includeInto(this) @properties container: null activeItem: null + focused: false @behavior 'active', -> @$container @@ -28,9 +27,6 @@ class PaneModel extends Model @items = Sequence.fromArray(params?.items ? []) @activeItem ?= @items[0] - @subscribe @$container, (container) => - @focusContext = container?.focusContext - @subscribe @items.onEach (item) => if typeof item.on is 'function' @subscribe item, 'destroyed', => @removeItem(item) @@ -38,9 +34,6 @@ class PaneModel extends Model @subscribe @items.onRemoval (item, index) => @unsubscribe item if typeof item.on is 'function' - @when @$focused, => @makeActive() - - @focus() if params?.focused @makeActive() if params?.active serializeParams: -> @@ -59,6 +52,13 @@ class PaneModel extends Model isActive: -> @active + focus: -> + @focused = true + @makeActive() + + blur: -> + @focused = false + makeActive: -> @container?.activePane = this getPanes: -> [this] @@ -119,7 +119,7 @@ class PaneModel extends Model item = @items[index] @showNextItem() if item is @activeItem and @items.length > 1 @items.splice(index, 1) - @suppressBlur => @emit 'item-removed', item, index, destroying + @emit 'item-removed', item, index, destroying @destroy() if @items.length is 0 # Public: Moves the given item to a the new index. @@ -160,11 +160,8 @@ class PaneModel extends Model # Private: Called by model superclass destroyed: -> + @container.makeNextPaneActive() if @isActive() item.destroy?() for item in @items.slice() - if @focused - @container.focusNextPane() - else if @isActive() - @container.makeNextPaneActive() # Public: Prompt the user to save the given item. promptToSaveItem: (item) -> @@ -245,10 +242,10 @@ class PaneModel extends Model if @parent.orientation isnt orientation @parent.replaceChild(this, new PaneAxisModel({@container, orientation, children: [this]})) - newPane = new @constructor(params) + newPane = new @constructor(extend({focused: true}, params)) switch side when 'before' then @parent.insertChildBefore(this, newPane) when 'after' then @parent.insertChildAfter(this, newPane) - newPane.focus() + newPane.makeActive() newPane diff --git a/src/pane.coffee b/src/pane.coffee index dcee94658..2c45de329 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -49,9 +49,6 @@ class Pane extends View @viewsByItem = new WeakMap() @handleEvents() - hasFocus: -> - @is(':focus') or @is(':has(:focus)') - handleEvents: -> @subscribe @model, 'destroyed', => @remove() @@ -63,16 +60,10 @@ class Pane extends View @subscribe @model, 'item-destroyed', @onItemDestroyed @subscribe @model.$active, 'value', @onActiveStatusChanged - @subscribe @model.$focused, 'value', (focused) => - if focused - @focus() unless @hasFocus() - else - @blur() if @hasFocus() - @subscribe this, 'focusin', => @model.focus() @subscribe this, 'focusout', => @model.blur() @subscribe this, 'focus', => - @model.suppressBlur => @activeView?.focus() + @activeView?.focus() false @command 'pane:save-items', => @saveItems() @@ -141,8 +132,8 @@ class Pane extends View @itemViews.children().not(view).hide() @itemViews.append(view) unless view.parent().is(@itemViews) view.show() if @attached - if isFocused - @model.suppressBlur -> view.focus() + view.focus() if isFocused + @activeView = view @trigger 'pane:active-item-changed', [item] @@ -150,7 +141,25 @@ class Pane extends View @trigger 'pane:item-added', [item, index] onItemRemoved: (item, index, destroyed) => - @cleanupItemView(item, destroyed) + if item instanceof $ + viewToRemove = item + else if viewToRemove = @viewsByItem.get(item) + @viewsByItem.delete(item) + + removingLastItem = @model.items.length is 0 + hasFocus = @hasFocus() + + @getContainer().focusNextPane() if hasFocus and removingLastItem + + if viewToRemove? + viewToRemove.setModel?(null) + if destroyed + viewToRemove.remove() + else + viewToRemove.detach() + + # @focus() if hasFocus and not removingLastItem + @trigger 'pane:item-removed', [item, index] onItemMoved: (item, newIndex) => @@ -167,20 +176,6 @@ class Pane extends View activeItemTitleChanged: => @trigger 'pane:active-item-title-changed' - # Private: - cleanupItemView: (item, destroyed) -> - if item instanceof $ - viewToRemove = item - else if viewToRemove = @viewsByItem.get(item) - @viewsByItem.delete(item) - - if viewToRemove? - viewToRemove.setModel?(null) - if destroyed - viewToRemove.remove() - else - viewToRemove.detach() - # Private: viewForItem: (item) -> if item instanceof $ @@ -210,6 +205,7 @@ class Pane extends View @closest('.panes').view() beforeRemove: -> + @getContainer()?.focusNextPane() if @hasFocus() @model.destroy() unless @model.isDestroyed() # Private: diff --git a/src/space-pen-extensions.coffee b/src/space-pen-extensions.coffee index d7504ebe2..02d08f8ab 100644 --- a/src/space-pen-extensions.coffee +++ b/src/space-pen-extensions.coffee @@ -59,6 +59,9 @@ jQuery.fn.destroyTooltip = -> @hideTooltip() @tooltip('destroy') +jQuery.fn.hasFocus = -> + @is(':focus') or @is(':has(:focus)') + jQuery.fn.setTooltip.getKeystroke = getKeystroke jQuery.fn.setTooltip.humanizeKeystrokes = humanizeKeystrokes