diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 67d5c7b14c..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, @@ -110,16 +118,26 @@ 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, }); 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, @@ -133,6 +151,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() { @@ -243,8 +264,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.getDockItem = () => { - const item = getDockItem(); - return item ? item.getDockItem() : null; + this.setStateKey = (value, options = {activateTab: false}) => { + return new Promise(resolve => setState(value, options, resolve)); }; } @@ -842,14 +861,29 @@ class TabTracker { const focusToRestore = document.activeElement; let shouldRestoreFocus = false; - if (!this.getState()) { - await this.setStateKey(true); + // 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, {activateTab: 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 if (this.getDockItem()) { - shouldRestoreFocus = await this.getWorkspace().toggle(this.getDockItem()) !== undefined; } else { - // Legacy panels. - await this.setStateKey(false); + // Rendered and an active item within a visible dock. + if (this.useLegacyPanels) { + await this.setStateKey(false); + } else { + await this.hide(); + } + shouldRestoreFocus = false; } if (shouldRestoreFocus) { @@ -875,15 +909,23 @@ class TabTracker { @autobind async ensureVisible() { if (!this.isVisible()) { - await this.setStateKey(true); - if (this.getDockItem()) { - await this.getWorkspace().open(this.getDockItem()); - } + await this.setStateKey(true, {activateTab: true}); + await this.reveal(); return true; } 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(); } @@ -902,7 +944,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() { diff --git a/lib/github-package.js b/lib/github-package.js index 63c3e6ea52..10f5390f65 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 { @@ -41,8 +39,9 @@ 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; + this.activated = false; const criteria = { projectPathCount: this.project.getPaths().length, @@ -127,11 +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.'); - } - - if (!this.workspace.getLeftDock || this.config.get('github.useLegacyPanels')) { - this.useLegacyPanels = true; + await writeFile(this.configPath, '# Store non-visible GitHub package state.\n'); } this.subscriptions.add( @@ -172,6 +167,7 @@ export default class GithubPackage { this.workspace.addOpener(IssueishPaneItem.opener), ); + this.activated = true; this.scheduleActiveContextUpdate(this.savedState); this.rerender(); } @@ -193,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(() => { diff --git a/lib/views/dock-item.js b/lib/views/dock-item.js index 8de921fe9e..52511f02ab 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,26 @@ export default class DockItem extends React.Component { } } + getDockItem() { + return this.dockItem; + } + + getDockItemPromise() { + 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()) { diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 0debb66255..86adb04d04 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -17,12 +17,8 @@ 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 isGitPaneDisplayed(wrapper) { - if (useLegacyPanels || !workspace.getLeftDock) { - return wrapper.find('Panel').prop('visible'); - } else { - return wrapper.find('DockItem').exists(); - } + function useDocks() { + return !useLegacyPanels && !!workspace.getLeftDock; } beforeEach(function() { @@ -58,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() { @@ -75,17 +100,51 @@ 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 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 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, + }); }); }); @@ -315,101 +374,158 @@ 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); + ['git', 'github'].forEach(function(tabName) { + describe(`${tabName} tab tracker`, function() { + let wrapper, tabTracker, stateKey, tabIndex, mockDockItem; - app = React.cloneElement(app, {repository}); - const wrapper = shallow(app); + beforeEach(async function() { + const workdirPath = await cloneRepository('multiple-commits'); + const repository = await buildRepository(workdirPath); - assert.isFalse(isGitPaneDisplayed(wrapper)); - wrapper.find('ObserveModelDecorator(StatusBarTileController)').prop('toggleGitTab')(); - assert.isTrue(isGitPaneDisplayed(wrapper)); - wrapper.find('ObserveModelDecorator(StatusBarTileController)').prop('toggleGitTab')(); - assert.isFalse(isGitPaneDisplayed(wrapper)); - }); - }); + const savedState = { + gitTabActive: false, + githubTabActive: false, + }; + app = React.cloneElement(app, {repository, savedState}); + wrapper = shallow(app); + tabTracker = wrapper.instance()[`${tabName}TabTracker`]; + + sinon.stub(tabTracker, 'focus'); + sinon.spy(workspace.getActivePane(), 'activate'); + + stateKey = `${tabName}TabActive`; + tabIndex = {git: 0, github: 1}[tabName]; + + const FAKE_PANE_ITEM = Symbol('fake pane item'); + mockDockItem = { + active: false, + reveal() { + this.active = true; + return Promise.resolve(); + }, + hide() { + this.active = false; + return Promise.resolve(); + }, + getDockItem() { + return FAKE_PANE_ITEM; + }, + }; - describe('git tab tracker', function() { - let wrapper, gitTabTracker; + if (useDocks()) { + wrapper.instance()[`${tabName}DockItem`] = mockDockItem; - beforeEach(async function() { - const workdirPath = await cloneRepository('multiple-commits'); - const repository = await buildRepository(workdirPath); + sinon.stub(workspace.getRightDock(), 'isVisible').returns(true); + sinon.stub(workspace.getRightDock(), 'getPanes').callsFake(() => [{ + getActiveItem() { return mockDockItem.active ? FAKE_PANE_ITEM : null; }, + }]); + } + }); - app = React.cloneElement(app, {repository}); - wrapper = shallow(app); - gitTabTracker = wrapper.instance().gitTabTracker; + function assertTabState({rendered, active}) { + if (useDocks()) { + const isRendered = wrapper.find('DockItem').find({stubItemSelector: `${tabName}-tab-controller`}).exists(); + const isActive = mockDockItem.active; - sinon.stub(gitTabTracker, 'focus'); - sinon.spy(workspace.getActivePane(), 'activate'); - }); + assert.equal(isRendered, rendered); + 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]; - describe('toggle()', function() { - it('toggles the visibility of the Git view', function() { - assert.isFalse(isGitPaneDisplayed(wrapper)); - gitTabTracker.toggle(); - assert.isTrue(isGitPaneDisplayed(wrapper)); - gitTabTracker.toggle(); - assert.isFalse(isGitPaneDisplayed(wrapper)); - }); - }); + const isActive = wrapper.find('Panel').find({visible: true}) + .find('Tabs').find({activeIndex: tabIndex}) + .find('TabPanel').find({title}) + .exists(); - 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); + assert.equal(isActive, active); + } + } - await gitTabTracker.toggleFocus(); + describe('toggle()', function() { + it(`renders and reveals the ${tabName} tab when item is not rendered`, async function() { + assertTabState({rendered: false, active: 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, active: true}); + }); - assert.isTrue(isGitPaneDisplayed(wrapper)); + if (!useLegacyPanels) { + it(`reveals the ${tabName} tab when the item is rendered but not active`, async function() { + wrapper.setState({[stateKey]: true}); - await gitTabTracker.toggleFocus(); + assertTabState({rendered: true, active: false}); - assert.isTrue(isGitPaneDisplayed(wrapper)); - assert.equal(gitTabTracker.focus.callCount, 1); - assert.isFalse(workspace.getActivePane().activate.called); - }); + await tabTracker.toggle(); + + assertTabState({rendered: true, active: true}); + }); + } - it('blurs the Git panel when it is already open and focused', async function() { - gitTabTracker.toggle(); - sinon.stub(gitTabTracker, 'hasFocus').returns(true); + it(`hides the ${tabName} tab when open`, async function() { + wrapper.setState({[stateKey]: true, activeTab: tabIndex}); + mockDockItem.active = true; - assert.isTrue(isGitPaneDisplayed(wrapper)); + assertTabState({rendered: true, active: true}); - await gitTabTracker.toggleFocus(); + await tabTracker.toggle(); - assert.isTrue(isGitPaneDisplayed(wrapper)); - assert.equal(gitTabTracker.focus.callCount, 0); - assert.isTrue(workspace.getActivePane().activate.called); + assertTabState({rendered: true, active: false}); + }); }); - }); - 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)); + describe('toggleFocus()', function() { + it(`opens and focuses the ${tabName} tab when it is initially closed`, async function() { + assertTabState({rendered: false, active: false}); + sinon.stub(tabTracker, 'hasFocus').returns(false); + + await tabTracker.toggleFocus(); + + 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, active: true}); + sinon.stub(tabTracker, 'hasFocus').returns(false); + + await tabTracker.toggleFocus(); - assert.isTrue(await gitTabTracker.ensureVisible()); + 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, active: true}); + sinon.stub(tabTracker, 'hasFocus').returns(true); + + await tabTracker.toggleFocus(); + + assertTabState({rendered: true, active: 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, active: false}); + assert.isTrue(await tabTracker.ensureVisible()); + assertTabState({rendered: true, active: true}); + }); - assert.isFalse(await gitTabTracker.ensureVisible()); - assert.isTrue(isGitPaneDisplayed(wrapper)); + it(`does nothing when the ${tabName} tab is already open`, async function() { + await tabTracker.toggle(); + assertTabState({rendered: true, active: true}); + assert.isFalse(await tabTracker.ensureVisible()); + assertTabState({rendered: true, active: true}); + }); }); }); }); diff --git a/test/helpers.js b/test/helpers.js index 3a4fa8b398..c95294de53 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; } @@ -53,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); } @@ -78,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}; }