From a317ecfe853bedf6caba53e7b6be72885d67b3b2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 09:49:07 -0400 Subject: [PATCH 01/21] Expand the TabTracker .toggle() tests --- test/controllers/root-controller.test.js | 39 ++++++++++++++++++++---- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 0debb66255..7350984d80 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -17,11 +17,15 @@ import RootController from '../../lib/controllers/root-controller'; describe(`RootController with useLegacyPanels set to ${useLegacyPanels}`, function() { let atomEnv, workspace, commandRegistry, notificationManager, tooltips, config, confirm, app; + function useDocks() { + return !useLegacyPanels && !!workspace.getLeftDock; + } + function isGitPaneDisplayed(wrapper) { - if (useLegacyPanels || !workspace.getLeftDock) { - return wrapper.find('Panel').prop('visible'); - } else { + if (useDocks()) { return wrapper.find('DockItem').exists(); + } else { + return wrapper.find('Panel').prop('visible'); } } @@ -347,12 +351,35 @@ import RootController from '../../lib/controllers/root-controller'; }); describe('toggle()', function() { - it('toggles the visibility of the Git view', function() { + beforeEach(function() { + if (useDocks()) { + sinon.spy(workspace, 'toggle'); + wrapper.instance().gitDockItem = { + getDockItem() { return {}; }, + }; + } + }); + + it('shows the Git view when closed', async function() { assert.isFalse(isGitPaneDisplayed(wrapper)); - gitTabTracker.toggle(); + assert.isNotOk(wrapper.state('gitTabActive')); + + await gitTabTracker.toggle(); + assert.isTrue(isGitPaneDisplayed(wrapper)); - gitTabTracker.toggle(); + assert.isTrue(wrapper.state('gitTabActive')); + if (useDocks()) { assert.isTrue(workspace.toggle.called); } + }); + + it('hides the Git view when open', async function() { + wrapper.setState({gitTabActive: true}); + assert.isTrue(isGitPaneDisplayed(wrapper)); + + await gitTabTracker.toggle(); + assert.isFalse(isGitPaneDisplayed(wrapper)); + assert.isFalse(wrapper.state('gitTabActive')); + if (useDocks()) { assert.isTrue(workspace.toggle.called); } }); }); From eb930df90ea70dc482c3b695b643dcff96738589 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 10:09:04 -0400 Subject: [PATCH 02/21] Keep tab active state and .toggle() calls in sync --- lib/controllers/root-controller.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 67d5c7b14c..f02db016bc 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -842,14 +842,14 @@ class TabTracker { const focusToRestore = document.activeElement; let shouldRestoreFocus = false; - if (!this.getState()) { - await this.setStateKey(true); - shouldRestoreFocus = true; - } else if (this.getDockItem()) { + await this.setStateKey(!this.getState()); + + if (this.getDockItem()) { + // Super happy Docks ultra fun times shouldRestoreFocus = await this.getWorkspace().toggle(this.getDockItem()) !== undefined; } else { // Legacy panels. - await this.setStateKey(false); + shouldRestoreFocus = true; } if (shouldRestoreFocus) { From 5761039284c26a44992fea17a098f817ca05a155 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:27:49 -0400 Subject: [PATCH 03/21] Access DockItem's item through a Promise --- lib/views/dock-item.js | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/views/dock-item.js b/lib/views/dock-item.js index 8de921fe9e..13b60fdab5 100644 --- a/lib/views/dock-item.js +++ b/lib/views/dock-item.js @@ -35,6 +35,14 @@ export default class DockItem extends React.Component { onDidCloseItem: dockItem => {}, } + constructor(props, context) { + super(props, context); + + this.dockItemPromise = new Promise(resolve => { + this.resolveDockItemPromise = resolve; + }); + } + componentDidMount() { this.setupDockItem(); } @@ -79,6 +87,7 @@ export default class DockItem extends React.Component { if (stub) { stub.setRealItem(itemToAdd); this.dockItem = stub; + this.resolveDockItemPromise(this.dockItem); if (this.props.activate) { this.activate(); } @@ -86,6 +95,7 @@ export default class DockItem extends React.Component { Promise.resolve(this.props.workspace.open(itemToAdd, {activatePane: false})) .then(item => { this.dockItem = item; + this.resolveDockItemPromise(this.dockItem); if (this.props.activate) { this.activate(); } }); } @@ -100,10 +110,6 @@ export default class DockItem extends React.Component { ); } - getDockItem() { - return this.dockItem; - } - componentWillUnmount() { this.subscriptions && this.subscriptions.dispose(); if (this.dockItem && !this.didCloseItem) { @@ -115,6 +121,14 @@ export default class DockItem extends React.Component { } } + getDockItem() { + return this.dockItem; + } + + getDockItemPromise() { + return this.dockItemPromise; + } + activate() { setTimeout(() => { if (!this.dockItem || this.didCloseItem || this.props.workspace.isDestroyed()) { From 2835c2ab892be782530866ce6091fb54dbd9b7e7 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:28:45 -0400 Subject: [PATCH 04/21] reveal() and hide() methods on DockItem --- lib/views/dock-item.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/views/dock-item.js b/lib/views/dock-item.js index 13b60fdab5..52511f02ab 100644 --- a/lib/views/dock-item.js +++ b/lib/views/dock-item.js @@ -129,6 +129,18 @@ export default class DockItem extends React.Component { return this.dockItemPromise; } + async reveal() { + const dockItem = await this.getDockItemPromise(); + return this.props.workspace.open(dockItem, { + searchAllPanes: true, + }); + } + + async hide() { + const dockItem = await this.getDockItemPromise(); + return this.props.workspace.hide(dockItem); + } + activate() { setTimeout(() => { if (!this.dockItem || this.didCloseItem || this.props.workspace.isDestroyed()) { From b4a6a223472e651b584602e9cf95c93299e1078d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:31:41 -0400 Subject: [PATCH 05/21] Pass useLegacyPanels to TabTrackers --- lib/controllers/root-controller.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index f02db016bc..b2c6c58767 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -110,6 +110,7 @@ export default class RootController extends React.Component { ); this.gitTabTracker = new TabTracker('git', { + useLegacyPanels: this.props.useLegacyPanels, getState: () => this.state.gitTabActive, setState: (value, callback) => this.setState({gitTabActive: value}, callback), getController: () => this.gitTabController, @@ -118,6 +119,7 @@ export default class RootController extends React.Component { }); this.githubTabTracker = new TabTracker('github', { + useLegacyPanels: this.props.useLegacyPanels, getState: () => this.state.githubTabActive, setState: (value, callback) => this.setState({githubTabActive: value}, callback), getController: () => this.githubTabController, @@ -811,9 +813,10 @@ export default class RootController extends React.Component { } class TabTracker { - constructor(name, {getState, setState, getController, getDockItem, getWorkspace}) { + constructor(name, {useLegacyPanels, getState, setState, getController, getDockItem, getWorkspace}) { this.name = name; + this.useLegacyPanels = useLegacyPanels; this.getState = getState; this.getWorkspace = getWorkspace; this.getController = getController; From 5d79962b17197023a47265bb7087f583fedb9918 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:32:13 -0400 Subject: [PATCH 06/21] Use DockItem accessor --- lib/controllers/root-controller.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index b2c6c58767..a0ca77b8fa 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -820,14 +820,11 @@ class TabTracker { this.getState = getState; this.getWorkspace = getWorkspace; this.getController = getController; + this.getDockItem = getDockItem; this.setStateKey = value => { return new Promise(resolve => setState(value, resolve)); }; - this.getDockItem = () => { - const item = getDockItem(); - return item ? item.getDockItem() : null; - }; } getControllerComponent() { @@ -905,7 +902,7 @@ class TabTracker { const workspace = this.getWorkspace(); return workspace.getPaneContainers() .filter(container => container === workspace.getCenter() || container.isVisible()) - .some(container => container.getPanes().some(pane => pane.getActiveItem() === item)); + .some(container => container.getPanes().some(pane => pane.getActiveItem() === item.getDockItem())); } hasFocus() { From 46c7121f232ccf00e248bf98a2d0e2e8700c00d1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:33:03 -0400 Subject: [PATCH 07/21] reveal() and hide() helpers in TabTracker --- lib/controllers/root-controller.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index a0ca77b8fa..da738770a4 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -884,6 +884,16 @@ class TabTracker { return false; } + reveal() { + const dockItem = this.getDockItem(); + return dockItem ? dockItem.reveal() : Promise.resolve(null); + } + + hide() { + const dockItem = this.getDockItem(); + return dockItem ? dockItem.hide() : Promise.resolve(null); + } + focus() { this.getControllerComponent().restoreFocus(); } From 2a5c7d93e4eb945064c015b09f9ba102ab7cc79f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:33:43 -0400 Subject: [PATCH 08/21] Use helpers in toggle() logic --- lib/controllers/root-controller.js | 33 +++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index da738770a4..e703837371 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -842,14 +842,29 @@ class TabTracker { const focusToRestore = document.activeElement; let shouldRestoreFocus = false; - await this.setStateKey(!this.getState()); - - if (this.getDockItem()) { - // Super happy Docks ultra fun times - shouldRestoreFocus = await this.getWorkspace().toggle(this.getDockItem()) !== undefined; - } else { - // Legacy panels. + // Rendered => the dock item is being rendered, whether or not the dock is visible or the item + // is visible within its dock. + // Visible => the item is active and the dock item is active within its dock. + const wasRendered = this.getState(); + const wasVisible = this.isVisible(); + + if (!wasRendered) { + // Not rendered. + await this.setStateKey(true); + await this.reveal(); + shouldRestoreFocus = true; + } else if (!wasVisible) { + // Rendered, but not an active item in a visible dock. + await this.reveal(); shouldRestoreFocus = true; + } else { + // Rendered and an active item within a visible dock. + if (this.useLegacyPanels) { + await this.setStateKey(false); + } else { + await this.hide(); + } + shouldRestoreFocus = false; } if (shouldRestoreFocus) { @@ -876,9 +891,7 @@ class TabTracker { async ensureVisible() { if (!this.isVisible()) { await this.setStateKey(true); - if (this.getDockItem()) { - await this.getWorkspace().open(this.getDockItem()); - } + await this.reveal(); return true; } return false; From 358adc58f248ad32c5ed45c44979f4e2b3ebb385 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:33:59 -0400 Subject: [PATCH 09/21] :fire: unused defaultState keys --- lib/github-package.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 63c3e6ea52..0a7bc70d5c 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -23,8 +23,6 @@ import AsyncQueue from './async-queue'; import WorkerManager from './worker-manager'; const defaultState = { - firstRun: true, - resolutionProgressByPath: {}, }; export default class GithubPackage { From 5dabad76752f3e866adbbdbe9482fe6ad108a82c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 31 May 2017 22:34:48 -0400 Subject: [PATCH 10/21] Explicitly test all 3 (x2 = 6) .toggle() cases --- test/controllers/root-controller.test.js | 51 +++++++++++++++++++----- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 7350984d80..8e99702c57 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -351,35 +351,66 @@ import RootController from '../../lib/controllers/root-controller'; }); describe('toggle()', function() { + const FAKEITEM = Symbol('fake dock item'); + let treatAsActive; + beforeEach(function() { if (useDocks()) { - sinon.spy(workspace, 'toggle'); + treatAsActive = false; wrapper.instance().gitDockItem = { - getDockItem() { return {}; }, + getDockItem() { return FAKEITEM; }, + getDockItemPromise() { return Promise.resolve(FAKEITEM); }, }; + + sinon.stub(workspace, 'toggle').callsFake(() => { + treatAsActive = !treatAsActive; + }); + + sinon.stub(workspace.getRightDock(), 'isVisible').returns(true); + sinon.stub(workspace.getRightDock(), 'getPanes').callsFake(() => { + return [{ + getActiveItem: () => (treatAsActive ? FAKEITEM : null), + }]; + }); } }); - it('shows the Git view when closed', async function() { - assert.isFalse(isGitPaneDisplayed(wrapper)); + it('renders and reveals the Git view when item is not rendered', async function() { assert.isNotOk(wrapper.state('gitTabActive')); + treatAsActive = false; await gitTabTracker.toggle(); - assert.isTrue(isGitPaneDisplayed(wrapper)); assert.isTrue(wrapper.state('gitTabActive')); if (useDocks()) { assert.isTrue(workspace.toggle.called); } }); - it('hides the Git view when open', async function() { + it('reveals the Git view when the item is rendered but not visible', async function() { wrapper.setState({gitTabActive: true}); - assert.isTrue(isGitPaneDisplayed(wrapper)); + treatAsActive = false; await gitTabTracker.toggle(); - assert.isFalse(isGitPaneDisplayed(wrapper)); - assert.isFalse(wrapper.state('gitTabActive')); - if (useDocks()) { assert.isTrue(workspace.toggle.called); } + if (useDocks()) { + assert.isTrue(wrapper.state('gitTabActive')); + assert.isTrue(workspace.toggle.called); + } else { + assert.isFalse(wrapper.state('gitTabActive')); + } + }); + + it('hides and unrenders the Git view when open', async function() { + wrapper.setState({gitTabActive: true}); + treatAsActive = true; + + await gitTabTracker.toggle(); + + if (useDocks()) { + assert.isTrue(wrapper.state('gitTabActive')); + assert.isTrue(workspace.toggle.called); + } else { + assert.isFalse(wrapper.state('gitTabActive')); + } }); }); From 77ac6aeef3e876ca95e5ebc6cb8f6cf470f82769 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 10:14:05 -0400 Subject: [PATCH 11/21] Adjust tests to mock DockItems more accurately --- test/controllers/root-controller.test.js | 68 ++++++++++++------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 8e99702c57..f82a70d109 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -319,22 +319,6 @@ import RootController from '../../lib/controllers/root-controller'; }); }); - describe('when the StatusBarTileController calls toggleGitTab', function() { - it('toggles the git panel', async function() { - const workdirPath = await cloneRepository('multiple-commits'); - const repository = await buildRepository(workdirPath); - - app = React.cloneElement(app, {repository}); - const wrapper = shallow(app); - - assert.isFalse(isGitPaneDisplayed(wrapper)); - wrapper.find('ObserveModelDecorator(StatusBarTileController)').prop('toggleGitTab')(); - assert.isTrue(isGitPaneDisplayed(wrapper)); - wrapper.find('ObserveModelDecorator(StatusBarTileController)').prop('toggleGitTab')(); - assert.isFalse(isGitPaneDisplayed(wrapper)); - }); - }); - describe('git tab tracker', function() { let wrapper, gitTabTracker; @@ -351,25 +335,41 @@ import RootController from '../../lib/controllers/root-controller'; }); describe('toggle()', function() { - const FAKEITEM = Symbol('fake dock item'); - let treatAsActive; + let mockDockItem; beforeEach(function() { - if (useDocks()) { - treatAsActive = false; - wrapper.instance().gitDockItem = { - getDockItem() { return FAKEITEM; }, - getDockItemPromise() { return Promise.resolve(FAKEITEM); }, - }; + const FAKE_PANE_ITEM = Symbol('fake pane item'); + mockDockItem = { + visible: false, + + reveal() { + this.visible = true; + return Promise.resolve(); + }, + + hide() { + this.visible = false; + return Promise.resolve(); + }, + + getDockItem() { + return FAKE_PANE_ITEM; + }, + }; - sinon.stub(workspace, 'toggle').callsFake(() => { - treatAsActive = !treatAsActive; - }); + if (useDocks()) { + wrapper.instance().gitDockItem = mockDockItem; sinon.stub(workspace.getRightDock(), 'isVisible').returns(true); sinon.stub(workspace.getRightDock(), 'getPanes').callsFake(() => { return [{ - getActiveItem: () => (treatAsActive ? FAKEITEM : null), + getActiveItem() { + if (mockDockItem.visible) { + return FAKE_PANE_ITEM; + } else { + return null; + } + }, }]; }); } @@ -377,23 +377,23 @@ import RootController from '../../lib/controllers/root-controller'; it('renders and reveals the Git view when item is not rendered', async function() { assert.isNotOk(wrapper.state('gitTabActive')); - treatAsActive = false; + mockDockItem.visible = false; await gitTabTracker.toggle(); assert.isTrue(wrapper.state('gitTabActive')); - if (useDocks()) { assert.isTrue(workspace.toggle.called); } + if (useDocks()) { assert.isTrue(mockDockItem.visible); } }); it('reveals the Git view when the item is rendered but not visible', async function() { wrapper.setState({gitTabActive: true}); - treatAsActive = false; + mockDockItem.visible = false; await gitTabTracker.toggle(); if (useDocks()) { assert.isTrue(wrapper.state('gitTabActive')); - assert.isTrue(workspace.toggle.called); + assert.isTrue(mockDockItem.visible); } else { assert.isFalse(wrapper.state('gitTabActive')); } @@ -401,13 +401,13 @@ import RootController from '../../lib/controllers/root-controller'; it('hides and unrenders the Git view when open', async function() { wrapper.setState({gitTabActive: true}); - treatAsActive = true; + mockDockItem.visible = true; await gitTabTracker.toggle(); if (useDocks()) { assert.isTrue(wrapper.state('gitTabActive')); - assert.isTrue(workspace.toggle.called); + assert.isFalse(mockDockItem.visible); } else { assert.isFalse(wrapper.state('gitTabActive')); } From ad551571f04c6e4242909e1aa446927f1d7df7f3 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 13:30:05 -0400 Subject: [PATCH 12/21] Handle rendered for legacy panels in tab tracker test helpers --- test/controllers/root-controller.test.js | 201 ++++++++++++----------- 1 file changed, 103 insertions(+), 98 deletions(-) diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index f82a70d109..7ac30ff96e 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -319,155 +319,160 @@ import RootController from '../../lib/controllers/root-controller'; }); }); - describe('git tab tracker', function() { - let wrapper, gitTabTracker; + ['git', 'github'].forEach(function(tabName) { + describe(`${tabName} tab tracker`, function() { + let wrapper, tabTracker, stateKey, mockDockItem; - beforeEach(async function() { - const workdirPath = await cloneRepository('multiple-commits'); - const repository = await buildRepository(workdirPath); + beforeEach(async function() { + const workdirPath = await cloneRepository('multiple-commits'); + const repository = await buildRepository(workdirPath); - app = React.cloneElement(app, {repository}); - wrapper = shallow(app); - gitTabTracker = wrapper.instance().gitTabTracker; + const savedState = { + gitTabActive: false, + githubTabActive: false, + }; + app = React.cloneElement(app, {repository, savedState}); + wrapper = shallow(app); + tabTracker = wrapper.instance()[`${tabName}TabTracker`]; - sinon.stub(gitTabTracker, 'focus'); - sinon.spy(workspace.getActivePane(), 'activate'); - }); + sinon.stub(tabTracker, 'focus'); + sinon.spy(workspace.getActivePane(), 'activate'); - describe('toggle()', function() { - let mockDockItem; + stateKey = `${tabName}TabActive`; - beforeEach(function() { const FAKE_PANE_ITEM = Symbol('fake pane item'); mockDockItem = { visible: false, - reveal() { this.visible = true; return Promise.resolve(); }, - hide() { this.visible = false; return Promise.resolve(); }, - getDockItem() { return FAKE_PANE_ITEM; }, }; if (useDocks()) { - wrapper.instance().gitDockItem = mockDockItem; + wrapper.instance()[`${tabName}DockItem`] = mockDockItem; sinon.stub(workspace.getRightDock(), 'isVisible').returns(true); - sinon.stub(workspace.getRightDock(), 'getPanes').callsFake(() => { - return [{ - getActiveItem() { - if (mockDockItem.visible) { - return FAKE_PANE_ITEM; - } else { - return null; - } - }, - }]; - }); + sinon.stub(workspace.getRightDock(), 'getPanes').callsFake(() => [{ + getActiveItem() { return mockDockItem.visible ? FAKE_PANE_ITEM : null; }, + }]); } }); - it('renders and reveals the Git view when item is not rendered', async function() { - assert.isNotOk(wrapper.state('gitTabActive')); - mockDockItem.visible = false; - - await gitTabTracker.toggle(); - - assert.isTrue(wrapper.state('gitTabActive')); - if (useDocks()) { assert.isTrue(mockDockItem.visible); } - }); + function assertTabState({rendered, visible}) { + if (useDocks()) { + const isRendered = wrapper.find('DockItem').find({stubItemSelector: `${tabName}-tab-controller`}).exists(); + const isVisible = mockDockItem.visible; - it('reveals the Git view when the item is rendered but not visible', async function() { - wrapper.setState({gitTabActive: true}); - mockDockItem.visible = false; + assert.deepEqual({rendered, visible}, {rendered: isRendered, visible: isVisible}); + } else { + // Legacy panels API. + // Legacy panels are always rendered, so disregard "rendered" assertions. + const title = {git: 'Git', github: 'GitHub (preview)'}[tabName]; + const activeIndex = {git: 0, github: 1}[tabName]; - await gitTabTracker.toggle(); + const isVisible = wrapper.find('Panel').find({visible: true}) + .find('Tabs').find({activeIndex}) + .find('TabPanel').find({title}) + .exists(); - if (useDocks()) { - assert.isTrue(wrapper.state('gitTabActive')); - assert.isTrue(mockDockItem.visible); - } else { - assert.isFalse(wrapper.state('gitTabActive')); + assert.deepEqual({visible}, {visible: isVisible}); } - }); + } - it('hides and unrenders the Git view when open', async function() { - wrapper.setState({gitTabActive: true}); - mockDockItem.visible = true; + describe('toggle()', function() { + it(`renders and reveals the ${tabName} tab when item is not rendered`, async function() { + console.log(`-- ${tabName} + ${useLegacyPanels} --`); + console.log(`precondition check:\n${wrapper.debug()}`); + assertTabState({rendered: false, visible: false}); - await gitTabTracker.toggle(); + console.log('about to toggle'); + await tabTracker.toggle(); + console.log('toggle complete'); - if (useDocks()) { - assert.isTrue(wrapper.state('gitTabActive')); - assert.isFalse(mockDockItem.visible); - } else { - assert.isFalse(wrapper.state('gitTabActive')); - } - }); - }); + console.log(`postcondition check:\n${wrapper.debug()}`); + assertTabState({rendered: true, visible: true}); + }); - describe('toggleFocus()', function() { - it('opens and focuses the Git panel when it is initially closed', async function() { - assert.isFalse(isGitPaneDisplayed(wrapper)); - sinon.stub(gitTabTracker, 'hasFocus').returns(false); + it(`reveals the ${tabName} tab when the item is rendered but not visible`, async function() { + wrapper.setState({[stateKey]: true}); - await gitTabTracker.toggleFocus(); + assertTabState({rendered: true, visible: false}); - assert.isTrue(isGitPaneDisplayed(wrapper)); - assert.equal(gitTabTracker.focus.callCount, 1); - assert.isFalse(workspace.getActivePane().activate.called); - }); + await tabTracker.toggle(); - it('focuses the Git panel when it is already open, but blurred', async function() { - gitTabTracker.toggle(); - sinon.stub(gitTabTracker, 'hasFocus').returns(false); + assertTabState({rendered: true, visible: true}); + }); - assert.isTrue(isGitPaneDisplayed(wrapper)); + it('hides and unrenders the Git view when open', async function() { + wrapper.setState({[stateKey]: true}); + mockDockItem.visible = true; - await gitTabTracker.toggleFocus(); + assertTabState({rendered: true, visible: true}); - assert.isTrue(isGitPaneDisplayed(wrapper)); - assert.equal(gitTabTracker.focus.callCount, 1); - assert.isFalse(workspace.getActivePane().activate.called); + await tabTracker.toggle(); + + assertTabState({rendered: true, visible: false}); + }); }); - it('blurs the Git panel when it is already open and focused', async function() { - gitTabTracker.toggle(); - sinon.stub(gitTabTracker, 'hasFocus').returns(true); + describe('toggleFocus()', function() { + it(`opens and focuses the ${tabName} tab when it is initially closed`, async function() { + assertTabState({rendered: false, visible: false}); + sinon.stub(tabTracker, 'hasFocus').returns(false); - assert.isTrue(isGitPaneDisplayed(wrapper)); + await tabTracker.toggleFocus(); - await gitTabTracker.toggleFocus(); + assertTabState({rendered: true, visible: true}); + assert.isTrue(tabTracker.focus.called); + assert.isFalse(workspace.getActivePane().activate.called); + }); - assert.isTrue(isGitPaneDisplayed(wrapper)); - assert.equal(gitTabTracker.focus.callCount, 0); - assert.isTrue(workspace.getActivePane().activate.called); - }); - }); + it(`focuses the ${tabName} tab when it is already open, but blurred`, async function() { + await tabTracker.ensureVisible(); + assertTabState({rendered: true, visible: true}); + sinon.stub(tabTracker, 'hasFocus').returns(false); + + await tabTracker.toggleFocus(); + + assertTabState({rendered: true, visible: true}); + assert.isTrue(tabTracker.focus.called); + assert.isFalse(workspace.getActivePane().activate.called); + }); + + it(`blurs the ${tabName} tab when it is already open and focused`, async function() { + await tabTracker.ensureVisible(); + assertTabState({rendered: true, visible: true}); + sinon.stub(tabTracker, 'hasFocus').returns(true); - describe('ensureGitTab()', function() { - it('opens the Git panel when it is initially closed', async function() { - sinon.stub(gitTabTracker, 'isVisible').returns(false); - assert.isFalse(isGitPaneDisplayed(wrapper)); + await tabTracker.toggleFocus(); - assert.isTrue(await gitTabTracker.ensureVisible()); + assertTabState({rendered: true, visible: true}); + assert.isFalse(tabTracker.focus.called); + assert.isTrue(workspace.getActivePane().activate.called); + }); }); - it('does nothing when the Git panel is already open', async function() { - gitTabTracker.toggle(); - sinon.stub(gitTabTracker, 'isVisible').returns(true); - assert.isTrue(isGitPaneDisplayed(wrapper)); + describe('ensureVisible()', function() { + it(`opens the ${tabName} tab when it is initially closed`, async function() { + assertTabState({rendered: false, visible: false}); + assert.isTrue(await tabTracker.ensureVisible()); + assertTabState({rendered: true, visible: true}); + }); - assert.isFalse(await gitTabTracker.ensureVisible()); - assert.isTrue(isGitPaneDisplayed(wrapper)); + it(`does nothing when the ${tabName} tab is already open`, async function() { + tabTracker.toggle(); + assertTabState({rendered: true, visible: true}); + assert.isFalse(await tabTracker.ensureVisible()); + assertTabState({rendered: true, visible: true}); + }); }); }); }); From a8c5f70304415885d024790521337962b0a7e19e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 14:21:55 -0400 Subject: [PATCH 13/21] More TabTracker helper work --- test/controllers/root-controller.test.js | 99 ++++++++++++++++-------- 1 file changed, 67 insertions(+), 32 deletions(-) diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 7ac30ff96e..12d935d630 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -21,14 +21,6 @@ import RootController from '../../lib/controllers/root-controller'; return !useLegacyPanels && !!workspace.getLeftDock; } - function isGitPaneDisplayed(wrapper) { - if (useDocks()) { - return wrapper.find('DockItem').exists(); - } else { - return wrapper.find('Panel').prop('visible'); - } - } - beforeEach(function() { atomEnv = global.buildAtomEnvironment(); workspace = atomEnv.workspace; @@ -62,14 +54,43 @@ import RootController from '../../lib/controllers/root-controller'; }); describe('initial panel visibility', function() { - it('is not visible when the saved state indicates they were not visible last run', async function() { + function assertInitialTabState({tabName, wrapper, rendered, visible, activated}) { + if (useDocks()) { + // Docks API. Disregard "visible" check because the persistence of dock visibility is handled by the + // docks API. + const dockItem = wrapper + .find('DockItem').find({stubItemSelector: `${tabName}-tab-controller`}); + const isRendered = dockItem.exists(); + const isActivated = isRendered ? dockItem.prop('activate') === true : false; + + assert.equal(rendered, isRendered); + assert.equal(activated, isActivated); + } else { + // Legacy panels. Only test for visibility. + const isVisible = wrapper + .find('Panel').find({visible: true}) + .exists(); + + assert.equal(visible, isVisible); + } + } + + it('is not rendered when the saved state indicates they were not visible last run', async function() { const workdirPath = await cloneRepository('multiple-commits'); const repository = await buildRepository(workdirPath); app = React.cloneElement(app, {repository, savedState: {gitTabActive: false, githubTabActive: false}}); const wrapper = shallow(app); - assert.isFalse(isGitPaneDisplayed(wrapper)); + assertInitialTabState({ + wrapper, tabName: 'git', + rendered: false, visible: false, activated: false, + }); + + assertInitialTabState({ + wrapper, tabName: 'github', + rendered: false, visible: false, activated: false, + }); }); it('is visible when the saved state indicates they were visible last run', async function() { @@ -79,17 +100,33 @@ import RootController from '../../lib/controllers/root-controller'; app = React.cloneElement(app, {repository, savedState: {gitTabActive: true, githubTabActive: true}}); const wrapper = shallow(app); - assert.isTrue(isGitPaneDisplayed(wrapper)); + assertInitialTabState({ + wrapper, tabName: 'git', + rendered: true, activated: false, visible: true, + }); + + assertInitialTabState({ + wrapper, tabName: 'github', + rendered: true, activated: false, visible: true, + }); }); - it('is always visible when the startOpen prop is true', async function() { + it('is initially activated when the startOpen prop is true', async function() { const workdirPath = await cloneRepository('multiple-commits'); const repository = await buildRepository(workdirPath); app = React.cloneElement(app, {repository, startOpen: true}); const wrapper = shallow(app); - assert.isTrue(isGitPaneDisplayed(wrapper)); + assertInitialTabState({ + wrapper, tabName: 'git', + rendered: true, activated: true, visible: true, + }); + + assertInitialTabState({ + wrapper, tabName: 'github', + rendered: true, activated: false, visible: true, + }); }); }); @@ -321,7 +358,7 @@ import RootController from '../../lib/controllers/root-controller'; ['git', 'github'].forEach(function(tabName) { describe(`${tabName} tab tracker`, function() { - let wrapper, tabTracker, stateKey, mockDockItem; + let wrapper, tabTracker, stateKey, tabIndex, mockDockItem; beforeEach(async function() { const workdirPath = await cloneRepository('multiple-commits'); @@ -339,6 +376,7 @@ import RootController from '../../lib/controllers/root-controller'; sinon.spy(workspace.getActivePane(), 'activate'); stateKey = `${tabName}TabActive`; + tabIndex = {git: 0, github: 1}[tabName]; const FAKE_PANE_ITEM = Symbol('fake pane item'); mockDockItem = { @@ -371,48 +409,45 @@ import RootController from '../../lib/controllers/root-controller'; const isRendered = wrapper.find('DockItem').find({stubItemSelector: `${tabName}-tab-controller`}).exists(); const isVisible = mockDockItem.visible; - assert.deepEqual({rendered, visible}, {rendered: isRendered, visible: isVisible}); + assert.equal(isRendered, rendered); + assert.equal(isVisible, visible); } else { // Legacy panels API. // Legacy panels are always rendered, so disregard "rendered" assertions. const title = {git: 'Git', github: 'GitHub (preview)'}[tabName]; - const activeIndex = {git: 0, github: 1}[tabName]; const isVisible = wrapper.find('Panel').find({visible: true}) - .find('Tabs').find({activeIndex}) + .find('Tabs').find({activeIndex: tabIndex}) .find('TabPanel').find({title}) .exists(); - assert.deepEqual({visible}, {visible: isVisible}); + assert.equal(isVisible, visible); } } describe('toggle()', function() { it(`renders and reveals the ${tabName} tab when item is not rendered`, async function() { - console.log(`-- ${tabName} + ${useLegacyPanels} --`); - console.log(`precondition check:\n${wrapper.debug()}`); assertTabState({rendered: false, visible: false}); - console.log('about to toggle'); await tabTracker.toggle(); - console.log('toggle complete'); - console.log(`postcondition check:\n${wrapper.debug()}`); assertTabState({rendered: true, visible: true}); }); - it(`reveals the ${tabName} tab when the item is rendered but not visible`, async function() { - wrapper.setState({[stateKey]: true}); + if (!useLegacyPanels) { + it(`reveals the ${tabName} tab when the item is rendered but not visible`, async function() { + wrapper.setState({[stateKey]: true}); - assertTabState({rendered: true, visible: false}); + assertTabState({rendered: true, visible: false}); - await tabTracker.toggle(); + await tabTracker.toggle(); - assertTabState({rendered: true, visible: true}); - }); + assertTabState({rendered: true, visible: true}); + }); + } - it('hides and unrenders the Git view when open', async function() { - wrapper.setState({[stateKey]: true}); + it(`hides the ${tabName} tab when open`, async function() { + wrapper.setState({[stateKey]: true, activeTab: tabIndex}); mockDockItem.visible = true; assertTabState({rendered: true, visible: true}); @@ -468,7 +503,7 @@ import RootController from '../../lib/controllers/root-controller'; }); it(`does nothing when the ${tabName} tab is already open`, async function() { - tabTracker.toggle(); + await tabTracker.toggle(); assertTabState({rendered: true, visible: true}); assert.isFalse(await tabTracker.ensureVisible()); assertTabState({rendered: true, visible: true}); From ebcb06afca38684b394fea28d933aaf906017a8e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 14:38:28 -0400 Subject: [PATCH 14/21] Rename "visible" to "active" to be a bit more specific --- test/controllers/root-controller.test.js | 54 ++++++++++++------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 12d935d630..a290802116 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -380,13 +380,13 @@ import RootController from '../../lib/controllers/root-controller'; const FAKE_PANE_ITEM = Symbol('fake pane item'); mockDockItem = { - visible: false, + active: false, reveal() { - this.visible = true; + this.active = true; return Promise.resolve(); }, hide() { - this.visible = false; + this.active = false; return Promise.resolve(); }, getDockItem() { @@ -399,97 +399,97 @@ import RootController from '../../lib/controllers/root-controller'; sinon.stub(workspace.getRightDock(), 'isVisible').returns(true); sinon.stub(workspace.getRightDock(), 'getPanes').callsFake(() => [{ - getActiveItem() { return mockDockItem.visible ? FAKE_PANE_ITEM : null; }, + getActiveItem() { return mockDockItem.active ? FAKE_PANE_ITEM : null; }, }]); } }); - function assertTabState({rendered, visible}) { + function assertTabState({rendered, active}) { if (useDocks()) { const isRendered = wrapper.find('DockItem').find({stubItemSelector: `${tabName}-tab-controller`}).exists(); - const isVisible = mockDockItem.visible; + const isActive = mockDockItem.active; assert.equal(isRendered, rendered); - assert.equal(isVisible, visible); + assert.equal(isActive, active); } else { // Legacy panels API. // Legacy panels are always rendered, so disregard "rendered" assertions. const title = {git: 'Git', github: 'GitHub (preview)'}[tabName]; - const isVisible = wrapper.find('Panel').find({visible: true}) + const isActive = wrapper.find('Panel').find({visible: true}) .find('Tabs').find({activeIndex: tabIndex}) .find('TabPanel').find({title}) .exists(); - assert.equal(isVisible, visible); + assert.equal(isActive, active); } } describe('toggle()', function() { it(`renders and reveals the ${tabName} tab when item is not rendered`, async function() { - assertTabState({rendered: false, visible: false}); + assertTabState({rendered: false, active: false}); await tabTracker.toggle(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); }); if (!useLegacyPanels) { - it(`reveals the ${tabName} tab when the item is rendered but not visible`, async function() { + it(`reveals the ${tabName} tab when the item is rendered but not active`, async function() { wrapper.setState({[stateKey]: true}); - assertTabState({rendered: true, visible: false}); + assertTabState({rendered: true, active: false}); await tabTracker.toggle(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); }); } it(`hides the ${tabName} tab when open`, async function() { wrapper.setState({[stateKey]: true, activeTab: tabIndex}); - mockDockItem.visible = true; + mockDockItem.active = true; - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); await tabTracker.toggle(); - assertTabState({rendered: true, visible: false}); + assertTabState({rendered: true, active: false}); }); }); describe('toggleFocus()', function() { it(`opens and focuses the ${tabName} tab when it is initially closed`, async function() { - assertTabState({rendered: false, visible: false}); + assertTabState({rendered: false, active: false}); sinon.stub(tabTracker, 'hasFocus').returns(false); await tabTracker.toggleFocus(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); assert.isTrue(tabTracker.focus.called); assert.isFalse(workspace.getActivePane().activate.called); }); it(`focuses the ${tabName} tab when it is already open, but blurred`, async function() { await tabTracker.ensureVisible(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); sinon.stub(tabTracker, 'hasFocus').returns(false); await tabTracker.toggleFocus(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); assert.isTrue(tabTracker.focus.called); assert.isFalse(workspace.getActivePane().activate.called); }); it(`blurs the ${tabName} tab when it is already open and focused`, async function() { await tabTracker.ensureVisible(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); sinon.stub(tabTracker, 'hasFocus').returns(true); await tabTracker.toggleFocus(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); assert.isFalse(tabTracker.focus.called); assert.isTrue(workspace.getActivePane().activate.called); }); @@ -497,16 +497,16 @@ import RootController from '../../lib/controllers/root-controller'; describe('ensureVisible()', function() { it(`opens the ${tabName} tab when it is initially closed`, async function() { - assertTabState({rendered: false, visible: false}); + assertTabState({rendered: false, active: false}); assert.isTrue(await tabTracker.ensureVisible()); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); }); it(`does nothing when the ${tabName} tab is already open`, async function() { await tabTracker.toggle(); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); assert.isFalse(await tabTracker.ensureVisible()); - assertTabState({rendered: true, visible: true}); + assertTabState({rendered: true, active: true}); }); }); }); From b80e4e77bd0f73cbeccdc5cefea46de6fe59f231 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 14:40:57 -0400 Subject: [PATCH 15/21] Set user.name and user.email for AppVeyor apparently --- test/helpers.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/helpers.js b/test/helpers.js index 3a4fa8b398..8b1bd01565 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -34,6 +34,8 @@ export async function cloneRepository(repoName = 'three-files') { await git.clone(path.join(__dirname, 'fixtures', `repo-${repoName}`, 'dot-git'), {noLocal: true}); await git.exec(['config', '--local', 'core.autocrlf', 'false']); await git.exec(['config', '--local', 'commit.gpgsign', 'false']); + await git.exec(['config', '--local', 'user.email', 'nope@nah.com']); + await git.exec(['config', '--local', 'user.name', 'Someone']); await git.exec(['checkout', '--', '.']); // discard \r in working directory cachedClonedRepos[repoName] = cachedPath; } From e7afed6c499d03c2c048059fd39140da739f6536 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 14:43:35 -0400 Subject: [PATCH 16/21] Activate tabs during setState() when requested --- lib/controllers/root-controller.js | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index e703837371..072860f02a 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -112,7 +112,11 @@ export default class RootController extends React.Component { this.gitTabTracker = new TabTracker('git', { useLegacyPanels: this.props.useLegacyPanels, getState: () => this.state.gitTabActive, - setState: (value, callback) => this.setState({gitTabActive: value}, callback), + setState: (value, {activateTab}, callback) => { + const newState = {gitTabActive: value}; + if (activateTab) { newState.activeTab = 0; } + this.setState(newState, callback); + }, getController: () => this.gitTabController, getDockItem: () => this.gitDockItem, getWorkspace: () => this.props.workspace, @@ -121,7 +125,11 @@ export default class RootController extends React.Component { this.githubTabTracker = new TabTracker('github', { useLegacyPanels: this.props.useLegacyPanels, getState: () => this.state.githubTabActive, - setState: (value, callback) => this.setState({githubTabActive: value}, callback), + setState: (value, {activateTab}, callback) => { + const newState = {githubTabActive: value}; + if (activateTab) { newState.activeTab = 1; } + this.setState(newState, callback); + }, getController: () => this.githubTabController, getDockItem: () => this.githubDockItem, getWorkspace: () => this.props.workspace, @@ -245,8 +253,8 @@ export default class RootController extends React.Component { this.setState({gitTabActive: false})} - visible={!!this.state.gitTabActive}> + onDidClosePanel={() => this.setState({gitTabActive: false, githubTabActive: false})} + visible={!!this.state.gitTabActive || !!this.state.githubTabActive}> { - return new Promise(resolve => setState(value, resolve)); + this.setStateKey = (value, options = {activateTab: false}) => { + return new Promise(resolve => setState(value, options, resolve)); }; } @@ -850,7 +858,7 @@ class TabTracker { if (!wasRendered) { // Not rendered. - await this.setStateKey(true); + await this.setStateKey(true, {activateTab: true}); await this.reveal(); shouldRestoreFocus = true; } else if (!wasVisible) { @@ -890,7 +898,7 @@ class TabTracker { @autobind async ensureVisible() { if (!this.isVisible()) { - await this.setStateKey(true); + await this.setStateKey(true, {activateTab: true}); await this.reveal(); return true; } From 626076a8f77680add961f5c2f9244e13b38fc609 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 14:58:51 -0400 Subject: [PATCH 17/21] Keep useLegacyPanels up to date in tab trackers --- lib/controllers/root-controller.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 072860f02a..b89f54cf26 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -143,6 +143,9 @@ export default class RootController extends React.Component { componentWillReceiveProps(newProps) { this.repositoryObserver.setActiveModel(newProps.repository); this.repositoryStateRegistry.setModel(newProps.repository); + + this.gitTabTracker.useLegacyPanels = newProps.useLegacyPanels; + this.githubTabTracker.useLegacyPanels = newProps.useLegacyPanels; } render() { From 8bc520925d57d2b0006b530b7493f51137903b5d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 15:03:47 -0400 Subject: [PATCH 18/21] Set git config in a few other cases --- test/helpers.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/helpers.js b/test/helpers.js index 8b1bd01565..c95294de53 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -55,6 +55,8 @@ export async function initRepository(repoName) { const workingDirPath = temp.mkdirSync('git-fixture-'); const git = new GitShellOutStrategy(workingDirPath); await git.exec(['init']); + await git.exec(['config', '--local', 'user.email', 'nope@nah.com']); + await git.exec(['config', '--local', 'user.name', 'Someone']); return fs.realpathSync(workingDirPath); } @@ -80,6 +82,8 @@ export async function setUpLocalAndRemoteRepositories(repoName = 'multiple-commi await localGit.clone(baseRepoPath, {noLocal: true}); await localGit.exec(['remote', 'set-url', 'origin', remoteRepoPath]); await localGit.exec(['config', '--local', 'commit.gpgsign', 'false']); + await localGit.exec(['config', '--local', 'user.email', 'nope@nah.com']); + await localGit.exec(['config', '--local', 'user.name', 'Someone']); return {baseRepoPath, remoteRepoPath, localRepoPath}; } From 8bb0ca7234a40f71b4badf313c4383657a1ef67e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 15:14:25 -0400 Subject: [PATCH 19/21] For new projects, render but do not activate git and GitHub tabs --- lib/controllers/root-controller.js | 12 ++++++++++-- test/controllers/root-controller.test.js | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index b89f54cf26..a0e3e28b1d 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -77,11 +77,19 @@ export default class RootController extends React.Component { constructor(props, context) { super(props, context); + + const renderGitTab = props.startOpen || + (props.savedState.gitTabActive === undefined ? + !this.props.useLegacyPanels : this.props.savedState.gitTabActive); + const renderGitHubTab = props.startOpen || + (props.savedState.githubTabActive === undefined ? + !this.props.useLegacyPanels : this.props.savedState.githubTabActive); + this.state = { ...nullFilePatchState, amending: false, - gitTabActive: props.startOpen || props.savedState.gitTabActive, - githubTabActive: props.startOpen || props.savedState.githubTabActive || props.savedState.githubPanelActive, + gitTabActive: renderGitTab, + githubTabActive: renderGitHubTab, panelSize: props.savedState.panelSize || 400, activeTab: props.savedState.activeTab || 0, cloneDialogActive: false, diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index a290802116..86adb04d04 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -111,6 +111,24 @@ import RootController from '../../lib/controllers/root-controller'; }); }); + it('is rendered but not activated when startOpen prop is false', async function() { + const workdirPath = await cloneRepository('multiple-commits'); + const repository = await buildRepository(workdirPath); + + app = React.cloneElement(app, {repository, startOpen: false}); + const wrapper = shallow(app); + + assertInitialTabState({ + wrapper, tabName: 'git', + rendered: true, activated: false, visible: false, + }); + + assertInitialTabState({ + wrapper, tabName: 'github', + rendered: true, activated: false, visible: false, + }); + }); + it('is initially activated when the startOpen prop is true', async function() { const workdirPath = await cloneRepository('multiple-commits'); const repository = await buildRepository(workdirPath); From 1b7634847ad7ed528b71b518afac28a6b044a5f2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 15:39:10 -0400 Subject: [PATCH 20/21] Set useLegacyPanels in constructor --- lib/github-package.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 0a7bc70d5c..871d9a5086 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -39,7 +39,7 @@ export default class GithubPackage { this.styleCalculator = new StyleCalculator(this.styles, this.config); this.confirm = confirm; - this.useLegacyPanels = false; + this.useLegacyPanels = !this.workspace.getLeftDock || this.config.get('github.useLegacyPanels'); this.startOpen = false; const criteria = { @@ -128,10 +128,6 @@ export default class GithubPackage { await writeFile(this.configPath, '# Store non-visible GitHub package state.'); } - if (!this.workspace.getLeftDock || this.config.get('github.useLegacyPanels')) { - this.useLegacyPanels = true; - } - this.subscriptions.add( atom.config.onDidChange('github.useLegacyPanels', ({newValue}) => { if (newValue) { From fe64450c3a49483ec0357a18cb38cddd4068341e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 1 Jun 2017 15:52:46 -0400 Subject: [PATCH 21/21] Avoid rerender() call before activation has completed --- lib/github-package.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/github-package.js b/lib/github-package.js index 871d9a5086..10f5390f65 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -41,6 +41,7 @@ export default class GithubPackage { this.confirm = confirm; this.useLegacyPanels = !this.workspace.getLeftDock || this.config.get('github.useLegacyPanels'); this.startOpen = false; + this.activated = false; const criteria = { projectPathCount: this.project.getPaths().length, @@ -125,7 +126,7 @@ export default class GithubPackage { const firstRun = !await fileExists(this.configPath); this.startOpen = firstRun && !this.config.get('welcome.showOnStartup'); if (firstRun) { - await writeFile(this.configPath, '# Store non-visible GitHub package state.'); + await writeFile(this.configPath, '# Store non-visible GitHub package state.\n'); } this.subscriptions.add( @@ -166,6 +167,7 @@ export default class GithubPackage { this.workspace.addOpener(IssueishPaneItem.opener), ); + this.activated = true; this.scheduleActiveContextUpdate(this.savedState); this.rerender(); } @@ -187,6 +189,10 @@ export default class GithubPackage { return; } + if (!this.activated) { + return; + } + if (!this.element) { this.element = document.createElement('div'); this.subscriptions.add(new Disposable(() => {