From f6b20cd31c06119bb58708b37d5a9dc3c4e9ed05 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Fri, 14 Apr 2017 15:14:28 -0700 Subject: [PATCH 1/2] Reduce dock initial size lookups Previously, we would get the initial size every time we didn't have an explicit one. With this commit, we only get the initial size when we deserialize and when we go from 0 -> 1 pane items. Also, if the dock doesn't already have an explicit size, we'll use the preferred size of the item being dragged when peaking the dock. That way, dropping it won't cause it to change size. --- src/dock.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/dock.js b/src/dock.js index f6a7aa0e4..91754e9be 100644 --- a/src/dock.js +++ b/src/dock.js @@ -56,6 +56,7 @@ module.exports = class Dock { this.didActivate(this) }), this.paneContainer.observePanes(pane => { + pane.onDidAddItem(this.handleDidAddPaneItem.bind(this)) pane.onDidRemoveItem(this.handleDidRemovePaneItem.bind(this)) }), this.paneContainer.onDidChangeActivePane((item) => params.didChangeActivePane(this, item)), @@ -206,7 +207,11 @@ module.exports = class Dock { } const shouldBeVisible = state.visible || state.showDropTarget - const size = Math.max(MINIMUM_SIZE, state.size == null ? this.getInitialSize() : state.size) + const size = Math.max(MINIMUM_SIZE, + state.size || + (state.draggingItem && getPreferredSize(state.draggingItem, this.location)) || + DEFAULT_INITIAL_SIZE + ) // We need to change the size of the mask... this.maskElement.style[this.widthOrHeight] = `${shouldBeVisible ? size : 0}px` @@ -224,10 +229,16 @@ module.exports = class Dock { }) } + handleDidAddPaneItem () { + if (this.state.size == null) { + this.setState({size: this.getInitialSize()}) + } + } + handleDidRemovePaneItem () { // Hide the dock if you remove the last item. if (this.paneContainer.getPaneItems().length === 0) { - this.setState({visible: false, hovered: false}) + this.setState({visible: false, hovered: false, size: null}) } } @@ -340,13 +351,12 @@ module.exports = class Dock { } getInitialSize () { - let initialSize // The item may not have been activated yet. If that's the case, just use the first item. const activePaneItem = this.paneContainer.getActivePaneItem() || this.paneContainer.getPaneItems()[0] - if (activePaneItem != null) { - initialSize = getPreferredSize(activePaneItem, this.location) - } - return initialSize == null ? DEFAULT_INITIAL_SIZE : initialSize + // If there are items, we should have an explicit width; if not, we shouldn't. + return activePaneItem + ? getPreferredSize(activePaneItem, this.location) || DEFAULT_INITIAL_SIZE + : null } serialize () { @@ -361,7 +371,7 @@ module.exports = class Dock { deserialize (serialized, deserializerManager) { this.paneContainer.deserialize(serialized.paneContainer, deserializerManager) this.setState({ - size: serialized.size, + size: serialized.size || this.getInitialSize(), // If no items could be deserialized, we don't want to show the dock (even if it was visible last time) visible: serialized.visible && (this.paneContainer.getPaneItems().length > 0) }) From d40a14be29d7b44ca8c63aea230509f8ab775957 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Sat, 15 Apr 2017 11:50:41 -0700 Subject: [PATCH 2/2] Add tests for dock sizing behavior --- spec/dock-spec.js | 119 ++++++++++++++++++++++++++++++++++++++++++++++ src/dock.js | 1 + 2 files changed, 120 insertions(+) diff --git a/spec/dock-spec.js b/spec/dock-spec.js index 8fda3900e..e5e26fd3a 100644 --- a/spec/dock-spec.js +++ b/spec/dock-spec.js @@ -120,4 +120,123 @@ describe('Dock', () => { }) }) }) + + describe('when you add an item to an empty dock', () => { + describe('when the item has a preferred size', () => { + it('is takes the preferred size of the item', async () => { + jasmine.attachToDOM(atom.workspace.getElement()) + + const createItem = preferredWidth => ({ + element: document.createElement('div'), + getDefaultLocation() { return 'left' }, + getPreferredWidth() { return preferredWidth } + }) + + const dock = atom.workspace.getLeftDock() + const dockElement = dock.getElement() + expect(dock.getPaneItems()).toHaveLength(0) + + const item1 = createItem(111) + await atom.workspace.open(item1) + + // It should update the width every time we go from 0 -> 1 items, not just the first. + expect(dock.isVisible()).toBe(true) + expect(dockElement.offsetWidth).toBe(111) + dock.destroyActivePane() + expect(dock.getPaneItems()).toHaveLength(0) + expect(dock.isVisible()).toBe(false) + const item2 = createItem(222) + await atom.workspace.open(item2) + expect(dock.isVisible()).toBe(true) + expect(dockElement.offsetWidth).toBe(222) + + // Adding a second shouldn't change the size. + const item3 = createItem(333) + await atom.workspace.open(item3) + expect(dockElement.offsetWidth).toBe(222) + }) + }) + + describe('when the item has no preferred size', () => { + it('is still has an explicit size', async () => { + jasmine.attachToDOM(atom.workspace.getElement()) + + const item = { + element: document.createElement('div'), + getDefaultLocation() { return 'left' } + } + const dock = atom.workspace.getLeftDock() + expect(dock.getPaneItems()).toHaveLength(0) + + expect(dock.state.size).toBe(null) + await atom.workspace.open(item) + expect(dock.state.size).not.toBe(null) + }) + }) + }) + + describe('a deserialized dock', () => { + it('restores the serialized size', async () => { + jasmine.attachToDOM(atom.workspace.getElement()) + + const item = { + element: document.createElement('div'), + getDefaultLocation() { return 'left' }, + getPreferredWidth() { return 122 }, + serialize: () => ({deserializer: 'DockTestItem'}) + } + const itemDeserializer = atom.deserializers.add({ + name: 'DockTestItem', + deserialize: () => item + }) + const dock = atom.workspace.getLeftDock() + const dockElement = dock.getElement() + + await atom.workspace.open(item) + dock.setState({size: 150}) + expect(dockElement.offsetWidth).toBe(150) + const serialized = dock.serialize() + dock.setState({size: 122}) + expect(dockElement.offsetWidth).toBe(122) + dock.destroyActivePane() + dock.deserialize(serialized, atom.deserializers) + expect(dockElement.offsetWidth).toBe(150) + }) + + it("isn't visible if it has no items", async () => { + jasmine.attachToDOM(atom.workspace.getElement()) + + const item = { + element: document.createElement('div'), + getDefaultLocation() { return 'left' }, + getPreferredWidth() { return 122 } + } + const dock = atom.workspace.getLeftDock() + + await atom.workspace.open(item) + expect(dock.isVisible()).toBe(true) + const serialized = dock.serialize() + dock.deserialize(serialized, atom.deserializers) + expect(dock.getPaneItems()).toHaveLength(0) + expect(dock.isVisible()).toBe(false) + }) + }) + + describe('when dragging an item over an empty dock', () => { + it('has the preferred size of the item', () => { + jasmine.attachToDOM(atom.workspace.getElement()) + + const item = { + element: document.createElement('div'), + getDefaultLocation() { return 'left' }, + getPreferredWidth() { return 144 }, + serialize: () => ({deserializer: 'DockTestItem'}) + } + const dock = atom.workspace.getLeftDock() + const dockElement = dock.getElement() + + dock.setDraggingItem(item) + expect(dock.wrapperElement.offsetWidth).toBe(144) + }) + }) }) diff --git a/src/dock.js b/src/dock.js index 91754e9be..4d9232a31 100644 --- a/src/dock.js +++ b/src/dock.js @@ -46,6 +46,7 @@ module.exports = class Dock { }) this.state = { + size: null, visible: false, shouldAnimate: false }