diff --git a/spec/workspace-spec.js b/spec/workspace-spec.js index 4ab183886..bf31a8e3f 100644 --- a/spec/workspace-spec.js +++ b/spec/workspace-spec.js @@ -178,7 +178,7 @@ describe('Workspace', () => { }) }) - describe('::open(uri, options)', () => { + describe('::open(itemOrURI, options)', () => { let openEvents = null beforeEach(() => { @@ -975,48 +975,107 @@ describe('Workspace', () => { } }) - it('removes matching items from the center', () => { - const pane = atom.workspace.getActivePane() - pane.addItem(item) - atom.workspace.hide(URI) - expect(pane.getItems().length).toBe(0) + describe('when called with a URI', () => { + it('if the item for the given URI is in the center, removes it', () => { + const pane = atom.workspace.getActivePane() + pane.addItem(item) + atom.workspace.hide(URI) + expect(pane.getItems().length).toBe(0) + }) + + it('if the item for the given URI is in a dock, hides the dock', () => { + const dock = atom.workspace.getLeftDock() + const pane = dock.getActivePane() + pane.addItem(item) + dock.activate() + expect(dock.isOpen()).toBe(true) + const itemFound = atom.workspace.hide(URI) + expect(itemFound).toBe(true) + expect(dock.isOpen()).toBe(false) + }) }) - it('hides the dock when an item matches', () => { - const dock = atom.workspace.getLeftDock() - const pane = dock.getActivePane() - pane.addItem(item) - dock.activate() - expect(dock.isOpen()).toBe(true) - const itemFound = atom.workspace.hide(URI) - expect(itemFound).toBe(true) - expect(dock.isOpen()).toBe(false) + describe('when called with an item', () => { + it('if the item is in the center, removes it', () => { + const pane = atom.workspace.getActivePane() + pane.addItem(item) + atom.workspace.hide(item) + expect(pane.getItems().length).toBe(0) + }) + + it('if the item is in a dock, hides the dock', () => { + const dock = atom.workspace.getLeftDock() + const pane = dock.getActivePane() + pane.addItem(item) + dock.activate() + expect(dock.isOpen()).toBe(true) + const itemFound = atom.workspace.hide(item) + expect(itemFound).toBe(true) + expect(dock.isOpen()).toBe(false) + }) }) }) - describe('::toggle(uri)', () => { - it('shows the item and dock if no item matches', () => { - const URI = 'atom://hide-test' - workspace.addOpener(uri => { - if (uri === URI) { - const el = document.createElement('div') - return { - getDefaultLocation: () => 'left', - getTitle: () => 'Item', - getElement: () => el, - getURI: () => URI - } + describe('::toggle(itemOrUri)', () => { + describe('when the location resolves to a dock', () => { + it('adds or shows the item and its dock if it is not currently visible, and otherwise hides the containing dock', async () => { + const item1 = { + getDefaultLocation () { return 'left' }, + getElement () { return (this.element = document.createElement('div')) } } + + const item2 = { + getDefaultLocation () { return 'left' }, + getElement () { return (this.element = document.createElement('div')) } + } + + const dock = workspace.getLeftDock() + expect(dock.isOpen()).toBe(false) + + await workspace.toggle(item1) + expect(dock.isOpen()).toBe(true) + expect(dock.getActivePaneItem()).toBe(item1) + + await workspace.toggle(item2) + expect(dock.isOpen()).toBe(true) + expect(dock.getActivePaneItem()).toBe(item2) + + await workspace.toggle(item1) + expect(dock.isOpen()).toBe(true) + expect(dock.getActivePaneItem()).toBe(item1) + + await workspace.toggle(item1) + expect(dock.isOpen()).toBe(false) + expect(dock.getActivePaneItem()).toBe(item1) + + await workspace.toggle(item2) + expect(dock.isOpen()).toBe(true) + expect(dock.getActivePaneItem()).toBe(item2) }) - const dock = workspace.getLeftDock() - expect(dock.isOpen()).toBe(false) - waitsFor(done => { - workspace.onDidOpen(({item}) => { - expect(item.getURI()).toBe(URI) - expect(dock.isOpen()).toBe(true) - done() - }) - workspace.toggle(URI) + }) + + describe('when the location resolves to the center', () => { + it('adds or shows the item if it is not currently the active pane item, and otherwise removes the item', async () => { + const item1 = { + getDefaultLocation () { return 'center' }, + getElement () { return (this.element = document.createElement('div')) } + } + + const item2 = { + getDefaultLocation () { return 'center' }, + getElement () { return (this.element = document.createElement('div')) } + } + + expect(workspace.getActivePaneItem()).toBeUndefined() + await workspace.toggle(item1) + expect(workspace.getActivePaneItem()).toBe(item1) + await workspace.toggle(item2) + expect(workspace.getActivePaneItem()).toBe(item2) + await workspace.toggle(item1) + expect(workspace.getActivePaneItem()).toBe(item1) + await workspace.toggle(item1) + expect(workspace.paneForItem(item1)).toBeUndefined() + expect(workspace.getActivePaneItem()).toBe(item2) }) }) }) diff --git a/src/workspace.js b/src/workspace.js index 7bc7b3244..ff2b6cb93 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -647,7 +647,7 @@ module.exports = class Workspace extends Model { this.applicationDelegate.addRecentDocument(uri) } - let container, pane + let container, pane, itemExistsInWorkspace // Try to find an existing item in the workspace. if (item || uri) { @@ -674,16 +674,23 @@ module.exports = class Workspace extends Model { } } - if (pane && !item) item = pane.itemForURI(uri) + if (pane) { + if (item) { + itemExistsInWorkspace = pane.getItems().includes(item) + } else { + item = pane.itemForURI(uri) + itemExistsInWorkspace = item != null + } + } } - // If an item is already present, yield the event loop to ensure this method - // is consistently asynchronous regardless of the workspace state. If no - // item is present, create one. - if (item) { - await Promise.resolve() - } else { - item = await this.createItemForURI(uri, options) + // If we already have an item at this stage, we won't need to do an async + // lookup of the URI, so we yield the event loop to ensure this method + // is consistently asynchronous. + if (item) await Promise.resolve() + + if (!itemExistsInWorkspace) { + item = item || await this.createItemForURI(uri, options) if (!item) return if (options.pane) { @@ -760,10 +767,11 @@ module.exports = class Workspace extends Model { // Essential: Search the workspace for items matching the given URI and hide them. // - // * `uri` (optional) A {String} containing a URI. + // * `itemOrURI` (optional) The item to hide or a {String} containing the URI + // of the item to hide. // // Returns a {boolean} indicating whether any items were found (and hidden). - hide (uri) { + hide (itemOrURI) { let foundItems = false // If any visible item has the given URI, hide it @@ -772,16 +780,19 @@ module.exports = class Workspace extends Model { if (isCenter || container.isOpen()) { for (const pane of container.getPanes()) { const activeItem = pane.getActiveItem() - if (activeItem != null && typeof activeItem.getURI === 'function') { - const itemURI = activeItem.getURI() - if (itemURI === uri) { - foundItems = true - // We can't really hide the center so we just destroy the item. - if (isCenter) { - pane.destroyItem(activeItem) - } else { - container.hide() - } + const foundItem = ( + activeItem != null && ( + activeItem === itemOrURI || + typeof activeItem.getURI === 'function' && activeItem.getURI() === itemOrURI + ) + ) + if (foundItem) { + foundItems = true + // We can't really hide the center so we just destroy the item. + if (isCenter) { + pane.destroyItem(activeItem) + } else { + container.hide() } } } @@ -794,9 +805,16 @@ module.exports = class Workspace extends Model { // Essential: Search the workspace for items matching the given URI. If any are found, hide them. // Otherwise, open the URL. // - // * `uri` (optional) A {String} containing a URI. - toggle (uri) { - if (!this.hide(uri)) this.open(uri, {searchAllPanes: true}) + // * `itemOrURI` (optional) The item to toggle or a {String} containing the URI + // of the item to toggle. + // + // Returns a Promise that resolves when the item is shown or hidden. + toggle (itemOrURI) { + if (this.hide(itemOrURI)) { + return Promise.resolve() + } else { + return this.open(itemOrURI, {searchAllPanes: true}) + } } // Open Atom's license in the active pane.