From 104d3c4b75ced662f21358433168a4ee6c9b0249 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 13:33:01 -0400 Subject: [PATCH 01/41] Rename commandRegistry to commands because shut up that's why --- docs/focus-management.md | 2 +- lib/controllers/commit-controller.js | 4 +- lib/controllers/editor-conflict-controller.js | 4 +- lib/controllers/git-tab-controller.js | 6 +- lib/controllers/recent-commits-controller.js | 4 +- .../repository-conflict-controller.js | 4 +- lib/controllers/root-controller.js | 30 +++++----- lib/controllers/status-bar-tile-controller.js | 6 +- lib/github-package.js | 6 +- lib/index.js | 2 +- lib/views/branch-menu-view.js | 4 +- lib/views/clone-dialog.js | 4 +- lib/views/co-author-form.js | 4 +- lib/views/commit-view.js | 10 ++-- lib/views/credential-dialog.js | 4 +- lib/views/git-tab-view.js | 10 ++-- lib/views/open-commit-dialog.js | 4 +- lib/views/open-issueish-dialog.js | 4 +- lib/views/recent-commits-view.js | 4 +- lib/views/staging-view.js | 6 +- test/atom/commands.test.js | 32 +++++----- test/controllers/commit-controller.test.js | 6 +- .../editor-conflict-controller.test.js | 58 +++++++++---------- test/controllers/git-tab-controller.test.js | 14 ++--- .../recent-commits-controller.test.js | 2 +- .../repository-conflict-controller.test.js | 4 +- test/controllers/root-controller.test.js | 10 ++-- .../status-bar-tile-controller.test.js | 14 ++--- test/fixtures/props/git-tab-props.js | 4 +- test/github-package.test.js | 10 ++-- test/integration/helpers.js | 2 +- test/views/clone-dialog.test.js | 6 +- test/views/co-author-form.test.js | 2 +- test/views/commit-view.test.js | 12 ++-- test/views/credential-dialog.test.js | 2 +- test/views/open-commit-dialog.test.js | 6 +- test/views/open-issueish-dialog.test.js | 6 +- test/views/recent-commits-view.test.js | 2 +- test/views/staging-view.test.js | 18 +++--- 39 files changed, 166 insertions(+), 166 deletions(-) diff --git a/docs/focus-management.md b/docs/focus-management.md index 2b24b5ed39..738a511aa6 100644 --- a/docs/focus-management.md +++ b/docs/focus-management.md @@ -29,7 +29,7 @@ We move focus around by registering Atom commands. For example, in `GitTabView`: ``` - this.props.commandRegistry.add(this.refRoot, { + this.props.commands.add(this.refRoot, { 'tool-panel:unfocus': this.blur, 'core:focus-next': this.advanceFocus, 'core:focus-previous': this.retreatFocus, diff --git a/lib/controllers/commit-controller.js b/lib/controllers/commit-controller.js index 7183eeb31a..f8d64d6bef 100644 --- a/lib/controllers/commit-controller.js +++ b/lib/controllers/commit-controller.js @@ -24,7 +24,7 @@ export default class CommitController extends React.Component { static propTypes = { workspace: PropTypes.object.isRequired, grammars: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, config: PropTypes.object.isRequired, tooltips: PropTypes.object.isRequired, @@ -104,7 +104,7 @@ export default class CommitController extends React.Component { prepareToCommit={this.props.prepareToCommit} commit={this.commit} abortMerge={this.props.abortMerge} - commandRegistry={this.props.commandRegistry} + commands={this.props.commands} maximumCharacterLimit={72} messageBuffer={this.commitMessageBuffer} isMerging={this.props.isMerging} diff --git a/lib/controllers/editor-conflict-controller.js b/lib/controllers/editor-conflict-controller.js index 79f7309512..ad4c3c2848 100644 --- a/lib/controllers/editor-conflict-controller.js +++ b/lib/controllers/editor-conflict-controller.js @@ -15,7 +15,7 @@ import {autobind} from '../helpers'; export default class EditorConflictController extends React.Component { static propTypes = { editor: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, resolutionProgress: PropTypes.object.isRequired, isRebase: PropTypes.bool.isRequired, refreshResolutionProgress: PropTypes.func.isRequired, @@ -56,7 +56,7 @@ export default class EditorConflictController extends React.Component { return (
{this.state.conflicts.size > 0 && ( - + diff --git a/lib/controllers/git-tab-controller.js b/lib/controllers/git-tab-controller.js index 647f8f9535..44ab1e8066 100644 --- a/lib/controllers/git-tab-controller.js +++ b/lib/controllers/git-tab-controller.js @@ -35,7 +35,7 @@ export default class GitTabController extends React.Component { fetchInProgress: PropTypes.bool.isRequired, workspace: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, grammars: PropTypes.object.isRequired, resolutionProgress: PropTypes.object.isRequired, notificationManager: PropTypes.object.isRequired, @@ -49,7 +49,7 @@ export default class GitTabController extends React.Component { undoLastDiscard: PropTypes.func.isRequired, discardWorkDirChangesForPaths: PropTypes.func.isRequired, openFiles: PropTypes.func.isRequired, - initializeRepo: PropTypes.func.isRequired, + openInitializeDialog: PropTypes.func.isRequired, controllerRef: RefHolderPropType, }; @@ -107,7 +107,7 @@ export default class GitTabController extends React.Component { resolutionProgress={this.props.resolutionProgress} workspace={this.props.workspace} - commandRegistry={this.props.commandRegistry} + commands={this.props.commands} grammars={this.props.grammars} tooltips={this.props.tooltips} notificationManager={this.props.notificationManager} diff --git a/lib/controllers/recent-commits-controller.js b/lib/controllers/recent-commits-controller.js index adfd83d2d2..fd64391098 100644 --- a/lib/controllers/recent-commits-controller.js +++ b/lib/controllers/recent-commits-controller.js @@ -15,7 +15,7 @@ export default class RecentCommitsController extends React.Component { undoLastCommit: PropTypes.func.isRequired, workspace: PropTypes.object.isRequired, repository: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, } static focus = RecentCommitsView.focus @@ -62,7 +62,7 @@ export default class RecentCommitsController extends React.Component { selectNextCommit={this.selectNextCommit} selectPreviousCommit={this.selectPreviousCommit} selectedCommitSha={this.state.selectedCommitSha} - commandRegistry={this.props.commandRegistry} + commands={this.props.commands} /> ); } diff --git a/lib/controllers/repository-conflict-controller.js b/lib/controllers/repository-conflict-controller.js index 68aabea1d6..e34b1a490a 100644 --- a/lib/controllers/repository-conflict-controller.js +++ b/lib/controllers/repository-conflict-controller.js @@ -19,7 +19,7 @@ const DEFAULT_REPO_DATA = { export default class RepositoryConflictController extends React.Component { static propTypes = { workspace: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, config: PropTypes.object.isRequired, resolutionProgress: PropTypes.object.isRequired, repository: PropTypes.object.isRequired, @@ -78,7 +78,7 @@ export default class RepositoryConflictController extends React.Component { {conflictingEditors.map(editor => ( @@ -258,7 +258,7 @@ export default class RootController extends React.Component { return ( - + ); } @@ -286,7 +286,7 @@ export default class RootController extends React.Component { return ( ); } @@ -322,7 +322,7 @@ export default class RootController extends React.Component { diff --git a/lib/controllers/status-bar-tile-controller.js b/lib/controllers/status-bar-tile-controller.js index 07842aa630..80c0ca3994 100644 --- a/lib/controllers/status-bar-tile-controller.js +++ b/lib/controllers/status-bar-tile-controller.js @@ -16,7 +16,7 @@ export default class StatusBarTileController extends React.Component { static propTypes = { workspace: PropTypes.object.isRequired, notificationManager: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, tooltips: PropTypes.object.isRequired, confirm: PropTypes.func.isRequired, repository: PropTypes.object.isRequired, @@ -111,7 +111,7 @@ export default class StatusBarTileController extends React.Component { return ( - + { this.controller = c; }} workspace={this.workspace} deserializers={this.deserializers} - commandRegistry={this.commandRegistry} + commands={this.commands} notificationManager={this.notificationManager} tooltips={this.tooltips} grammars={this.grammars} diff --git a/lib/index.js b/lib/index.js index accea10ea8..814bca9773 100644 --- a/lib/index.js +++ b/lib/index.js @@ -6,7 +6,7 @@ const entry = { pack = new GithubPackage({ workspace: atom.workspace, project: atom.project, - commandRegistry: atom.commands, + commands: atom.commands, notificationManager: atom.notifications, tooltips: atom.tooltips, styles: atom.styles, diff --git a/lib/views/branch-menu-view.js b/lib/views/branch-menu-view.js index e81adf7254..85b0edcb0e 100644 --- a/lib/views/branch-menu-view.js +++ b/lib/views/branch-menu-view.js @@ -10,7 +10,7 @@ import {autobind} from '../helpers'; export default class BranchMenuView extends React.Component { static propTypes = { workspace: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, notificationManager: PropTypes.object.isRequired, repository: PropTypes.object, branches: BranchSetPropType.isRequired, @@ -85,7 +85,7 @@ export default class BranchMenuView extends React.Component { return (
- + diff --git a/lib/views/clone-dialog.js b/lib/views/clone-dialog.js index 68d41854be..69080f46de 100644 --- a/lib/views/clone-dialog.js +++ b/lib/views/clone-dialog.js @@ -10,7 +10,7 @@ import {autobind} from '../helpers'; export default class CloneDialog extends React.Component { static propTypes = { config: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, inProgress: PropTypes.bool, didAccept: PropTypes.func, didCancel: PropTypes.func, @@ -56,7 +56,7 @@ export default class CloneDialog extends React.Component { renderDialog() { return (
- + diff --git a/lib/views/co-author-form.js b/lib/views/co-author-form.js index 23d5f9f1ba..92efdd695c 100644 --- a/lib/views/co-author-form.js +++ b/lib/views/co-author-form.js @@ -7,7 +7,7 @@ import {autobind} from '../helpers'; export default class CoAuthorForm extends React.Component { static propTypes = { - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, onSubmit: PropTypes.func, onCancel: PropTypes.func, name: PropTypes.string, @@ -36,7 +36,7 @@ export default class CoAuthorForm extends React.Component { render() { return (
- + diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index a12b50f0c9..0572af2f3a 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -40,7 +40,7 @@ export default class CommitView extends React.Component { workspace: PropTypes.object.isRequired, config: PropTypes.object.isRequired, tooltips: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, lastCommit: PropTypes.object.isRequired, currentBranch: PropTypes.object.isRequired, @@ -147,14 +147,14 @@ export default class CommitView extends React.Component { return (
- + - + @@ -168,7 +168,7 @@ export default class CommitView extends React.Component { - +
@@ -358,7 +358,7 @@ export default class CommitView extends React.Component { return ( - + diff --git a/lib/views/git-tab-view.js b/lib/views/git-tab-view.js index 20e54b9164..1539ccb6eb 100644 --- a/lib/views/git-tab-view.js +++ b/lib/views/git-tab-view.js @@ -40,7 +40,7 @@ export default class GitTabView extends React.Component { updateSelectedCoAuthors: PropTypes.func.isRequired, workspace: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, grammars: PropTypes.object.isRequired, resolutionProgress: PropTypes.object.isRequired, notificationManager: PropTypes.object.isRequired, @@ -75,7 +75,7 @@ export default class GitTabView extends React.Component { componentDidMount() { this.props.refRoot.map(root => { return this.subscriptions.add( - this.props.commandRegistry.add(root, { + this.props.commands.add(root, { 'tool-panel:unfocus': this.blur, 'core:focus-next': this.advanceFocus, 'core:focus-previous': this.retreatFocus, @@ -143,7 +143,7 @@ export default class GitTabView extends React.Component { ref={this.props.refRoot.setter}> - + diff --git a/lib/views/open-issueish-dialog.js b/lib/views/open-issueish-dialog.js index dbd21ff04d..069dbbf601 100644 --- a/lib/views/open-issueish-dialog.js +++ b/lib/views/open-issueish-dialog.js @@ -9,7 +9,7 @@ const ISSUEISH_URL_REGEX = /^(?:https?:\/\/)?github.com\/([^/]+)\/([^/]+)\/(?:is export default class OpenIssueishDialog extends React.Component { static propTypes = { - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, didAccept: PropTypes.func, didCancel: PropTypes.func, } @@ -43,7 +43,7 @@ export default class OpenIssueishDialog extends React.Component { renderDialog() { return (
- + diff --git a/lib/views/recent-commits-view.js b/lib/views/recent-commits-view.js index aee9718f2c..0624bfb0db 100644 --- a/lib/views/recent-commits-view.js +++ b/lib/views/recent-commits-view.js @@ -118,7 +118,7 @@ export default class RecentCommitsView extends React.Component { selectedCommitSha: PropTypes.string.isRequired, // Atom environment - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, // Action methods undoLastCommit: PropTypes.func.isRequired, @@ -160,7 +160,7 @@ export default class RecentCommitsView extends React.Component { render() { return (
- + diff --git a/lib/views/staging-view.js b/lib/views/staging-view.js index 474db1aaa0..fa73b425fc 100644 --- a/lib/views/staging-view.js +++ b/lib/views/staging-view.js @@ -56,7 +56,7 @@ export default class StagingView extends React.Component { workingDirectoryPath: PropTypes.string, resolutionProgress: PropTypes.object, hasUndoHistory: PropTypes.bool.isRequired, - commandRegistry: PropTypes.object.isRequired, + commands: PropTypes.object.isRequired, notificationManager: PropTypes.object.isRequired, workspace: PropTypes.object.isRequired, openFiles: PropTypes.func.isRequired, @@ -267,7 +267,7 @@ export default class StagingView extends React.Component { renderCommands() { return ( - + this.selectPrevious()} /> this.selectNext()} /> @@ -288,7 +288,7 @@ export default class StagingView extends React.Component { - + diff --git a/test/atom/commands.test.js b/test/atom/commands.test.js index 246fdc462b..b073a94b45 100644 --- a/test/atom/commands.test.js +++ b/test/atom/commands.test.js @@ -5,11 +5,11 @@ import Commands, {Command} from '../../lib/atom/commands'; import RefHolder from '../../lib/models/ref-holder'; describe('Commands', function() { - let atomEnv, commandRegistry; + let atomEnv, commands; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); - commandRegistry = atomEnv.commands; + commands = atomEnv.commands; }); afterEach(function() { @@ -21,16 +21,16 @@ describe('Commands', function() { const callback2 = sinon.stub(); const element = document.createElement('div'); const app = ( - + ); const wrapper = mount(app); - commandRegistry.dispatch(element, 'github:do-thing1'); + commands.dispatch(element, 'github:do-thing1'); assert.equal(callback1.callCount, 1); - commandRegistry.dispatch(element, 'github:do-thing2'); + commands.dispatch(element, 'github:do-thing2'); assert.equal(callback2.callCount, 1); await new Promise(resolve => { @@ -39,18 +39,18 @@ describe('Commands', function() { callback1.reset(); callback2.reset(); - commandRegistry.dispatch(element, 'github:do-thing1'); + commands.dispatch(element, 'github:do-thing1'); assert.equal(callback1.callCount, 1); - commandRegistry.dispatch(element, 'github:do-thing2'); + commands.dispatch(element, 'github:do-thing2'); assert.equal(callback2.callCount, 0); wrapper.unmount(); callback1.reset(); callback2.reset(); - commandRegistry.dispatch(element, 'github:do-thing1'); + commands.dispatch(element, 'github:do-thing1'); assert.equal(callback1.callCount, 0); - commandRegistry.dispatch(element, 'github:do-thing2'); + commands.dispatch(element, 'github:do-thing2'); assert.equal(callback2.callCount, 0); }); @@ -63,7 +63,7 @@ describe('Commands', function() { render() { return ( ; const wrapper = mount(app); - commandRegistry.dispatch(element, 'user:command1'); + commands.dispatch(element, 'user:command1'); assert.equal(callback1.callCount, 1); await new Promise(resolve => { @@ -83,10 +83,10 @@ describe('Commands', function() { }); callback1.reset(); - commandRegistry.dispatch(element, 'user:command1'); + commands.dispatch(element, 'user:command1'); assert.equal(callback1.callCount, 0); assert.equal(callback2.callCount, 0); - commandRegistry.dispatch(element, 'user:command2'); + commands.dispatch(element, 'user:command2'); assert.equal(callback1.callCount, 0); assert.equal(callback2.callCount, 1); }); @@ -95,17 +95,17 @@ describe('Commands', function() { const callback = sinon.spy(); const holder = new RefHolder(); mount( - + , ); const element = document.createElement('div'); - commandRegistry.dispatch(element, 'github:do-thing'); + commands.dispatch(element, 'github:do-thing'); assert.isFalse(callback.called); holder.setter(element); - commandRegistry.dispatch(element, 'github:do-thing'); + commands.dispatch(element, 'github:do-thing'); assert.isTrue(callback.called); }); }); diff --git a/test/controllers/commit-controller.test.js b/test/controllers/commit-controller.test.js index 2b7ba9f32a..e1121a27f6 100644 --- a/test/controllers/commit-controller.test.js +++ b/test/controllers/commit-controller.test.js @@ -13,13 +13,13 @@ import {cloneRepository, buildRepository, buildRepositoryWithPipeline, registerG import * as reporterProxy from '../../lib/reporter-proxy'; describe('CommitController', function() { - let atomEnvironment, workspace, commandRegistry, notificationManager, lastCommit, config, confirm, tooltips; + let atomEnvironment, workspace, commands, notificationManager, lastCommit, config, confirm, tooltips; let app; beforeEach(function() { atomEnvironment = global.buildAtomEnvironment(); workspace = atomEnvironment.workspace; - commandRegistry = atomEnvironment.commands; + commands = atomEnvironment.commands; notificationManager = atomEnvironment.notifications; config = atomEnvironment.config; tooltips = atomEnvironment.tooltips; @@ -35,7 +35,7 @@ describe('CommitController', function() { e.setText(newMessage)); - commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit'); + commands.dispatch(workspaceElement, 'github:amend-last-commit'); // verify that coAuthor was passed await assert.async.deepEqual( @@ -600,7 +600,7 @@ describe('GitTabController', function() { commitView.onSelectedCoAuthorsChanged([]); // amend again - commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit'); + commands.dispatch(workspaceElement, 'github:amend-last-commit'); // verify that NO coAuthor was passed await assert.async.deepEqual( repository.commit.args[0][1], diff --git a/test/controllers/recent-commits-controller.test.js b/test/controllers/recent-commits-controller.test.js index 875c2fb3fa..949bf58ea8 100644 --- a/test/controllers/recent-commits-controller.test.js +++ b/test/controllers/recent-commits-controller.test.js @@ -22,7 +22,7 @@ describe('RecentCommitsController', function() { isLoading={false} undoLastCommit={() => { }} workspace={atomEnv.workspace} - commandRegistry={atomEnv.commands} + commands={atomEnv.commands} repository={repository} /> ); diff --git a/test/controllers/repository-conflict-controller.test.js b/test/controllers/repository-conflict-controller.test.js index af3963a05e..b1d7e84faa 100644 --- a/test/controllers/repository-conflict-controller.test.js +++ b/test/controllers/repository-conflict-controller.test.js @@ -14,9 +14,9 @@ describe('RepositoryConflictController', () => { atomEnv = global.buildAtomEnvironment(); atomEnv.config.set('github.graphicalConflictResolution', true); workspace = atomEnv.workspace; - const commandRegistry = atomEnv.commands; + const commands = atomEnv.commands; - app = ; + app = ; }); afterEach(() => atomEnv.destroy()); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 91295710de..626d08dc41 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -31,7 +31,7 @@ describe('RootController', function() { beforeEach(function() { atomEnv = global.buildAtomEnvironment(); workspace = atomEnv.workspace; - commandRegistry = atomEnv.commands; + commands = atomEnv.commands; deserializers = atomEnv.deserializers; grammars = atomEnv.grammars; notificationManager = atomEnv.notifications; @@ -49,7 +49,7 @@ describe('RootController', function() { app = ( ({initialPaths}); githubPackage1 = new GithubPackage({ - workspace, project, commandRegistry, notificationManager, tooltips, styles, grammars, keymaps, + workspace, project, commands, notificationManager, tooltips, styles, grammars, keymaps, config, deserializers, confirm, getLoadSettings: getLoadSettings1, configDirPath, }); @@ -501,7 +501,7 @@ describe('GithubPackage', function() { project.setPaths([workdir1]); await workspace.open(path.join(workdir0, 'a.txt')); - commandRegistry.dispatch(atomEnv.views.getView(workspace), 'tree-view:toggle-focus'); + commands.dispatch(atomEnv.views.getView(workspace), 'tree-view:toggle-focus'); workspace.getLeftDock().activate(); await githubPackage.scheduleActiveContextUpdate(); diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 616d66f705..4bbce8fa8b 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -79,7 +79,7 @@ export async function setup(options = {}) { const githubPackage = new GithubPackage({ workspace: atomEnv.workspace, project: atomEnv.project, - commandRegistry: atomEnv.commands, + commands: atomEnv.commands, notificationManager: atomEnv.notifications, tooltips: atomEnv.tooltips, styles: atomEnv.styles, diff --git a/test/views/clone-dialog.test.js b/test/views/clone-dialog.test.js index fd3c9a1f39..fb015dc2ef 100644 --- a/test/views/clone-dialog.test.js +++ b/test/views/clone-dialog.test.js @@ -5,13 +5,13 @@ import path from 'path'; import CloneDialog from '../../lib/views/clone-dialog'; describe('CloneDialog', function() { - let atomEnv, config, commandRegistry; + let atomEnv, config, commands; let app, wrapper, didAccept, didCancel; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); config = atomEnv.config; - commandRegistry = atomEnv.commands; + commands = atomEnv.commands; sinon.stub(config, 'get').returns(path.join('home', 'me', 'codes')); didAccept = sinon.stub(); @@ -20,7 +20,7 @@ describe('CloneDialog', function() { app = ( diff --git a/test/views/co-author-form.test.js b/test/views/co-author-form.test.js index b2980682ce..cf67e10990 100644 --- a/test/views/co-author-form.test.js +++ b/test/views/co-author-form.test.js @@ -16,7 +16,7 @@ describe('CoAuthorForm', function() { app = ( diff --git a/test/views/commit-view.test.js b/test/views/commit-view.test.js index fe239fceb0..fe3776b5f4 100644 --- a/test/views/commit-view.test.js +++ b/test/views/commit-view.test.js @@ -15,13 +15,13 @@ import StagingView from '../../lib/views/staging-view'; import * as reporterProxy from '../../lib/reporter-proxy'; describe('CommitView', function() { - let atomEnv, commandRegistry, tooltips, config, lastCommit; + let atomEnv, commands, tooltips, config, lastCommit; let messageBuffer; let app; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); - commandRegistry = atomEnv.commands; + commands = atomEnv.commands; tooltips = atomEnv.tooltips; config = atomEnv.config; @@ -35,7 +35,7 @@ describe('CommitView', function() { app = ( diff --git a/test/views/recent-commits-view.test.js b/test/views/recent-commits-view.test.js index 9f494283c7..b0eda50bba 100644 --- a/test/views/recent-commits-view.test.js +++ b/test/views/recent-commits-view.test.js @@ -16,7 +16,7 @@ describe('RecentCommitsView', function() { commits={[]} isLoading={false} selectedCommitSha="" - commandRegistry={atomEnv.commands} + commands={atomEnv.commands} undoLastCommit={() => { }} openCommit={() => { }} selectNextCommit={() => { }} diff --git a/test/views/staging-view.test.js b/test/views/staging-view.test.js index 996a4c6f6a..31f72f37bf 100644 --- a/test/views/staging-view.test.js +++ b/test/views/staging-view.test.js @@ -11,12 +11,12 @@ import {assertEqualSets} from '../helpers'; describe('StagingView', function() { const workingDirectoryPath = '/not/real/'; - let atomEnv, commandRegistry, workspace, notificationManager; + let atomEnv, commands, workspace, notificationManager; let app; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); - commandRegistry = atomEnv.commands; + commands = atomEnv.commands; workspace = atomEnv.workspace; notificationManager = atomEnv.notifications; @@ -31,7 +31,7 @@ describe('StagingView', function() { stagedChanges={[]} workingDirectoryPath={workingDirectoryPath} hasUndoHistory={false} - commandRegistry={commandRegistry} + commands={commands} notificationManager={notificationManager} workspace={workspace} openFiles={noop} @@ -98,7 +98,7 @@ describe('StagingView', function() { .simulate('mousedown', {button: 0}); await wrapper.instance().mouseup(); - commandRegistry.dispatch(wrapper.getDOMNode(), 'core:confirm'); + commands.dispatch(wrapper.getDOMNode(), 'core:confirm'); await assert.async.isTrue(attemptFileStageOperation.calledWith(['b.txt'], 'unstaged')); }); @@ -113,7 +113,7 @@ describe('StagingView', function() { .simulate('mousedown', {button: 0}); await wrapper.instance().mouseup(); - commandRegistry.dispatch(wrapper.getDOMNode(), 'core:confirm'); + commands.dispatch(wrapper.getDOMNode(), 'core:confirm'); await assert.async.isTrue(attemptFileStageOperation.calledWith(['b.txt'], 'staged')); }); @@ -683,7 +683,7 @@ describe('StagingView', function() { it('invokes a callback only when a single file is selected', async function() { await wrapper.instance().selectFirst(); - commandRegistry.dispatch(wrapper.getDOMNode(), 'core:move-left'); + commands.dispatch(wrapper.getDOMNode(), 'core:move-left'); assert.isTrue(showFilePatchItem.calledWith('unstaged-1.txt'), 'Callback invoked with unstaged-1.txt'); @@ -693,7 +693,7 @@ describe('StagingView', function() { const selectedFilePaths = wrapper.instance().getSelectedItems().map(item => item.filePath).sort(); assert.deepEqual(selectedFilePaths, ['unstaged-1.txt', 'unstaged-2.txt']); - commandRegistry.dispatch(wrapper.getDOMNode(), 'core:move-left'); + commands.dispatch(wrapper.getDOMNode(), 'core:move-left'); assert.equal(showFilePatchItem.callCount, 0); }); @@ -702,7 +702,7 @@ describe('StagingView', function() { await wrapper.instance().activateNextList(); await wrapper.instance().selectFirst(); - commandRegistry.dispatch(wrapper.getDOMNode(), 'core:move-left'); + commands.dispatch(wrapper.getDOMNode(), 'core:move-left'); assert.isTrue(showMergeConflictFileForPath.calledWith('conflict-1.txt'), 'Callback invoked with conflict-1.txt'); @@ -711,7 +711,7 @@ describe('StagingView', function() { const selectedFilePaths = wrapper.instance().getSelectedItems().map(item => item.filePath).sort(); assert.deepEqual(selectedFilePaths, ['conflict-1.txt', 'conflict-2.txt']); - commandRegistry.dispatch(wrapper.getDOMNode(), 'core:move-left'); + commands.dispatch(wrapper.getDOMNode(), 'core:move-left'); assert.equal(showMergeConflictFileForPath.callCount, 0); }); From 14c7308b08b48c4956d8843b8166aef70091a18e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 13:42:10 -0400 Subject: [PATCH 02/41] Use InitDialog as a motivating sample for DialogsController --- lib/controllers/dialogs-controller.js | 121 ++++++++++++++++ lib/views/init-dialog.js | 148 ++++++++------------ styles/dialog.less | 2 +- test/controllers/dialogs-controller.test.js | 130 +++++++++++++++++ test/views/init-dialog.test.js | 89 ++++++------ 5 files changed, 358 insertions(+), 132 deletions(-) create mode 100644 lib/controllers/dialogs-controller.js create mode 100644 test/controllers/dialogs-controller.test.js diff --git a/lib/controllers/dialogs-controller.js b/lib/controllers/dialogs-controller.js new file mode 100644 index 0000000000..c9ce4b96e5 --- /dev/null +++ b/lib/controllers/dialogs-controller.js @@ -0,0 +1,121 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import Panel from '../atom/panel'; +import InitDialog from '../views/init-dialog'; + +export default class DialogsController extends React.Component { + static propTypes = { + // Model + request: PropTypes.shape({ + identifier: PropTypes.string.isRequired, + isProgressing: PropTypes.bool.isRequired, + }).isRequired, + + // Atom environment + workspace: PropTypes.object.isRequired, + }; + + constructor(props) { + super(props); + + this.dialogRenderFns = { + null: () => null, + init: this.renderInitDialog, + }; + + this.state = { + requestInProgress: null, + requestError: [null, null], + }; + } + + render() { + return this.dialogRenderFns[this.props.request.identifier](); + } + + renderInitDialog = () => ( + + + + ) + + getCommonProps() { + const {request} = this.props; + const accept = request.isProgressing + ? async (...args) => { + this.setState({requestInProgress: request}); + try { + return await request.accept(...args); + } catch (error) { + this.setState({requestError: [request, error]}); + return undefined; + } finally { + this.setState({requestInProgress: null}); + } + } : (...args) => { + try { + return request.accept(...args); + } catch (error) { + this.setState({requestError: [request, error]}); + return undefined; + } + }; + const wrapped = wrapDialogRequest(request, {accept}); + + return { + inProgress: this.state.requestInProgress === request, + error: this.state.requestError[0] === request ? this.state.requestError[1] : null, + request: wrapped, + }; + } +} + +class DialogRequest { + constructor(identifier, params = {}) { + this.identifier = identifier; + this.params = params; + this.isProgressing = false; + this.accept = () => {}; + this.cancel = () => {}; + } + + onAccept(cb) { + this.accept = cb; + } + + onProgressingAccept(cb) { + this.isProgressing = true; + this.onAccept(cb); + } + + onCancel(cb) { + this.cancel = cb; + } + + getParams() { + return this.params; + } +} + +function wrapDialogRequest(original, {accept}) { + const dup = new DialogRequest(original.identifier, original.getParams()); + dup.isProgressing = original.isProgressing; + dup.onAccept(accept); + dup.onCancel(original.cancel); + return dup; +} + +export const dialogRequests = { + null: { + identifier: 'null', + isProgressing: false, + params: {}, + accept: () => {}, + cancel: () => {}, + }, + + init({dirPath}) { + return new DialogRequest('init', {dirPath}); + }, +}; diff --git a/lib/views/init-dialog.js b/lib/views/init-dialog.js index 43985985e8..f0e901adac 100644 --- a/lib/views/init-dialog.js +++ b/lib/views/init-dialog.js @@ -1,118 +1,88 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {CompositeDisposable} from 'event-kit'; +import {TextBuffer} from 'atom'; -import Commands, {Command} from '../atom/commands'; -import {autobind} from '../helpers'; +import AtomTextEditor from '../atom/atom-text-editor'; export default class InitDialog extends React.Component { static propTypes = { - config: PropTypes.object.isRequired, - commandRegistry: PropTypes.object.isRequired, - didAccept: PropTypes.func, - didCancel: PropTypes.func, - initPath: PropTypes.string, + request: PropTypes.shape({ + getParams: PropTypes.func.isRequired, + accept: PropTypes.func.isRequired, + cancel: PropTypes.func.isRequired, + }).isRequired, + inProgress: PropTypes.bool, + error: PropTypes.instanceOf(Error), } - static defaultProps = { - didAccept: () => {}, - didCancel: () => {}, - } + constructor(props) { + super(props); + + this.destinationPath = new TextBuffer({ + text: this.props.request.getParams().dirPath, + }); - constructor(props, context) { - super(props, context); - autobind(this, 'init', 'cancel', 'editorRef', 'setInitEnablement'); + this.sub = this.destinationPath.onDidChange(this.setAcceptEnablement); this.state = { - initDisabled: false, + acceptEnabled: !this.destinationPath.isEmpty(), }; - - this.subs = new CompositeDisposable(); - } - - componentDidMount() { - if (this.projectPathEditor) { - this.projectPathEditor.setText(this.props.initPath || this.props.config.get('core.projectHome')); - this.projectPathModified = false; - } - - if (this.projectPathElement) { - setTimeout(() => this.projectPathElement.focus()); - } } render() { return (
- - - - -
-
); } - init() { - if (this.getProjectPath().length === 0) { - return; - } - - this.props.didAccept(this.getProjectPath()); - } - - cancel() { - this.props.didCancel(); + componentWillUnmount() { + this.sub.dispose(); } - editorRef() { - return element => { - if (!element) { - return; - } - - this.projectPathElement = element; - const editor = element.getModel(); - if (this.projectPathEditor !== editor) { - this.projectPathEditor = editor; - - if (this.projectPathSubs) { - this.projectPathSubs.dispose(); - this.subs.remove(this.projectPathSubs); - } - - this.projectPathSubs = editor.onDidChange(this.setInitEnablement); - this.subs.add(this.projectPathSubs); - } - }; - } + accept = () => this.props.request.accept(this.destinationPath.getText()); - getProjectPath() { - return this.projectPathEditor ? this.projectPathEditor.getText() : ''; - } - - getRemoteUrl() { - return this.remoteUrlEditor ? this.remoteUrlEditor.getText() : ''; - } - - setInitEnablement() { - this.setState({initDisabled: this.getProjectPath().length === 0}); + setAcceptEnablement = () => { + const enablement = !this.destinationPath.isEmpty(); + if (enablement !== this.state.acceptEnabled) { + this.setState({acceptEnabled: enablement}); + } } } diff --git a/styles/dialog.less b/styles/dialog.less index ef9df6efbe..1ddb0e8c64 100644 --- a/styles/dialog.less +++ b/styles/dialog.less @@ -10,7 +10,7 @@ font-size: 1.2em; } - &Inputs { + &Form { display: flex; flex-direction: column; padding: @github-dialog-spacing; diff --git a/test/controllers/dialogs-controller.test.js b/test/controllers/dialogs-controller.test.js new file mode 100644 index 0000000000..61a23dea00 --- /dev/null +++ b/test/controllers/dialogs-controller.test.js @@ -0,0 +1,130 @@ +import React from 'react'; +import {shallow} from 'enzyme'; + +import DialogsController, {dialogRequests} from '../../lib/controllers/dialogs-controller'; + +describe('DialogsController', function() { + let atomEnv; + + beforeEach(function() { + atomEnv = global.buildAtomEnvironment(); + }); + + afterEach(function() { + atomEnv.destroy(); + }); + + function buildApp(overrides = {}) { + return ( + + ); + } + + it('renders nothing when a nullDialogRequest is provided', function() { + const wrapper = shallow(buildApp({ + request: dialogRequests.null, + })); + assert.isTrue(wrapper.isEmptyRender()); + }); + + it('renders a chosen dialog when the appropriate DialogRequest is provided', function() { + const wrapper = shallow(buildApp({ + request: dialogRequests.init({dirPath: __dirname}), + })); + + const panel = wrapper.find('Panel'); + assert.strictEqual(panel.prop('location'), 'modal'); + assert.strictEqual(panel.prop('workspace'), atomEnv.workspace); + assert.isTrue(panel.exists('InitDialog')); + }); + + it('passes inProgress to the dialog when the accept callback is asynchronous', async function() { + let completeWork = () => {}; + const workPromise = new Promise(resolve => { + completeWork = resolve; + }); + const accept = sinon.stub().returns(workPromise); + + const request = dialogRequests.init({dirPath: '/not/home'}); + request.onProgressingAccept(accept); + + const wrapper = shallow(buildApp({request})); + assert.isFalse(wrapper.find('InitDialog').prop('inProgress')); + assert.isFalse(accept.called); + + const acceptPromise = wrapper.find('InitDialog').prop('request').accept('an-argument'); + assert.isTrue(wrapper.find('InitDialog').prop('inProgress')); + assert.isTrue(accept.calledWith('an-argument')); + + completeWork('some-result'); + assert.strictEqual(await acceptPromise, 'some-result'); + + wrapper.update(); + assert.isFalse(wrapper.find('InitDialog').prop('inProgress')); + }); + + describe('error handling', function() { + it('passes a raised error to the dialog when raised during a synchronous accept callback', function() { + const e = new Error('wtf'); + const request = dialogRequests.init({dirPath: __dirname}); + request.onAccept(() => { throw e; }); + + const wrapper = shallow(buildApp({request})); + wrapper.find('InitDialog').prop('request').accept(); + assert.strictEqual(wrapper.find('InitDialog').prop('error'), e); + }); + + it('passes a raised error to the dialog when raised during an asynchronous accept callback', async function() { + let breakWork = () => {}; + const workPromise = new Promise((_, reject) => { + breakWork = reject; + }); + const accept = sinon.stub().returns(workPromise); + + const request = dialogRequests.init({dirPath: '/not/home'}); + request.onProgressingAccept(accept); + + const wrapper = shallow(buildApp({request})); + const acceptPromise = wrapper.find('InitDialog').prop('request').accept('an-argument'); + assert.isTrue(wrapper.find('InitDialog').prop('inProgress')); + assert.isTrue(accept.calledWith('an-argument')); + + const e = new Error('ouch'); + breakWork(e); + await acceptPromise; + + wrapper.update(); + assert.strictEqual(wrapper.find('InitDialog').prop('error'), e); + }); + }); + + describe('specific dialogs', function() { + it('passes appropriate props to InitDialog', function() { + const accept = sinon.spy(); + const cancel = sinon.spy(); + const request = dialogRequests.init({dirPath: '/some/path'}); + request.onAccept(accept); + request.onCancel(cancel); + + const wrapper = shallow(buildApp({request})); + const dialog = wrapper.find('InitDialog'); + const req = dialog.prop('request'); + + req.accept(); + assert.isTrue(accept.called); + + req.cancel(); + assert.isTrue(cancel.called); + + assert.strictEqual(req.getParams().dirPath, '/some/path'); + }); + + it('passes appropriate props to CloneDialog'); + + it('passes appropriate props to CredentialDialog'); + + it('passes appropriate props to OpenIssueishDialog'); + + it('passes appropriate props to OpenCommitDialog'); + }); +}); diff --git a/test/views/init-dialog.test.js b/test/views/init-dialog.test.js index 974f7e5d8f..2081ddcdfa 100644 --- a/test/views/init-dialog.test.js +++ b/test/views/init-dialog.test.js @@ -1,69 +1,74 @@ import React from 'react'; -import {mount} from 'enzyme'; +import {shallow} from 'enzyme'; import path from 'path'; import InitDialog from '../../lib/views/init-dialog'; +import {dialogRequests} from '../../lib/controllers/dialogs-controller'; describe('InitDialog', function() { - let atomEnv, config, commandRegistry; - let app, wrapper, didAccept, didCancel; - - beforeEach(function() { - atomEnv = global.buildAtomEnvironment(); - config = atomEnv.config; - commandRegistry = atomEnv.commands; - sinon.stub(config, 'get').returns(path.join('home', 'me', 'codes')); - - didAccept = sinon.stub(); - didCancel = sinon.stub(); - - app = ( + function buildApp(overrides = {}) { + return ( ); - wrapper = mount(app); + } + + it('defaults the destination directory to the dirPath parameter', function() { + const wrapper = shallow(buildApp({ + request: dialogRequests.init({dirPath: path.join('/home/me/src')}), + })); + assert.strictEqual(wrapper.find('AtomTextEditor').prop('buffer').getText(), path.join('/home/me/src')); }); - afterEach(function() { - atomEnv.destroy(); + it('disables the initialize button when the project path is empty', function() { + const wrapper = shallow(buildApp({})); + + assert.isFalse(wrapper.find('button.icon-repo-create').prop('disabled')); + wrapper.find('AtomTextEditor').prop('buffer').setText(''); + assert.isTrue(wrapper.find('button.icon-repo-create').prop('disabled')); + wrapper.find('AtomTextEditor').prop('buffer').setText('/some/path'); + assert.isFalse(wrapper.find('button.icon-repo-create').prop('disabled')); }); - const setTextIn = function(selector, text) { - wrapper.find(selector).getDOMNode().getModel().setText(text); - wrapper.update(); - }; + it('calls the request accept method with the chosen path', function() { + const accept = sinon.spy(); + const request = dialogRequests.init({dirPath: __dirname}); + request.onAccept(accept); + + const wrapper = shallow(buildApp({request})); + wrapper.find('AtomTextEditor').prop('buffer').setText('/some/path'); + wrapper.find('button.icon-repo-create').simulate('click'); - it('defaults to your project home path', function() { - const text = wrapper.find('atom-text-editor').getDOMNode().getModel().getText(); - assert.equal(text, path.join('home', 'me', 'codes')); + assert.isTrue(accept.calledWith('/some/path')); }); - it('disables the initialize button with no project path', function() { - setTextIn('.github-ProjectPath atom-text-editor', ''); + it('displays a spinner while initialization is in progress', function() { + const wrapper = shallow(buildApp({inProgress: true})); + assert.isTrue(wrapper.find('AtomTextEditor').prop('readOnly')); assert.isTrue(wrapper.find('button.icon-repo-create').prop('disabled')); + assert.isTrue(wrapper.exists('span.loading-spinner-small')); }); - it('enables the initialize button when the project path is populated', function() { - setTextIn('.github-ProjectPath atom-text-editor', path.join('somewhere', 'else')); + it('displays an error when the accept callback has failed', function() { + const e = new Error('unfriendly message'); + e.userMessage = 'friendly message'; - assert.isFalse(wrapper.find('button.icon-repo-create').prop('disabled')); + const wrapper = shallow(buildApp({error: e})); + assert.strictEqual(wrapper.find('.error-messages li').text(), 'friendly message'); }); - it('calls the acceptance callback', function() { - setTextIn('.github-ProjectPath atom-text-editor', '/somewhere/directory/'); + it('calls the request cancel callback', function() { + const cancel = sinon.spy(); + const request = dialogRequests.init({dirPath: __dirname}); + request.onCancel(cancel); - wrapper.find('button.icon-repo-create').simulate('click'); - - assert.isTrue(didAccept.calledWith('/somewhere/directory/')); - }); + const wrapper = shallow(buildApp({request})); - it('calls the cancellation callback', function() { - wrapper.find('button.github-CancelButton').simulate('click'); - assert.isTrue(didCancel.called); + wrapper.find('button.github-Dialog-cancelButton').simulate('click'); + assert.isTrue(cancel.called); }); }); From d40987a5340f686f4c5a71ce3e6452a83108a7d2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 13:42:41 -0400 Subject: [PATCH 03/41] Rename "createRepositoryForProjectPath" to "initialize" --- lib/github-package.js | 6 +++--- lib/views/git-tab-view.js | 4 ++-- test/fixtures/props/git-tab-props.js | 4 ++-- test/github-package.test.js | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 74785d1bde..1d49816580 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -39,7 +39,7 @@ export default class GithubPackage { autobind( this, 'consumeStatusBar', 'createGitTimingsView', 'createIssueishPaneItemStub', 'createDockItemStub', - 'createFilePatchControllerStub', 'destroyGitTabItem', 'destroyGithubTabItem', 'createRepositoryForProjectPath', + 'createFilePatchControllerStub', 'destroyGitTabItem', 'destroyGithubTabItem', 'cloneRepositoryForProjectPath', 'getRepositoryForWorkdir', 'scheduleActiveContextUpdate', ); @@ -286,7 +286,7 @@ export default class GithubPackage { repository={this.getActiveRepository()} resolutionProgress={this.getActiveResolutionProgress()} statusBar={this.statusBar} - createRepositoryForProjectPath={this.createRepositoryForProjectPath} + initialize={this.initialize} cloneRepositoryForProjectPath={this.cloneRepositoryForProjectPath} switchboard={this.switchboard} startOpen={this.startOpen} @@ -425,7 +425,7 @@ export default class GithubPackage { } } - async createRepositoryForProjectPath(projectPath) { + initialize = async projectPath => { await fs.mkdirs(projectPath); const repository = this.contextPool.add(projectPath).getRepository(); diff --git a/lib/views/git-tab-view.js b/lib/views/git-tab-view.js index 1539ccb6eb..6c8d09b7e9 100644 --- a/lib/views/git-tab-view.js +++ b/lib/views/git-tab-view.js @@ -48,7 +48,7 @@ export default class GitTabView extends React.Component { project: PropTypes.object.isRequired, tooltips: PropTypes.object.isRequired, - initializeRepo: PropTypes.func.isRequired, + openInitializeDialog: PropTypes.func.isRequired, abortMerge: PropTypes.func.isRequired, commit: PropTypes.func.isRequired, undoLastCommit: PropTypes.func.isRequired, @@ -215,7 +215,7 @@ export default class GitTabView extends React.Component { initPath = projectPath; } } - this.props.initializeRepo(initPath); + this.props.openInitializeDialog(initPath); } getFocus(element) { diff --git a/test/fixtures/props/git-tab-props.js b/test/fixtures/props/git-tab-props.js index 22fb3cd7ad..fd3dfa2e65 100644 --- a/test/fixtures/props/git-tab-props.js +++ b/test/fixtures/props/git-tab-props.js @@ -24,7 +24,7 @@ export function gitTabItemProps(atomEnv, repository, overrides = {}) { undoLastDiscard: noop, discardWorkDirChangesForPaths: noop, openFiles: noop, - initializeRepo: noop, + openInitializeDialog: noop, ...overrides }; } @@ -85,7 +85,7 @@ export async function gitTabViewProps(atomEnv, repository, overrides = {}) { project: atomEnv.project, tooltips: atomEnv.tooltips, - initializeRepo: () => {}, + openInitializeDialog: () => {}, abortMerge: () => {}, commit: () => {}, undoLastCommit: () => {}, diff --git a/test/github-package.test.js b/test/github-package.test.js index df2225f129..6e037704fa 100644 --- a/test/github-package.test.js +++ b/test/github-package.test.js @@ -682,7 +682,7 @@ describe('GithubPackage', function() { }); }); - describe('createRepositoryForProjectPath()', function() { + describe('initialize', function() { it('creates and sets a repository for the given project path', async function() { const nonRepositoryPath = await getTempDir(); project.setPaths([nonRepositoryPath]); @@ -693,7 +693,7 @@ describe('GithubPackage', function() { assert.isTrue(githubPackage.getActiveRepository().isEmpty()); assert.isFalse(githubPackage.getActiveRepository().isAbsent()); - await githubPackage.createRepositoryForProjectPath(nonRepositoryPath); + await githubPackage.initialize(nonRepositoryPath); assert.isTrue(githubPackage.getActiveRepository().isPresent()); assert.strictEqual( From e768682b7dcb622973113fa33012acddfcbfd3bb Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 13:43:49 -0400 Subject: [PATCH 04/41] Replace InitDialog management with DialogsController logic --- lib/controllers/root-controller.js | 53 ++++++++------ test/controllers/root-controller.test.js | 90 +++++++++++------------- 2 files changed, 70 insertions(+), 73 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 44c624f7df..087bb85808 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -23,6 +23,7 @@ import GitTabItem from '../items/git-tab-item'; import GitHubTabItem from '../items/github-tab-item'; import ReviewsItem from '../items/reviews-item'; import CommentDecorationsContainer from '../containers/comment-decorations-container'; +import DialogsController, {dialogRequests} from './dialogs-controller'; import StatusBarTileController from './status-bar-tile-controller'; import RepositoryConflictController from './repository-conflict-controller'; import GitCacheView from '../views/git-cache-view'; @@ -36,6 +37,7 @@ import {incrementCounter, addEvent} from '../reporter-proxy'; export default class RootController extends React.Component { static propTypes = { + // Atom enviornment workspace: PropTypes.object.isRequired, commands: PropTypes.object.isRequired, deserializers: PropTypes.object.isRequired, @@ -45,18 +47,24 @@ export default class RootController extends React.Component { grammars: PropTypes.object.isRequired, config: PropTypes.object.isRequired, project: PropTypes.object.isRequired, - loginModel: PropTypes.object.isRequired, confirm: PropTypes.func.isRequired, - createRepositoryForProjectPath: PropTypes.func, - cloneRepositoryForProjectPath: PropTypes.func, + + // Models + loginModel: PropTypes.object.isRequired, + workdirContextPool: WorkdirContextPoolPropType.isRequired, repository: PropTypes.object.isRequired, resolutionProgress: PropTypes.object.isRequired, statusBar: PropTypes.object, switchboard: PropTypes.instanceOf(Switchboard), + pipelineManager: PropTypes.object, + + // Git actions + initialize: PropTypes.func.isRequired, + cloneRepositoryForProjectPath: PropTypes.func, + + // Control startOpen: PropTypes.bool, startRevealed: PropTypes.bool, - pipelineManager: PropTypes.object, - workdirContextPool: WorkdirContextPoolPropType.isRequired, } static defaultProps = { @@ -69,8 +77,8 @@ export default class RootController extends React.Component { super(props, context); autobind( this, - 'installReactDevTools', 'clearGithubToken', 'initializeRepo', 'showOpenIssueishDialog', - 'showWaterfallDiagnostics', 'showCacheDiagnostics', 'acceptClone', 'cancelClone', 'acceptInit', 'cancelInit', + 'installReactDevTools', 'clearGithubToken', 'showOpenIssueishDialog', + 'showWaterfallDiagnostics', 'showCacheDiagnostics', 'acceptClone', 'cancelClone', 'acceptOpenIssueish', 'cancelOpenIssueish', 'destroyFilePatchPaneItems', 'destroyEmptyFilePatchPaneItems', 'openCloneDialog', 'quietlySelectItem', 'viewUnstagedChangesForCurrentFile', 'viewStagedChangesForCurrentFile', 'openFiles', 'getUnsavedFiles', 'ensureNoUnsavedFiles', @@ -78,11 +86,9 @@ export default class RootController extends React.Component { ); this.state = { + dialogRequest: dialogRequests.null, cloneDialogActive: false, cloneDialogInProgress: false, - initDialogActive: false, - initDialogPath: null, - initDialogResolve: null, credentialDialogQuery: null, }; @@ -100,7 +106,7 @@ export default class RootController extends React.Component { this.props.repository.onPullError(this.gitTabTracker.ensureVisible), ); - this.props.commandRegistry.onDidDispatch(event => { + this.props.commands.onDidDispatch(event => { if (event.type && event.type.startsWith('github:') && event.detail && event.detail[0] && event.detail[0].contextCommand) { addEvent('context-menu-action', { @@ -132,7 +138,7 @@ export default class RootController extends React.Component { const devMode = global.atom && global.atom.inDevMode(); return ( - + {devMode && } @@ -143,6 +149,7 @@ export default class RootController extends React.Component { + - {this.renderInitDialog()} + {this.renderCloneDialog()} {this.renderCredentialDialog()} {this.renderOpenIssueishDialog()} @@ -331,7 +338,7 @@ export default class RootController extends React.Component { config={this.props.config} repository={this.props.repository} loginModel={this.props.loginModel} - initializeRepo={this.initializeRepo} + openInitializeDialog={this.openInitializeDialog} resolutionProgress={this.props.resolutionProgress} ensureGitTab={this.gitTabTracker.ensureVisible} openFiles={this.openFiles} @@ -566,18 +573,18 @@ export default class RootController extends React.Component { return this.props.loginModel.removeToken('https://api.github.com'); } - initializeRepo(initDialogPath) { - if (this.state.initDialogActive) { - return null; - } + cancelDialog = () => new Promise(resolve => this.setState({dialogRequest: dialogRequests.null})); - if (!initDialogPath) { - initDialogPath = this.props.repository.getWorkingDirectoryPath(); + openInitializeDialog = dirPath => { + if (!dirPath) { + dirPath = this.props.config.get('core.projectHome'); } - return new Promise(resolve => { - this.setState({initDialogActive: true, initDialogPath, initDialogResolve: resolve}); - }); + const dialogRequest = dialogRequests.init({dirPath}); + dialogRequest.onProgressingAccept(chosenPath => this.props.initialize(chosenPath)); + dialogRequest.onCancel(this.cancelDialog); + + return new Promise(resolve => this.setState({dialogRequest}, resolve)); } toggleCommitPreviewItem = () => { diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 626d08dc41..c503bbe470 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -10,11 +10,13 @@ import {multiFilePatchBuilder} from '../builder/patch'; import {GitError} from '../../lib/git-shell-out-strategy'; import Repository from '../../lib/models/repository'; import WorkdirContextPool from '../../lib/models/workdir-context-pool'; +import ResolutionProgress from '../../lib/models/conflicts/resolution-progress'; +import RefHolder from '../../lib/models/ref-holder'; import GithubLoginModel from '../../lib/models/github-login-model'; import {InMemoryStrategy} from '../../lib/shared/keytar-strategy'; +import {dialogRequests} from '../../lib/controllers/dialogs-controller'; import GitTabItem from '../../lib/items/git-tab-item'; import GitHubTabItem from '../../lib/items/github-tab-item'; -import ResolutionProgress from '../../lib/models/conflicts/resolution-progress'; import IssueishDetailItem from '../../lib/items/issueish-detail-item'; import CommitPreviewItem from '../../lib/items/commit-preview-item'; import CommitDetailItem from '../../lib/items/commit-detail-item'; @@ -25,7 +27,7 @@ import OpenCommitDialog from '../../lib/views/open-commit-dialog'; describe('RootController', function() { let atomEnv, app; - let workspace, commandRegistry, notificationManager, tooltips, config, confirm, deserializers, grammars, project; + let workspace, commands, notificationManager, tooltips, config, confirm, deserializers, grammars, project; let workdirContextPool; beforeEach(function() { @@ -58,12 +60,16 @@ describe('RootController', function() { confirm={confirm} project={project} keymaps={atomEnv.keymaps} + loginModel={loginModel} + workdirContextPool={workdirContextPool} repository={absentRepository} resolutionProgress={emptyResolutionProgress} + + initialize={() => {}} + startOpen={false} startRevealed={false} - workdirContextPool={workdirContextPool} /> ); }); @@ -255,73 +261,57 @@ describe('RootController', function() { }); }); - describe('initializeRepo', function() { - let createRepositoryForProjectPath, resolveInit, rejectInit; + describe('initialize', function() { + let initialize; beforeEach(function() { - createRepositoryForProjectPath = sinon.stub().returns(new Promise((resolve, reject) => { - resolveInit = resolve; - rejectInit = reject; - })); + initialize = sinon.stub().resolves(); + app = React.cloneElement(app, {initialize}); }); - it('renders the modal init panel', function() { - app = React.cloneElement(app, {createRepositoryForProjectPath}); - const wrapper = shallow(app); + it('requests the init dialog with a command', function() { + sinon.stub(config, 'get').returns(path.join('/home/me/src')); - wrapper.instance().initializeRepo(); - wrapper.update(); + const wrapper = shallow(app); - assert.lengthOf(wrapper.find('Panel').find({location: 'modal'}).find('InitDialog'), 1); + wrapper.find('Command[command="github:initialize"]').prop('callback')(); + const req = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req.identifier, 'init'); + assert.strictEqual(req.getParams().dirPath, path.join('/home/me/src')); }); - it('triggers the init callback on accept', function() { - app = React.cloneElement(app, {createRepositoryForProjectPath}); + it('requests the init dialog from the git tab', function() { const wrapper = shallow(app); + const gitTabWrapper = wrapper + .find('PaneItem[className="github-Git-root"]') + .renderProp('children')({itemHolder: new RefHolder()}); - wrapper.instance().initializeRepo(); - wrapper.update(); - const dialog = wrapper.find('InitDialog'); - dialog.prop('didAccept')('/a/path'); - resolveInit(); + gitTabWrapper.find('GitTabItem').prop('openInitializeDialog')(path.join('/some/workdir')); - assert.isTrue(createRepositoryForProjectPath.calledWith('/a/path')); + const req = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req.identifier, 'init'); + assert.strictEqual(req.getParams().dirPath, path.join('/some/workdir')); }); - it('dismisses the init callback on cancel', function() { - app = React.cloneElement(app, {createRepositoryForProjectPath}); + it('triggers the initialize callback on accept', async function() { const wrapper = shallow(app); + wrapper.find('Command[command="github:initialize"]').prop('callback')(); - wrapper.instance().initializeRepo(); - wrapper.update(); - const dialog = wrapper.find('InitDialog'); - dialog.prop('didCancel')(); - - wrapper.update(); - assert.isFalse(wrapper.find('InitDialog').exists()); + const req = wrapper.find('DialogsController').prop('request'); + await req.accept(path.join('/home/me/src')); + assert.isTrue(initialize.calledWith(path.join('/home/me/src'))); }); - it('creates a notification if the init fails', async function() { - sinon.stub(notificationManager, 'addError'); - - app = React.cloneElement(app, {createRepositoryForProjectPath}); + it('dismisses the dialog with its cancel callback', function() { const wrapper = shallow(app); + wrapper.find('Command[command="github:initialize"]').prop('callback')(); - wrapper.instance().initializeRepo(); - wrapper.update(); - const dialog = wrapper.find('InitDialog'); - const acceptPromise = dialog.prop('didAccept')('/a/path'); - const err = new GitError('git init exited with status 1'); - err.stdErr = 'this is stderr'; - rejectInit(err); - await acceptPromise; + const req0 = wrapper.find('DialogsController').prop('request'); + assert.notStrictEqual(req0, dialogRequests.null); + req0.cancel(); - wrapper.update(); - assert.isFalse(wrapper.find('InitDialog').exists()); - assert.isTrue(notificationManager.addError.calledWith( - 'Unable to initialize git repository in /a/path', - sinon.match({detail: sinon.match(/this is stderr/)}), - )); + const req1 = wrapper.update().find('DialogsController').prop('request'); + assert.strictEqual(req1, dialogRequests.null); }); }); From e3ac43afd58304831ed116804dbda7c9d0760514 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 13:43:56 -0400 Subject: [PATCH 05/41] Missed a prop --- lib/controllers/git-tab-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/controllers/git-tab-controller.js b/lib/controllers/git-tab-controller.js index 44ab1e8066..b75afe868d 100644 --- a/lib/controllers/git-tab-controller.js +++ b/lib/controllers/git-tab-controller.js @@ -115,7 +115,7 @@ export default class GitTabController extends React.Component { confirm={this.props.confirm} config={this.props.config} - initializeRepo={this.props.initializeRepo} + openInitializeDialog={this.props.openInitializeDialog} openFiles={this.props.openFiles} discardWorkDirChangesForPaths={this.props.discardWorkDirChangesForPaths} undoLastDiscard={this.props.undoLastDiscard} From 569a9bc89a5f55d1377b7bf2a0e4cbee4cb884ba Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 14:39:33 -0400 Subject: [PATCH 06/41] Move command registration into InitDialog --- lib/views/init-dialog.js | 9 +++++++++ test/views/init-dialog.test.js | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/views/init-dialog.js b/lib/views/init-dialog.js index f0e901adac..040c741f81 100644 --- a/lib/views/init-dialog.js +++ b/lib/views/init-dialog.js @@ -3,9 +3,11 @@ import PropTypes from 'prop-types'; import {TextBuffer} from 'atom'; import AtomTextEditor from '../atom/atom-text-editor'; +import Commands, {Command} from '../atom/commands'; export default class InitDialog extends React.Component { static propTypes = { + // Model request: PropTypes.shape({ getParams: PropTypes.func.isRequired, accept: PropTypes.func.isRequired, @@ -13,6 +15,9 @@ export default class InitDialog extends React.Component { }).isRequired, inProgress: PropTypes.bool, error: PropTypes.instanceOf(Error), + + // Atom environment + commands: PropTypes.object.isRequired, } constructor(props) { @@ -32,6 +37,10 @@ export default class InitDialog extends React.Component { render() { return (
+ + + +
+
+ {this.props.error && ( +
    +
  • {this.props.error.userMessage || this.props.error.message}
    • +
+ )} + +
); } - confirm() { + accept = () => { + const request = this.props.request; + const params = request.getParams(); + const payload = {password: this.state.password}; - if (this.props.includeUsername) { + if (params.includeUsername) { payload.username = this.state.username; } - if (this.props.includeRemember) { + if (params.includeRemember) { payload.remember = this.state.remember; } - this.props.onSubmit(payload); + return request.accept(payload); } - cancel() { - this.props.onCancel(); - } - - onUsernameChange(e) { - this.setState({username: e.target.value}); - } - - onPasswordChange(e) { - this.setState({password: e.target.value}); - } + didChangeUsername = e => this.setState({username: e.target.value}); - onRememberChange(e) { - this.setState({remember: e.target.checked}); - } + didChangePassword = e => this.setState({password: e.target.value}); - focusFirstInput() { - (this.usernameInput || this.passwordInput).focus(); - } + didChangeRemember = e => this.setState({remember: e.target.checked}); - toggleShowPassword() { - this.setState({showPassword: !this.state.showPassword}); - } + toggleShowPassword = () => this.setState({showPassword: !this.state.showPassword}); } diff --git a/test/views/credential-dialog.test.js b/test/views/credential-dialog.test.js index 84c638a4d9..0efc4c3c95 100644 --- a/test/views/credential-dialog.test.js +++ b/test/views/credential-dialog.test.js @@ -1,114 +1,115 @@ import React from 'react'; -import {mount} from 'enzyme'; +import {shallow} from 'enzyme'; import CredentialDialog from '../../lib/views/credential-dialog'; +import {dialogRequests} from '../../lib/controllers/dialogs-controller'; describe('CredentialDialog', function() { let atomEnv; - let app, wrapper, didSubmit, didCancel; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); - - didSubmit = sinon.stub(); - didCancel = sinon.stub(); - - app = ( - - ); }); afterEach(function() { atomEnv.destroy(); }); - const setTextIn = function(selector, text) { - wrapper.find(selector).simulate('change', {target: {value: text}}); - }; + function buildApp(overrides = {}) { + return ( + + ); + } - describe('confirm', function() { + describe('accept', function() { it('reports the current username and password', function() { - wrapper = mount(app); - - setTextIn('.github-CredentialDialog-Username', 'someone'); - setTextIn('.github-CredentialDialog-Password', 'letmein'); + const accept = sinon.spy(); + const request = dialogRequests.credential({includeUsername: true}); + request.onAccept(accept); + const wrapper = shallow(buildApp({request})); + wrapper.find('.github-Credential-username').simulate('change', {target: {value: 'someone'}}); + wrapper.find('.github-Credential-password').simulate('change', {target: {value: 'letmein'}}); wrapper.find('.btn-primary').simulate('click'); - assert.deepEqual(didSubmit.firstCall.args[0], { + assert.isTrue(accept.calledWith({ username: 'someone', password: 'letmein', - }); + })); }); it('omits the username if includeUsername is false', function() { - wrapper = mount(React.cloneElement(app, {includeUsername: false})); - - assert.isFalse(wrapper.find('.github-CredentialDialog-Username').exists()); - setTextIn('.github-CredentialDialog-Password', 'twowordsuppercase'); + const accept = sinon.spy(); + const request = dialogRequests.credential({includeUsername: false}); + request.onAccept(accept); + const wrapper = shallow(buildApp({request})); + assert.isFalse(wrapper.find('.github-Credential-username').exists()); + wrapper.find('.github-Credential-password').simulate('change', {target: {value: 'twowordsuppercase'}}); wrapper.find('.btn-primary').simulate('click'); - assert.deepEqual(didSubmit.firstCall.args[0], { + assert.isTrue(accept.calledWith({ password: 'twowordsuppercase', - }); + })); }); it('includes a "remember me" checkbox', function() { - wrapper = mount(React.cloneElement(app, {includeRemember: true})); + const accept = sinon.spy(); + const request = dialogRequests.credential({includeUsername: true, includeRemember: true}); + request.onAccept(accept); + const wrapper = shallow(buildApp({request})); - const rememberBox = wrapper.find('.github-CredentialDialog-remember'); + const rememberBox = wrapper.find('.github-Credential-remember'); assert.isTrue(rememberBox.exists()); rememberBox.simulate('change', {target: {checked: true}}); - setTextIn('.github-CredentialDialog-Username', 'someone'); - setTextIn('.github-CredentialDialog-Password', 'letmein'); - + wrapper.find('.github-Credential-username').simulate('change', {target: {value: 'someone'}}); + wrapper.find('.github-Credential-password').simulate('change', {target: {value: 'letmein'}}); wrapper.find('.btn-primary').simulate('click'); - assert.deepEqual(didSubmit.firstCall.args[0], { + assert.isTrue(accept.calledWith({ username: 'someone', password: 'letmein', remember: true, - }); + })); }); it('omits the "remember me" checkbox', function() { - wrapper = mount(app); - assert.isFalse(wrapper.find('.github-CredentialDialog-remember').exists()); + const request = dialogRequests.credential({includeRemember: false}); + const wrapper = shallow(buildApp({request})); + assert.isFalse(wrapper.exists('.github-Credential-remember')); }); }); it('calls the cancel callback', function() { - wrapper = mount(app); - wrapper.find('.github-CancelButton').simulate('click'); - assert.isTrue(didCancel.called); + const cancel = sinon.spy(); + const request = dialogRequests.credential(); + request.onCancel(cancel); + const wrapper = shallow(buildApp({request})); + + wrapper.find('.github-Dialog-cancelButton').simulate('click'); + assert.isTrue(cancel.called); }); describe('show password', function() { it('sets the passwords input type to "text" on the first click', function() { - wrapper = mount(app); + const wrapper = shallow(buildApp()); + wrapper.find('.github-Credential-visibility').simulate('click'); - wrapper.find('.github-DialogLabelButton').simulate('click'); - - const passwordInput = wrapper.find('.github-CredentialDialog-Password'); - assert.equal(passwordInput.prop('type'), 'text'); + const passwordInput = wrapper.find('.github-Credential-password'); + assert.strictEqual(passwordInput.prop('type'), 'text'); }); it('sets the passwords input type back to "password" on the second click', function() { - wrapper = mount(app); - - wrapper.find('.github-DialogLabelButton').simulate('click').simulate('click'); + const wrapper = shallow(buildApp()); + wrapper.find('.github-Credential-visibility').simulate('click').simulate('click'); - const passwordInput = wrapper.find('.github-CredentialDialog-Password'); - assert.equal(passwordInput.prop('type'), 'password'); + const passwordInput = wrapper.find('.github-Credential-password'); + assert.strictEqual(passwordInput.prop('type'), 'password'); }); }); }); From 43929f6d6fc7b879628a5af861e7b7a98a47a6a7 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 17 Jul 2019 15:47:45 -0400 Subject: [PATCH 12/41] Remove CredentialDialog stuff from RootController --- lib/controllers/root-controller.js | 41 ++++++------------------ lib/github-package.js | 2 +- test/controllers/root-controller.test.js | 41 +++++++++++------------- 3 files changed, 30 insertions(+), 54 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 851a6f8e49..1066b48d5e 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -11,7 +11,6 @@ import Panel from '../atom/panel'; import PaneItem from '../atom/pane-item'; import OpenIssueishDialog from '../views/open-issueish-dialog'; import OpenCommitDialog from '../views/open-commit-dialog'; -import CredentialDialog from '../views/credential-dialog'; import Commands, {Command} from '../atom/commands'; import ChangedFileItem from '../items/changed-file-item'; import IssueishDetailItem from '../items/issueish-detail-item'; @@ -85,7 +84,6 @@ export default class RootController extends React.Component { this.state = { dialogRequest: dialogRequests.null, - credentialDialogQuery: null, }; this.gitTabTracker = new TabTracker('git', { @@ -198,7 +196,6 @@ export default class RootController extends React.Component { workspace={this.props.workspace} request={this.state.dialogRequest} /> - {this.renderCredentialDialog()} {this.renderOpenIssueishDialog()} {this.renderOpenCommitDialog()} @@ -238,18 +235,6 @@ export default class RootController extends React.Component { ); } - renderCredentialDialog() { - if (this.state.credentialDialogQuery === null) { - return null; - } - - return ( - - - - ); - } - renderCommentDecorations() { if (!this.props.repository) { return null; @@ -559,6 +544,16 @@ export default class RootController extends React.Component { return new Promise(resolve => this.setState({dialogRequest}, resolve)); } + openCredentialsDialog = query => { + return new Promise((resolve, reject) => { + const dialogRequest = dialogRequests.credential(query); + dialogRequest.onAccept(resolve); + dialogRequest.onCancel(reject); + + this.setState({dialogRequest}); + }); + } + toggleCommitPreviewItem = () => { const workdir = this.props.repository.getWorkingDirectoryPath(); return this.props.workspace.toggle(CommitPreviewItem.buildURI(workdir)); @@ -896,22 +891,6 @@ export default class RootController extends React.Component { }); }); } - - /* - * Display the credential entry dialog. Return a Promise that will resolve with the provided credentials on accept - * or reject on cancel. - */ - promptForCredentials(query) { - return new Promise((resolve, reject) => { - this.setState({ - credentialDialogQuery: { - ...query, - onSubmit: response => this.setState({credentialDialogQuery: null}, () => resolve(response)), - onCancel: () => this.setState({credentialDialogQuery: null}, reject), - }, - }); - }); - } } class TabTracker { diff --git a/lib/github-package.js b/lib/github-package.js index 327525471a..86046495d0 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -74,7 +74,7 @@ export default class GithubPackage { this.contextPool = new WorkdirContextPool({ window, workspace, - promptCallback: query => this.controller.promptForCredentials(query), + promptCallback: query => this.controller.openCredentialsDialog(query), pipelineManager: this.pipelineManager, }); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 64afe9d63e..cbdea98a07 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -486,52 +486,49 @@ describe('RootController', function() { }); }); - describe('promptForCredentials()', function() { - let wrapper; - - beforeEach(function() { - wrapper = shallow(app); - }); - + describe('openCredentialsDialog()', function() { it('renders the modal credentials dialog', function() { - wrapper.instance().promptForCredentials({ + const wrapper = shallow(app); + + wrapper.instance().openCredentialsDialog({ prompt: 'Password plz', includeUsername: true, }); wrapper.update(); - const dialog = wrapper.find('Panel').find({location: 'modal'}).find('CredentialDialog'); - assert.isTrue(dialog.exists()); - assert.equal(dialog.prop('prompt'), 'Password plz'); - assert.isTrue(dialog.prop('includeUsername')); + const req = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req.identifier, 'credential'); + assert.deepEqual(req.getParams(), { + prompt: 'Password plz', + includeUsername: true, + includeRemember: false, + }); }); it('resolves the promise with credentials on accept', async function() { - const credentialPromise = wrapper.instance().promptForCredentials({ + const wrapper = shallow(app); + const credentialPromise = wrapper.instance().openCredentialsDialog({ prompt: 'Speak "friend" and enter', includeUsername: false, }); wrapper.update(); - wrapper.find('CredentialDialog').prop('onSubmit')({password: 'friend'}); + const req = wrapper.find('DialogsController').prop('request'); + req.accept({password: 'friend'}); assert.deepEqual(await credentialPromise, {password: 'friend'}); - - wrapper.update(); - assert.isFalse(wrapper.find('CredentialDialog').exists()); }); it('rejects the promise on cancel', async function() { - const credentialPromise = wrapper.instance().promptForCredentials({ + const wrapper = shallow(app); + const credentialPromise = wrapper.instance().openCredentialsDialog({ prompt: 'Enter the square root of 1244313452349528345', includeUsername: false, }); wrapper.update(); - wrapper.find('CredentialDialog').prop('onCancel')(); + const req = wrapper.find('DialogsController').prop('request'); + req.cancel(new Error('cancelled')); await assert.isRejected(credentialPromise); - - wrapper.update(); - assert.isFalse(wrapper.find('CredentialDialog').exists()); }); }); From d353db8986ce64855344ee8a800d9bd906b41e0d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 08:45:44 -0400 Subject: [PATCH 13/41] Use path.join to platform-normalize test path --- test/views/clone-dialog.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/views/clone-dialog.test.js b/test/views/clone-dialog.test.js index 6dbf2f7fa4..c94d45c206 100644 --- a/test/views/clone-dialog.test.js +++ b/test/views/clone-dialog.test.js @@ -99,7 +99,7 @@ describe('CloneDialog', function() { wrapper.find('.github-Clone-sourceURL').prop('buffer').setText('git@github.com:atom/github.git'); wrapper.find('button.icon-repo-clone').simulate('click'); - assert.isTrue(accept.calledWith('git@github.com:atom/github.git', '/some/where')); + assert.isTrue(accept.calledWith('git@github.com:atom/github.git', path.join('/some/where'))); }); it('calls the cancellation callback', function() { From 90ceabd2ed0678ef2e8a7f42236d5164c1b22932 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 09:38:20 -0400 Subject: [PATCH 14/41] Rework OpenIssueishDialog --- lib/controllers/dialogs-controller.js | 4 + lib/views/open-issueish-dialog.js | 158 ++++++++++-------------- test/views/open-issueish-dialog.test.js | 137 ++++++++++---------- 3 files changed, 147 insertions(+), 152 deletions(-) diff --git a/lib/controllers/dialogs-controller.js b/lib/controllers/dialogs-controller.js index 3a6a56d12c..c3096d6928 100644 --- a/lib/controllers/dialogs-controller.js +++ b/lib/controllers/dialogs-controller.js @@ -154,4 +154,8 @@ export const dialogRequests = { ...opts, }); }, + + issueish() { + return new DialogRequest('issueish'); + }, }; diff --git a/lib/views/open-issueish-dialog.js b/lib/views/open-issueish-dialog.js index 069dbbf601..6eb8d2cad2 100644 --- a/lib/views/open-issueish-dialog.js +++ b/lib/views/open-issueish-dialog.js @@ -1,125 +1,88 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {CompositeDisposable} from 'event-kit'; +import {TextBuffer} from 'atom'; import Commands, {Command} from '../atom/commands'; -import {autobind} from '../helpers'; +import {AtomTextEditor} from '../atom/atom-text-editor'; +import IssueishDetailItem from '../items/issueish-detail-item'; +import {addEvent} from '../reporter-proxy'; -const ISSUEISH_URL_REGEX = /^(?:https?:\/\/)?github.com\/([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)/; +const ISSUEISH_URL_REGEX = /^(?:https?:\/\/)?(github.com)\/([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)/; export default class OpenIssueishDialog extends React.Component { static propTypes = { + // Model + request: PropTypes.shape({ + getParams: PropTypes.func.isRequired, + accept: PropTypes.func.isRequired, + cancel: PropTypes.func.isRequired, + }).isRequired, + error: PropTypes.instanceOf(Error), + + // Atom environment commands: PropTypes.object.isRequired, - didAccept: PropTypes.func, - didCancel: PropTypes.func, } - static defaultProps = { - didAccept: () => {}, - didCancel: () => {}, - } + constructor(props) { + super(props); - constructor(props, context) { - super(props, context); - autobind(this, 'accept', 'cancel', 'editorRefs', 'didChangeIssueishUrl'); + this.url = new TextBuffer(); this.state = { - cloneDisabled: false, + openEnabled: false, }; - this.subs = new CompositeDisposable(); + this.sub = this.url.onDidChange(this.didChangeURL); } - componentDidMount() { - if (this.issueishUrlElement) { - setTimeout(() => this.issueishUrlElement.focus()); - } + componentWillUnmount() { + this.sub.dispose(); } render() { - return this.renderDialog(); - } - - renderDialog() { return (
- - + + -
-
); } - accept() { - if (this.getIssueishUrl().length === 0) { - return; - } - - const parsed = this.parseUrl(); - if (!parsed) { - this.setState({ - error: 'That is not a valid issue or pull request URL.', - }); - return; + accept = () => { + const issueishURL = this.url.getText(); + if (issueishURL.length === 0) { + return Promise.resolve(); } - const {repoOwner, repoName, issueishNumber} = parsed; - this.props.didAccept({repoOwner, repoName, issueishNumber}); - } - - cancel() { - this.props.didCancel(); - } - - editorRefs(baseName) { - const elementName = `${baseName}Element`; - const modelName = `${baseName}Editor`; - const subName = `${baseName}Subs`; - const changeMethodName = `didChange${baseName[0].toUpperCase()}${baseName.substring(1)}`; - - return element => { - if (!element) { - return; - } - - this[elementName] = element; - const editor = element.getModel(); - if (this[modelName] !== editor) { - this[modelName] = editor; - - if (this[subName]) { - this[subName].dispose(); - this.subs.remove(this[subName]); - } - - this[subName] = editor.onDidChange(this[changeMethodName]); - this.subs.add(this[subName]); - } - }; - } - - didChangeIssueishUrl() { - this.setState({error: null}); + return this.props.request.accept(issueishURL); } parseUrl() { @@ -132,7 +95,22 @@ export default class OpenIssueishDialog extends React.Component { return {repoOwner, repoName, issueishNumber}; } - getIssueishUrl() { - return this.issueishUrlEditor ? this.issueishUrlEditor.getText() : ''; + didChangeURL = () => { + const enabled = !this.url.isEmpty(); + if (this.state.openEnabled !== enabled) { + this.setState({openEnabled: enabled}); + } + } +} + +export async function openIssueishItem(issueishURL, {workspace, workdir}) { + const matches = ISSUEISH_URL_REGEX.exec(issueishURL); + if (!matches) { + throw new Error('Not a valid issue or pull request URL'); } + const [, host, owner, repo, number] = matches; + const uri = IssueishDetailItem.buildURI({host, owner, repo, number, workdir}); + const item = await workspace.open(uri, {searchAllPanes: true}); + addEvent('open-issueish-in-pane', {package: 'github', from: 'dialog'}); + return item; } diff --git a/test/views/open-issueish-dialog.test.js b/test/views/open-issueish-dialog.test.js index d8c4c9c6e6..691d4b9938 100644 --- a/test/views/open-issueish-dialog.test.js +++ b/test/views/open-issueish-dialog.test.js @@ -1,95 +1,108 @@ import React from 'react'; -import {mount} from 'enzyme'; +import {shallow} from 'enzyme'; -import OpenIssueishDialog from '../../lib/views/open-issueish-dialog'; +import OpenIssueishDialog, {openIssueishItem} from '../../lib/views/open-issueish-dialog'; +import IssueishDetailItem from '../../lib/items/issueish-detail-item'; +import {dialogRequests} from '../../lib/controllers/dialogs-controller'; +import * as reporterProxy from '../../lib/reporter-proxy'; describe('OpenIssueishDialog', function() { - let atomEnv, commands; - let app, wrapper, didAccept, didCancel; + let atomEnv; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); - commands = atomEnv.commands; - - didAccept = sinon.stub(); - didCancel = sinon.stub(); - - app = ( - - ); - wrapper = mount(app); + sinon.stub(reporterProxy, 'addEvent').returns(); }); afterEach(function() { atomEnv.destroy(); }); - const setTextIn = function(selector, text) { - wrapper.find(selector).getDOMNode().getModel().setText(text); - }; - - describe('entering a issueish url', function() { - it("updates the issue url automatically if it hasn't been modified", function() { - setTextIn('.github-IssueishUrl atom-text-editor', 'https://github.com/atom/github/pull/1807'); + function buildApp(overrides = {}) { + const request = dialogRequests.issueish(); - assert.equal(wrapper.instance().getIssueishUrl(), 'https://github.com/atom/github/pull/1807'); - }); - - it('does update the issue url if it was modified automatically', function() { - setTextIn('.github-IssueishUrl atom-text-editor', 'https://github.com/atom/github/pull/1807'); - assert.equal(wrapper.instance().getIssueishUrl(), 'https://github.com/atom/github/pull/1807'); - - setTextIn('.github-IssueishUrl atom-text-editor', 'https://github.com/atom/github/issues/1655'); - assert.equal(wrapper.instance().getIssueishUrl(), 'https://github.com/atom/github/issues/1655'); - }); - }); + return ( + + ); + } describe('open button enablement', function() { it('disables the open button with no issue url', function() { - setTextIn('.github-IssueishUrl atom-text-editor', ''); - wrapper.update(); + const wrapper = shallow(buildApp()); - assert.isTrue(wrapper.find('button.icon-git-pull-request').prop('disabled')); + wrapper.find('.github-OpenIssueish-url').prop('buffer').setText(''); + assert.isTrue(wrapper.update().find('button.icon-git-pull-request').prop('disabled')); }); it('enables the open button when issue url box is populated', function() { - setTextIn('.github-IssueishUrl atom-text-editor', 'https://github.com/atom/github/pull/1807'); - wrapper.update(); + const wrapper = shallow(buildApp()); + wrapper.find('.github-OpenIssueish-url').prop('buffer').setText('https://github.com/atom/github/pull/1807'); - assert.isFalse(wrapper.find('button.icon-git-pull-request').prop('disabled')); + assert.isFalse(wrapper.update().find('button.icon-git-pull-request').prop('disabled')); }); }); - describe('parseUrl', function() { - it('returns an object with repo owner, repo name, and issueish number', function() { - setTextIn('.github-IssueishUrl atom-text-editor', 'https://github.com/atom/github/pull/1807'); + it('calls the acceptance callback with the entered URL', function() { + const accept = sinon.spy(); + const request = dialogRequests.issueish(); + request.onAccept(accept); + const wrapper = shallow(buildApp({request})); + wrapper.find('.github-OpenIssueish-url').prop('buffer').setText('https://github.com/atom/github/pull/1807'); + wrapper.find('button.icon-git-pull-request').simulate('click'); - assert.deepEqual(wrapper.instance().parseUrl(), { - repoOwner: 'atom', - repoName: 'github', - issueishNumber: '1807', - }); - }); + assert.isTrue(accept.calledWith('https://github.com/atom/github/pull/1807')); }); - it('calls the acceptance callback', function() { - setTextIn('.github-IssueishUrl atom-text-editor', 'https://github.com/atom/github/pull/1807'); - - wrapper.find('button.icon-git-pull-request').simulate('click'); + it('calls the cancellation callback', function() { + const cancel = sinon.spy(); + const request = dialogRequests.issueish(); + request.onCancel(cancel); + const wrapper = shallow(buildApp({request})); + wrapper.find('button.github-Dialog-cancelButton').simulate('click'); - assert.isTrue(didAccept.calledWith({ - repoOwner: 'atom', - repoName: 'github', - issueishNumber: '1807', - })); + assert.isTrue(cancel.called); }); - it('calls the cancellation callback', function() { - wrapper.find('button.github-CancelButton').simulate('click'); - assert.isTrue(didCancel.called); + describe('openIssueishItem', function() { + it('opens an item for a valid issue URL', async function() { + sinon.stub(atomEnv.workspace, 'open').resolves('item'); + assert.strictEqual( + await openIssueishItem('https://github.com/atom/github/issues/2203', { + workspace: atomEnv.workspace, workdir: __dirname, + }), + 'item', + ); + assert.isTrue(atomEnv.workspace.open.calledWith( + IssueishDetailItem.buildURI({ + host: 'github.com', owner: 'atom', repo: 'github', number: 2203, workdir: __dirname, + }), + )); + }); + + it('opens an item for a valid PR URL', async function() { + sinon.stub(atomEnv.workspace, 'open').resolves('item'); + assert.strictEqual( + await openIssueishItem('https://github.com/smashwilson/az-coordinator/pull/10', { + workspace: atomEnv.workspace, workdir: __dirname, + }), + 'item', + ); + assert.isTrue(atomEnv.workspace.open.calledWith( + IssueishDetailItem.buildURI({ + host: 'github.com', owner: 'smashwilson', repo: 'az-coordinator', number: 10, workdir: __dirname, + }), + )); + }); + + it('rejects with an error for an invalid URL', async function() { + await assert.isRejected( + openIssueishItem('https://azurefire.net/not-an-issue', {workspace: atomEnv.workspace, workdir: __dirname}), + 'Not a valid issue or pull request URL', + ); + }); }); }); From c3c18da3523c4be755432c74c8c934be3a814e63 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 09:43:55 -0400 Subject: [PATCH 15/41] Include OpenIssueishDialog in DialogsController --- lib/controllers/dialogs-controller.js | 14 ++++-- test/controllers/dialogs-controller.test.js | 48 ++++++++++++++++++++- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/lib/controllers/dialogs-controller.js b/lib/controllers/dialogs-controller.js index c3096d6928..d7e29b18d7 100644 --- a/lib/controllers/dialogs-controller.js +++ b/lib/controllers/dialogs-controller.js @@ -5,6 +5,7 @@ import Panel from '../atom/panel'; import InitDialog from '../views/init-dialog'; import CloneDialog from '../views/clone-dialog'; import CredentialDialog from '../views/credential-dialog'; +import IssueishDialog from '../views/open-issueish-dialog'; export default class DialogsController extends React.Component { static propTypes = { @@ -28,6 +29,7 @@ export default class DialogsController extends React.Component { init: this.renderInitDialog, clone: this.renderCloneDialog, credential: this.renderCredentialDialog, + issueish: this.renderIssueishDialog, }; this.state = { @@ -52,11 +54,17 @@ export default class DialogsController extends React.Component { ) - renderCredentialDialog = () => { + renderCredentialDialog = () => ( - ; - } + + ) + + renderIssueishDialog = () => ( + + + + ) getCommonProps() { const {request} = this.props; diff --git a/test/controllers/dialogs-controller.test.js b/test/controllers/dialogs-controller.test.js index b0046eb579..7f7fba5b9d 100644 --- a/test/controllers/dialogs-controller.test.js +++ b/test/controllers/dialogs-controller.test.js @@ -151,9 +151,53 @@ describe('DialogsController', function() { assert.strictEqual(req.getParams().destPath, '/some/path'); }); - it('passes appropriate props to CredentialDialog'); + it('passes appropriate props to CredentialDialog', function() { + const accept = sinon.spy(); + const cancel = sinon.spy(); + const request = dialogRequests.credential({ + prompt: 'who the hell are you', + includeUsername: true, + includeRemember: true, + }); + request.onAccept(accept); + request.onCancel(cancel); + + const wrapper = shallow(buildApp({request})); + const dialog = wrapper.find('CredentialDialog'); + assert.strictEqual(dialog.prop('commands'), atomEnv.commands); + + const req = dialog.prop('request'); + + req.accept({username: 'me', password: 'whatever'}); + assert.isTrue(accept.calledWith({username: 'me', password: 'whatever'})); + + req.cancel(); + assert.isTrue(cancel.called); + + assert.strictEqual(req.getParams().prompt, 'who the hell are you'); + assert.isTrue(req.getParams().includeUsername); + assert.isTrue(req.getParams().includeRemember); + }); + + it('passes appropriate props to OpenIssueishDialog', function() { + const accept = sinon.spy(); + const cancel = sinon.spy(); + const request = dialogRequests.issueish(); + request.onAccept(accept); + request.onCancel(cancel); - it('passes appropriate props to OpenIssueishDialog'); + const wrapper = shallow(buildApp({request})); + const dialog = wrapper.find('OpenIssueishDialog'); + assert.strictEqual(dialog.prop('commands'), atomEnv.commands); + + const req = dialog.prop('request'); + + req.accept('https://github.com/atom/github/issue/123'); + assert.isTrue(accept.calledWith('https://github.com/atom/github/issue/123')); + + req.cancel(); + assert.isTrue(cancel.called); + }); it('passes appropriate props to OpenCommitDialog'); }); From 8b5572db8ab7ba57e27b0f924d92cfd868882be3 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 10:14:19 -0400 Subject: [PATCH 16/41] Use DialogsController to open Issueish dialog --- lib/controllers/root-controller.js | 55 +++------- test/controllers/root-controller.test.js | 130 ++++++++++------------- 2 files changed, 71 insertions(+), 114 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 1066b48d5e..f96506f317 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -9,7 +9,7 @@ import {CompositeDisposable} from 'event-kit'; import StatusBar from '../atom/status-bar'; import Panel from '../atom/panel'; import PaneItem from '../atom/pane-item'; -import OpenIssueishDialog from '../views/open-issueish-dialog'; +import {openIssueishItem} from '../views/open-issueish-dialog'; import OpenCommitDialog from '../views/open-commit-dialog'; import Commands, {Command} from '../atom/commands'; import ChangedFileItem from '../items/changed-file-item'; @@ -74,10 +74,10 @@ export default class RootController extends React.Component { super(props, context); autobind( this, - 'installReactDevTools', 'clearGithubToken', 'showOpenIssueishDialog', + 'installReactDevTools', 'clearGithubToken', 'showWaterfallDiagnostics', 'showCacheDiagnostics', - 'acceptOpenIssueish', 'cancelOpenIssueish', 'destroyFilePatchPaneItems', - 'destroyEmptyFilePatchPaneItems', 'quietlySelectItem', 'viewUnstagedChangesForCurrentFile', + 'destroyFilePatchPaneItems', 'destroyEmptyFilePatchPaneItems', + 'quietlySelectItem', 'viewUnstagedChangesForCurrentFile', 'viewStagedChangesForCurrentFile', 'openFiles', 'getUnsavedFiles', 'ensureNoUnsavedFiles', 'discardWorkDirChangesForPaths', 'discardLines', 'undoLastDiscard', 'refreshResolutionProgress', ); @@ -138,13 +138,13 @@ export default class RootController extends React.Component { - + - {this.renderOpenIssueishDialog()} {this.renderOpenCommitDialog()} ); } - renderOpenIssueishDialog() { - if (!this.state.openIssueishDialogActive) { - return null; - } - - return ( - - - - ); - } - renderOpenCommitDialog() { if (!this.state.openCommitDialogActive) { return null; @@ -559,8 +542,15 @@ export default class RootController extends React.Component { return this.props.workspace.toggle(CommitPreviewItem.buildURI(workdir)); } - showOpenIssueishDialog() { - this.setState({openIssueishDialogActive: true}); + openIssueishDialog = () => { + const dialogRequest = dialogRequests.issueish(); + dialogRequest.onAccept(url => openIssueishItem(url, { + workspace: this.props.workspace, + workdir: this.props.repository.getWorkingDirectoryPath(), + })); + dialogRequest.onCancel(this.cancelDialog); + + return new Promise(resolve => this.setState({dialogRequest}, resolve)); } showOpenCommitDialog = () => { @@ -575,23 +565,6 @@ export default class RootController extends React.Component { this.props.workspace.open(GitCacheView.buildURI()); } - acceptOpenIssueish({repoOwner, repoName, issueishNumber}) { - const uri = IssueishDetailItem.buildURI({ - host: 'github.com', - owner: repoOwner, - repo: repoName, - number: issueishNumber, - }); - this.setState({openIssueishDialogActive: false}); - this.props.workspace.open(uri).then(() => { - addEvent('open-issueish-in-pane', {package: 'github', from: 'dialog'}); - }); - } - - cancelOpenIssueish() { - this.setState({openIssueishDialogActive: false}); - } - isValidCommit = async ref => { try { await this.props.repository.getCommit(ref); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index cbdea98a07..b0aecfc48f 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -315,7 +315,7 @@ describe('RootController', function() { }); }); - describe('clone', function() { + describe('openCloneDialog()', function() { let clone; beforeEach(function() { @@ -357,6 +357,62 @@ describe('RootController', function() { }); }); + describe('openIssueishDialog()', function() { + let repository, workdir; + + beforeEach(async function() { + workdir = await cloneRepository('multiple-commits'); + repository = await buildRepository(workdir); + }); + + it('renders the OpenIssueish dialog', function() { + const wrapper = shallow(app); + wrapper.find('Command[command="github:open-issue-or-pull-request"]').prop('callback')(); + wrapper.update(); + + assert.strictEqual(wrapper.find('DialogsController').prop('request').identifier, 'issueish'); + }); + + it('triggers the open callback on accept and fires `open-commit-in-pane` event', async function() { + sinon.stub(reporterProxy, 'addEvent'); + sinon.stub(workspace, 'open').resolves(); + + const wrapper = shallow(React.cloneElement(app, {repository})); + wrapper.find('Command[command="github:open-issue-or-pull-request"]').prop('callback')(); + wrapper.update(); + + const request = wrapper.find('DialogsController').prop('request'); + await request.accept('https://github.com/atom/github/pull/123'); + + assert.isTrue(workspace.open.calledWith( + IssueishDetailItem.buildURI({ + host: 'github.com', + owner: 'atom', + repo: 'github', + number: 123, + workdir, + }), + {searchAllPanes: true}, + )); + assert.isTrue(reporterProxy.addEvent.calledWith( + 'open-issueish-in-pane', {package: 'github', from: 'dialog'}), + ); + }); + + it('dismisses the OpenIssueish dialog on cancel', function() { + const wrapper = shallow(app); + wrapper.find('Command[command="github:open-issue-or-pull-request"]').prop('callback')(); + wrapper.update(); + + const req0 = wrapper.find('DialogsController').prop('request'); + req0.cancel(); + + wrapper.update(); + const req1 = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req1, dialogRequests.null); + }); + }); + describe('github:open-commit', function() { let workdirPath, wrapper, openCommitDetails, resolveOpenCommit, repository; @@ -424,68 +480,6 @@ describe('RootController', function() { }); }); - describe('github:open-issue-or-pull-request', function() { - let workdirPath, wrapper, openIssueishDetails, resolveOpenIssueish; - - beforeEach(async function() { - openIssueishDetails = sinon.stub(atomEnv.workspace, 'open').returns(new Promise(resolve => { - resolveOpenIssueish = resolve; - })); - - workdirPath = await cloneRepository('multiple-commits'); - const repository = await buildRepository(workdirPath); - - app = React.cloneElement(app, {repository}); - wrapper = shallow(app); - }); - - it('renders the modal open-commit panel', function() { - wrapper.instance().showOpenIssueishDialog(); - wrapper.update(); - - assert.lengthOf(wrapper.find('Panel').find({location: 'modal'}).find('OpenIssueishDialog'), 1); - }); - - it('triggers the open callback on accept and fires `open-commit-in-pane` event', async function() { - sinon.stub(reporterProxy, 'addEvent'); - wrapper.instance().showOpenIssueishDialog(); - wrapper.update(); - - const dialog = wrapper.find('OpenIssueishDialog'); - const repoOwner = 'owner'; - const repoName = 'name'; - const issueishNumber = 1234; - - const promise = dialog.prop('didAccept')({repoOwner, repoName, issueishNumber}); - resolveOpenIssueish(); - await promise; - - const uri = IssueishDetailItem.buildURI({ - host: 'github.com', - owner: repoOwner, - repo: repoName, - number: issueishNumber, - }); - - assert.isTrue(openIssueishDetails.calledWith(uri)); - - await assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-pane', {package: 'github', from: 'dialog'})); - }); - - it('dismisses the open-commit panel on cancel', function() { - wrapper.instance().showOpenIssueishDialog(); - wrapper.update(); - - const dialog = wrapper.find('OpenIssueishDialog'); - dialog.prop('didCancel')(); - - wrapper.update(); - assert.lengthOf(wrapper.find('OpenIssueishDialog'), 0); - assert.isFalse(openIssueishDetails.called); - assert.isFalse(wrapper.state('openIssueishDialogActive')); - }); - }); - describe('openCredentialsDialog()', function() { it('renders the modal credentials dialog', function() { const wrapper = shallow(app); @@ -1183,16 +1177,6 @@ describe('RootController', function() { assert.strictEqual(item.getTitle(), 'owner/repo#123'); assert.lengthOf(wrapper.update().find('IssueishDetailItem'), 1); }); - - describe('acceptOpenIssueish', function() { - it('records an event', async function() { - const wrapper = mount(app); - sinon.stub(reporterProxy, 'addEvent'); - sinon.stub(workspace, 'open').returns(Promise.resolve()); - await wrapper.instance().acceptOpenIssueish({repoOwner: 'owner', repoName: 'repo', issueishNumber: 123}); - assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-pane', {package: 'github', from: 'dialog'})); - }); - }); }); describe('opening a CommitPreviewItem', function() { From 38a5433343ce1ca2693005ec202c2392504d81f0 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 10:47:36 -0400 Subject: [PATCH 17/41] Port OpenCommitDialog --- lib/controllers/dialogs-controller.js | 4 + lib/views/open-commit-dialog.js | 145 +++++++++++++------------- test/views/open-commit-dialog.test.js | 98 +++++++---------- 3 files changed, 112 insertions(+), 135 deletions(-) diff --git a/lib/controllers/dialogs-controller.js b/lib/controllers/dialogs-controller.js index d7e29b18d7..471507c6b1 100644 --- a/lib/controllers/dialogs-controller.js +++ b/lib/controllers/dialogs-controller.js @@ -166,4 +166,8 @@ export const dialogRequests = { issueish() { return new DialogRequest('issueish'); }, + + commit() { + return new DialogRequest('commit'); + }, }; diff --git a/lib/views/open-commit-dialog.js b/lib/views/open-commit-dialog.js index 82c9af828c..a163fe5c1b 100644 --- a/lib/views/open-commit-dialog.js +++ b/lib/views/open-commit-dialog.js @@ -1,113 +1,110 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {CompositeDisposable} from 'event-kit'; +import {TextBuffer} from 'atom'; import Commands, {Command} from '../atom/commands'; +import AtomTextEditor from '../atom/atom-text-editor'; +import CommitDetailItem from '../items/commit-detail-item'; +import {GitError} from '../git-shell-out-strategy'; export default class OpenCommitDialog extends React.Component { static propTypes = { + // Model + request: PropTypes.shape({ + getParams: PropTypes.func.isRequired, + accept: PropTypes.func.isRequired, + cancel: PropTypes.func.isRequired, + }).isRequired, + error: PropTypes.instanceOf(Error), + + // Atom environment commands: PropTypes.object.isRequired, - didAccept: PropTypes.func.isRequired, - didCancel: PropTypes.func.isRequired, - isValidEntry: PropTypes.func.isRequired, } - constructor(props, context) { - super(props, context); + constructor(props) { + super(props); + + this.ref = new TextBuffer(); + this.sub = this.ref.onDidChange(this.didChangeRef); this.state = { - error: null, + openEnabled: false, }; - this.subs = new CompositeDisposable(); - } - - componentDidMount() { - setTimeout(() => this.commitRefElement.focus()); } componentWillUnmount() { - this.subs.dispose(); + this.sub.dispose(); } render() { - return this.renderDialog(); - } - - renderDialog() { return (
- - + + -
+
- {this.state.error && {this.state.error}}
-
- - -
+
+ {this.props.error && ( +
    +
  • {this.props.error.userMessage || this.props.error.message}
    • +
+ )} +
+ + +
+
); } - accept = async () => { - const ref = this.getCommitRef(); - const valid = await this.props.isValidEntry(ref); - if (valid === true) { - this.props.didAccept({ref}); - } else { - this.setState({error: `There is no commit associated with "${ref}" in this repository`}); + accept = () => { + const ref = this.ref.getText(); + if (ref.length === 0) { + return Promise.resolve(); } - } - - cancel = () => this.props.didCancel() - - editorRefs = baseName => { - const elementName = `${baseName}Element`; - const modelName = `${baseName}Editor`; - const subName = `${baseName}Subs`; - const changeMethodName = `didChange${baseName[0].toUpperCase()}${baseName.substring(1)}`; - return element => { - if (!element) { - return; - } + return this.props.request.accept(ref); + } - this[elementName] = element; - const editor = element.getModel(); - if (this[modelName] !== editor) { - this[modelName] = editor; + didChangeRef = () => { + const enabled = !this.ref.isEmpty(); + if (this.state.openEnabled !== enabled) { + this.setState({openEnabled: enabled}); + } + } +} - /* istanbul ignore if */ - if (this[subName]) { - this[subName].dispose(); - this.subs.remove(this[subName]); - } +export async function openCommitItem(ref, {workspace, repository}) { + try { + await repository.getCommit(ref); + } catch (error) { + if (error instanceof GitError && error.code === 128) { + error.userMessage = 'There is no commit associated with that reference.'; + } - this[subName] = editor.onDidChange(this[changeMethodName]); - this.subs.add(this[subName]); - } - }; + throw error; } - didChangeCommitRef = () => new Promise(resolve => { - this.setState({error: null}, resolve); - }) + const item = await workspace.open( + CommitDetailItem.buildURI(repository.getWorkingDirectoryPath(), ref), + {searchAllPanes: true}, + ); - getCommitRef() { - return this.commitRefEditor ? this.commitRefEditor.getText() : ''; - } + return item; } diff --git a/test/views/open-commit-dialog.test.js b/test/views/open-commit-dialog.test.js index 78c8b2fbac..7057b6eace 100644 --- a/test/views/open-commit-dialog.test.js +++ b/test/views/open-commit-dialog.test.js @@ -1,97 +1,73 @@ import React from 'react'; -import {mount} from 'enzyme'; +import {shallow} from 'enzyme'; import OpenCommitDialog from '../../lib/views/open-commit-dialog'; +import {dialogRequests} from '../../lib/controllers/dialogs-controller'; describe('OpenCommitDialog', function() { - let atomEnv, commands; - let app, wrapper, didAccept, didCancel, isValidEntry; + let atomEnv; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); - commands = atomEnv.commands; - - didAccept = sinon.stub(); - didCancel = sinon.stub(); - isValidEntry = sinon.stub().returns(true); - - app = ( - - ); - wrapper = mount(app); }); afterEach(function() { atomEnv.destroy(); }); - const setTextIn = function(selector, text) { - wrapper.find(selector).getDOMNode().getModel().setText(text); - }; + function isValidRef(ref) { + return Promise.resolve(ref === 'abcd1234'); + } - describe('entering a commit sha', function() { - it("updates the commit ref automatically if it hasn't been modified", function() { - setTextIn('.github-CommitRef atom-text-editor', 'asdf1234'); + function buildApp(overrides = {}) { + const request = dialogRequests.commit(); - assert.equal(wrapper.instance().getCommitRef(), 'asdf1234'); - }); - - it('does update the ref if it was modified automatically', function() { - setTextIn('.github-CommitRef atom-text-editor', 'asdf1234'); - assert.equal(wrapper.instance().getCommitRef(), 'asdf1234'); - - setTextIn('.github-CommitRef atom-text-editor', 'zxcv5678'); - assert.equal(wrapper.instance().getCommitRef(), 'zxcv5678'); - }); - }); + return ( + + ); + } - describe('open button enablement and error state', function() { + describe('open button enablement', function() { it('disables the open button with no commit ref', function() { - setTextIn('.github-CommitRef atom-text-editor', ''); - wrapper.update(); - - assert.isTrue(wrapper.find('button.icon-commit').prop('disabled')); - assert.isFalse(wrapper.find('.error').exists()); - }); - - it('disables the open button when the commit does not exist in repo', async function() { - isValidEntry.returns(false); - const ref = 'abcd1234'; - setTextIn('.github-CommitRef atom-text-editor', ref); - wrapper.find('button.icon-commit').simulate('click'); + const wrapper = shallow(buildApp()); - await assert.async.strictEqual(wrapper.update().find('.error').text(), `There is no commit associated with "${ref}" in this repository`); assert.isTrue(wrapper.find('button.icon-commit').prop('disabled')); }); - it('enables the open button when commit sha box is populated with a valid sha', function() { - setTextIn('.github-CommitRef atom-text-editor', 'abcd1234'); - wrapper.update(); + it('enables the open button when commit sha box is populated', function() { + const wrapper = shallow(buildApp()); + wrapper.find('AtomTextEditor').prop('buffer').setText('abcd1234'); assert.isFalse(wrapper.find('button.icon-commit').prop('disabled')); - assert.isFalse(wrapper.find('.error').exists()); }); }); - it('calls the acceptance callback after validation', async function() { - isValidEntry.returns(true); - const ref = 'abcd1234'; - setTextIn('.github-CommitRef atom-text-editor', ref); + it('calls the acceptance callback with the entered ref', function() { + const accept = sinon.spy(); + const request = dialogRequests.commit(); + request.onAccept(accept); + const wrapper = shallow(buildApp({request})); + wrapper.find('AtomTextEditor').prop('buffer').setText('abcd1234'); wrapper.find('button.icon-commit').simulate('click'); - await assert.async.isTrue(didAccept.calledWith({ref})); + assert.isTrue(accept.calledWith('abcd1234')); wrapper.unmount(); }); it('calls the cancellation callback', function() { - wrapper.find('button.github-CancelButton').simulate('click'); - assert.isTrue(didCancel.called); - wrapper.unmount(); + const cancel = sinon.spy(); + const request = dialogRequests.commit(); + request.onCancel(cancel); + + const wrapper = shallow(buildApp({request})); + + wrapper.find('button.github-Dialog-cancelButton').simulate('click'); + assert.isTrue(cancel.called); }); }); From 2fd134cf21fd15e7bb98c31cc4e4f90afd6c5577 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 10:50:09 -0400 Subject: [PATCH 18/41] Add OpenCommitDialog to DialogsController --- lib/controllers/dialogs-controller.js | 12 ++++++++++-- test/controllers/dialogs-controller.test.js | 20 +++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/controllers/dialogs-controller.js b/lib/controllers/dialogs-controller.js index 471507c6b1..122d680db6 100644 --- a/lib/controllers/dialogs-controller.js +++ b/lib/controllers/dialogs-controller.js @@ -5,7 +5,8 @@ import Panel from '../atom/panel'; import InitDialog from '../views/init-dialog'; import CloneDialog from '../views/clone-dialog'; import CredentialDialog from '../views/credential-dialog'; -import IssueishDialog from '../views/open-issueish-dialog'; +import OpenIssueishDialog from '../views/open-issueish-dialog'; +import OpenCommitDialog from '../views/open-commit-dialog'; export default class DialogsController extends React.Component { static propTypes = { @@ -30,6 +31,7 @@ export default class DialogsController extends React.Component { clone: this.renderCloneDialog, credential: this.renderCredentialDialog, issueish: this.renderIssueishDialog, + commit: this.renderCommitDialog, }; this.state = { @@ -62,7 +64,13 @@ export default class DialogsController extends React.Component { renderIssueishDialog = () => ( - + + + ) + + renderCommitDialog = () => ( + + ) diff --git a/test/controllers/dialogs-controller.test.js b/test/controllers/dialogs-controller.test.js index 7f7fba5b9d..4772d3b855 100644 --- a/test/controllers/dialogs-controller.test.js +++ b/test/controllers/dialogs-controller.test.js @@ -199,6 +199,24 @@ describe('DialogsController', function() { assert.isTrue(cancel.called); }); - it('passes appropriate props to OpenCommitDialog'); + it('passes appropriate props to OpenCommitDialog', function() { + const accept = sinon.spy(); + const cancel = sinon.spy(); + const request = dialogRequests.commit(); + request.onAccept(accept); + request.onCancel(cancel); + + const wrapper = shallow(buildApp({request})); + const dialog = wrapper.find('OpenCommitDialog'); + assert.strictEqual(dialog.prop('commands'), atomEnv.commands); + + const req = dialog.prop('request'); + + req.accept('abcd1234'); + assert.isTrue(accept.calledWith('abcd1234')); + + req.cancel(); + assert.isTrue(cancel.called); + }); }); }); From 0083537b607e1896f9a58d245ae25896fa3dc6ad Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 11:11:52 -0400 Subject: [PATCH 19/41] Test openCommitDetailItem --- lib/views/open-commit-dialog.js | 5 ++- test/views/open-commit-dialog.test.js | 62 ++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/lib/views/open-commit-dialog.js b/lib/views/open-commit-dialog.js index a163fe5c1b..a43c0908c9 100644 --- a/lib/views/open-commit-dialog.js +++ b/lib/views/open-commit-dialog.js @@ -6,6 +6,7 @@ import Commands, {Command} from '../atom/commands'; import AtomTextEditor from '../atom/atom-text-editor'; import CommitDetailItem from '../items/commit-detail-item'; import {GitError} from '../git-shell-out-strategy'; +import {addEvent} from '../reporter-proxy'; export default class OpenCommitDialog extends React.Component { static propTypes = { @@ -90,7 +91,7 @@ export default class OpenCommitDialog extends React.Component { } } -export async function openCommitItem(ref, {workspace, repository}) { +export async function openCommitDetailItem(ref, {workspace, repository}) { try { await repository.getCommit(ref); } catch (error) { @@ -105,6 +106,6 @@ export async function openCommitItem(ref, {workspace, repository}) { CommitDetailItem.buildURI(repository.getWorkingDirectoryPath(), ref), {searchAllPanes: true}, ); - + addEvent('open-commit-in-pane', {package: 'github', from: OpenCommitDialog.name}); return item; } diff --git a/test/views/open-commit-dialog.test.js b/test/views/open-commit-dialog.test.js index 7057b6eace..c3372e74f9 100644 --- a/test/views/open-commit-dialog.test.js +++ b/test/views/open-commit-dialog.test.js @@ -1,8 +1,11 @@ import React from 'react'; import {shallow} from 'enzyme'; -import OpenCommitDialog from '../../lib/views/open-commit-dialog'; +import OpenCommitDialog, {openCommitDetailItem} from '../../lib/views/open-commit-dialog'; import {dialogRequests} from '../../lib/controllers/dialogs-controller'; +import CommitDetailItem from '../../lib/items/commit-detail-item'; +import {GitError} from '../../lib/git-shell-out-strategy'; +import * as reporterProxy from '../../lib/reporter-proxy'; describe('OpenCommitDialog', function() { let atomEnv; @@ -70,4 +73,61 @@ describe('OpenCommitDialog', function() { wrapper.find('button.github-Dialog-cancelButton').simulate('click'); assert.isTrue(cancel.called); }); + + describe('openCommitDetailItem()', function() { + let repository; + + beforeEach(function() { + sinon.stub(atomEnv.workspace, 'open').resolves('item'); + sinon.stub(reporterProxy, 'addEvent'); + + repository = { + getWorkingDirectoryPath() { + return __dirname; + }, + getCommit(ref) { + if (ref === 'abcd1234') { + return Promise.resolve('ok'); + } + + if (ref === 'bad') { + const e = new GitError('bad ref'); + e.code = 128; + return Promise.reject(e); + } + + return Promise.reject(new GitError('other error')); + }, + }; + }); + + it('opens a CommitDetailItem with the chosen valid ref and records an event', async function() { + assert.strictEqual(await openCommitDetailItem('abcd1234', {workspace: atomEnv.workspace, repository}), 'item'); + assert.isTrue(atomEnv.workspace.open.calledWith( + CommitDetailItem.buildURI(__dirname, 'abcd1234'), + {searchAllPanes: true}, + )); + assert.isTrue(reporterProxy.addEvent.calledWith( + 'open-commit-in-pane', + {package: 'github', from: OpenCommitDialog.name}, + )); + }); + + it('raises a friendly error if the ref is invalid', async function() { + const e = await openCommitDetailItem('bad', {workspace: atomEnv.workspace, repository}).then( + () => { throw new Error('unexpected success'); }, + error => error, + ); + assert.strictEqual(e.userMessage, 'There is no commit associated with that reference.'); + }); + + it('passes other errors through directly', async function() { + const e = await openCommitDetailItem('nope', {workspace: atomEnv.workspace, repository}).then( + () => { throw new Error('unexpected success'); }, + error => error, + ); + assert.isUndefined(e.userMessage); + assert.strictEqual(e.message, 'other error'); + }); + }); }); From 81142cf14f074b16bc9727899a106ca44e7fd0bf Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 11:25:32 -0400 Subject: [PATCH 20/41] Manage the OpenCommitDialog via DialogsController --- lib/controllers/root-controller.js | 74 ++++++------------------ test/controllers/root-controller.test.js | 74 +++++++++--------------- 2 files changed, 45 insertions(+), 103 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index f96506f317..0bab13f9e6 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -7,10 +7,9 @@ import PropTypes from 'prop-types'; import {CompositeDisposable} from 'event-kit'; import StatusBar from '../atom/status-bar'; -import Panel from '../atom/panel'; import PaneItem from '../atom/pane-item'; import {openIssueishItem} from '../views/open-issueish-dialog'; -import OpenCommitDialog from '../views/open-commit-dialog'; +import {openCommitDetailItem} from '../views/open-commit-dialog'; import Commands, {Command} from '../atom/commands'; import ChangedFileItem from '../items/changed-file-item'; import IssueishDetailItem from '../items/issueish-detail-item'; @@ -145,7 +144,7 @@ export default class RootController extends React.Component { - + - - {this.renderOpenCommitDialog()} - - ); - } - - renderOpenCommitDialog() { - if (!this.state.openCommitDialogActive) { - return null; - } - - return ( - - - + ); } @@ -553,8 +532,15 @@ export default class RootController extends React.Component { return new Promise(resolve => this.setState({dialogRequest}, resolve)); } - showOpenCommitDialog = () => { - this.setState({openCommitDialogActive: true}); + openCommitDialog = () => { + const dialogRequest = dialogRequests.commit(); + dialogRequest.onAccept(ref => openCommitDetailItem(ref, { + workspace: this.props.workspace, + repository: this.props.repository, + })); + dialogRequest.onCancel(this.cancelDialog); + + return new Promise(resolve => this.setState({dialogRequest}, resolve)); } showWaterfallDiagnostics() { @@ -565,32 +551,6 @@ export default class RootController extends React.Component { this.props.workspace.open(GitCacheView.buildURI()); } - isValidCommit = async ref => { - try { - await this.props.repository.getCommit(ref); - return true; - } catch (error) { - if (error instanceof GitError && error.code === 128) { - return false; - } else { - throw error; - } - } - } - - acceptOpenCommit = ({ref}) => { - const workdir = this.props.repository.getWorkingDirectoryPath(); - const uri = CommitDetailItem.buildURI(workdir, ref); - this.setState({openCommitDialogActive: false}); - this.props.workspace.open(uri).then(() => { - addEvent('open-commit-in-pane', {package: 'github', from: OpenCommitDialog.name}); - }); - } - - cancelOpenCommit = () => { - this.setState({openCommitDialogActive: false}); - } - surfaceFromFileAtPath = (filePath, stagingStatus) => { const gitTab = this.gitTabTracker.getComponent(); return gitTab && gitTab.focusAndSelectStagingItem(filePath, stagingStatus); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index b0aecfc48f..e704c89baa 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -22,7 +22,6 @@ import CommitDetailItem from '../../lib/items/commit-detail-item'; import * as reporterProxy from '../../lib/reporter-proxy'; import RootController from '../../lib/controllers/root-controller'; -import OpenCommitDialog from '../../lib/views/open-commit-dialog'; describe('RootController', function() { let atomEnv, app; @@ -413,70 +412,53 @@ describe('RootController', function() { }); }); - describe('github:open-commit', function() { - let workdirPath, wrapper, openCommitDetails, resolveOpenCommit, repository; + describe('openCommitDialog()', function() { + let workdirPath, repository; beforeEach(async function() { - openCommitDetails = sinon.stub(atomEnv.workspace, 'open').returns(new Promise(resolve => { - resolveOpenCommit = resolve; - })); + sinon.stub(reporterProxy, 'addEvent'); + sinon.stub(atomEnv.workspace, 'open').resolves('item'); workdirPath = await cloneRepository('multiple-commits'); repository = await buildRepository(workdirPath); + sinon.stub(repository, 'getCommit').callsFake(ref => { + return ref === 'abcd1234' ? Promise.resolve('ok') : Promise.reject(new Error('nah')); + }); app = React.cloneElement(app, {repository}); - wrapper = shallow(app); }); - it('renders the modal open-commit panel', function() { - wrapper.instance().showOpenCommitDialog(); - wrapper.update(); + it('renders the OpenCommitDialog', function() { + const wrapper = shallow(app); - assert.lengthOf(wrapper.find('Panel').find({location: 'modal'}).find('OpenCommitDialog'), 1); + wrapper.find('Command[command="github:open-commit"]').prop('callback')(); + assert.strictEqual(wrapper.find('DialogsController').prop('request').identifier, 'commit'); }); - it('triggers the open callback on accept and fires `open-commit-in-pane` event', async function() { - sinon.stub(reporterProxy, 'addEvent'); - wrapper.instance().showOpenCommitDialog(); - wrapper.update(); - - const dialog = wrapper.find('OpenCommitDialog'); - const ref = 'asdf1234'; - - const promise = dialog.prop('didAccept')({ref}); - resolveOpenCommit(); - await promise; - - const uri = CommitDetailItem.buildURI(workdirPath, ref); + it('triggers the open callback on accept', async function() { + const wrapper = shallow(app); + wrapper.find('Command[command="github:open-commit"]').prop('callback')(); - assert.isTrue(openCommitDetails.calledWith(uri)); + const req = wrapper.find('DialogsController').prop('request'); + await req.accept('abcd1234'); - await assert.isTrue(reporterProxy.addEvent.calledWith('open-commit-in-pane', {package: 'github', from: OpenCommitDialog.name})); + assert.isTrue(workspace.open.calledWith( + CommitDetailItem.buildURI(repository.getWorkingDirectoryPath(), 'abcd1234'), + {searchAllPanes: true}, + )); + assert.isTrue(reporterProxy.addEvent.called); }); - it('dismisses the open-commit panel on cancel', function() { - wrapper.instance().showOpenCommitDialog(); - wrapper.update(); + it('dismisses the OpenCommitDialog on cancel', function() { + const wrapper = shallow(app); + wrapper.find('Command[command="github:open-commit"]').prop('callback')(); - const dialog = wrapper.find('OpenCommitDialog'); - dialog.prop('didCancel')(); + const req0 = wrapper.find('DialogsController').prop('request'); + req0.cancel(); wrapper.update(); - assert.lengthOf(wrapper.find('OpenCommitDialog'), 0); - assert.isFalse(openCommitDetails.called); - assert.isFalse(wrapper.state('openCommitDialogActive')); - }); - - describe('isValidCommit', function() { - it('returns true if commit exists in repo, false if not', async function() { - assert.isTrue(await wrapper.instance().isValidCommit('HEAD')); - assert.isFalse(await wrapper.instance().isValidCommit('invalidCommitRef')); - }); - - it('re-throws exceptions encountered during validation check', async function() { - sinon.stub(repository, 'getCommit').throws(new Error('Oh shit')); - await assert.isRejected(wrapper.instance().isValidCommit('HEAD'), 'Oh shit'); - }); + const req1 = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req1, dialogRequests.null); }); }); From 1b9e36337d36bb5766c290ac8c9c8e0d1e29c29c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 13:50:32 -0400 Subject: [PATCH 21/41] AutoFocus class to reduce focus management boilerplate in dialogs --- lib/autofocus.js | 75 ++++++++++++++++++++++++++++++++++++++++ lib/views/init-dialog.js | 19 ++++++++-- test/autofocus.test.js | 61 ++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 lib/autofocus.js create mode 100644 test/autofocus.test.js diff --git a/lib/autofocus.js b/lib/autofocus.js new file mode 100644 index 0000000000..0f9ee9027e --- /dev/null +++ b/lib/autofocus.js @@ -0,0 +1,75 @@ +/** + * When triggered, automatically focus the first element ref passed to this object. + * + * To unconditionally focus a single element: + * + * ``` + * class SomeComponent extends React.Component { + * constructor(props) { + * super(props); + * this.autofocus = new Autofocus(); + * } + * + * render() { + * return ( + *
+ * + * + *
+ * ); + * } + * + * componentDidMount() { + * this.autofocus.trigger(); + * } + * } + * ``` + * + * If multiple form elements are present, use `firstTarget` to create the ref instead. The rendered ref you assign the + * lowest numeric index will be focused on trigger: + * + * ``` + * class SomeComponent extends React.Component { + * constructor(props) { + * super(props); + * this.autofocus = new Autofocus(); + * } + * + * render() { + * return ( + *
+ * {this.props.someProp && } + * + * + *
+ * ); + * } + * + * componentDidMount() { + * this.autofocus.trigger(); + * } + * } + * ``` + * + */ +export default class AutoFocus { + constructor() { + this.index = Infinity; + this.captured = null; + } + + target = element => this.firstTarget(0)(element); + + firstTarget = index => element => { + if (index < this.index) { + this.index = index; + this.captured = element; + } + }; + + trigger() { + if (this.captured !== null) { + setTimeout(() => this.captured.focus(), 0); + } + } +} diff --git a/lib/views/init-dialog.js b/lib/views/init-dialog.js index 040c741f81..cf7ac49099 100644 --- a/lib/views/init-dialog.js +++ b/lib/views/init-dialog.js @@ -4,6 +4,7 @@ import {TextBuffer} from 'atom'; import AtomTextEditor from '../atom/atom-text-editor'; import Commands, {Command} from '../atom/commands'; +import AutoFocus from '../autofocus'; export default class InitDialog extends React.Component { static propTypes = { @@ -23,6 +24,8 @@ export default class InitDialog extends React.Component { constructor(props) { super(props); + this.autofocus = new AutoFocus(); + this.destinationPath = new TextBuffer({ text: this.props.request.getParams().dirPath, }); @@ -37,7 +40,7 @@ export default class InitDialog extends React.Component { render() { return (
- + @@ -45,6 +48,7 @@ export default class InitDialog extends React.Component { From b1b8f7c8350e5faa05ab5fb8aa5b71d84a03f597 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 18 Jul 2019 16:02:06 -0400 Subject: [PATCH 26/41] Improve the initialize dialog default a little --- lib/controllers/root-controller.js | 23 +++++++++++- lib/github-package.js | 16 ++++++--- test/controllers/root-controller.test.js | 45 ++++++++++++++++++++---- 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 5492c0c401..ade5b01b11 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -486,7 +486,28 @@ export default class RootController extends React.Component { closeDialog = () => new Promise(resolve => this.setState({dialogRequest: dialogRequests.null}, resolve)); - openInitializeDialog = dirPath => { + openInitializeDialog = async dirPath => { + if (!dirPath) { + const activeEditor = this.props.workspace.getActiveTextEditor(); + if (activeEditor) { + const [projectPath] = this.props.project.relativizePath(activeEditor.getPath()); + if (projectPath) { + dirPath = projectPath; + } + } + } + + if (!dirPath) { + const directories = this.props.project.getDirectories(); + const withRepositories = await Promise.all( + directories.map(async d => [d, await this.props.project.repositoryForDirectory(d)]), + ); + const firstUninitialized = withRepositories.find(([d, r]) => !r); + if (firstUninitialized && firstUninitialized[0]) { + dirPath = firstUninitialized[0].getPath(); + } + } + if (!dirPath) { dirPath = this.props.config.get('core.projectHome'); } diff --git a/lib/github-package.js b/lib/github-package.js index 86046495d0..e49389bdba 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -436,6 +436,7 @@ export default class GithubPackage { this.project.addPath(projectPath); } + await this.refreshAtomGitRepository(projectPath); await this.scheduleActiveContextUpdate(); } @@ -585,11 +586,16 @@ export default class GithubPackage { this.setActiveContext(nextActiveContext); } - refreshAtomGitRepository(workdir) { - const atomGitRepo = this.project.getRepositories().find(repo => { - return repo && path.normalize(repo.getWorkingDirectory()) === workdir; - }); - return atomGitRepo ? atomGitRepo.refreshStatus() : Promise.resolve(); + async refreshAtomGitRepository(workdir) { + const directory = this.project.getDirectoryForProjectPath(workdir); + if (!directory) { + return; + } + + const atomGitRepo = await this.project.repositoryForDirectory(directory); + if (atomGitRepo) { + await atomGitRepo.refreshStatus(); + } } } diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 37260a3e55..e5881ef17d 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -4,6 +4,7 @@ import fs from 'fs-extra'; import React from 'react'; import {shallow, mount} from 'enzyme'; import dedent from 'dedent-js'; +import temp from 'temp'; import {cloneRepository, buildRepository} from '../helpers'; import {multiFilePatchBuilder} from '../builder/patch'; @@ -268,24 +269,54 @@ describe('RootController', function() { app = React.cloneElement(app, {initialize}); }); - it('requests the init dialog with a command', function() { + it('requests the init dialog with a command', async function() { sinon.stub(config, 'get').returns(path.join('/home/me/src')); const wrapper = shallow(app); - wrapper.find('Command[command="github:initialize"]').prop('callback')(); + await wrapper.find('Command[command="github:initialize"]').prop('callback')(); const req = wrapper.find('DialogsController').prop('request'); assert.strictEqual(req.identifier, 'init'); assert.strictEqual(req.getParams().dirPath, path.join('/home/me/src')); }); - it('requests the init dialog from the git tab', function() { + it('defaults to the project directory containing the open file if there is one', async function() { + const noRepo0 = await new Promise((resolve, reject) => temp.mkdir({}, (err, p) => (err ? reject(err) : resolve(p)))); + const noRepo1 = await new Promise((resolve, reject) => temp.mkdir({}, (err, p) => (err ? reject(err) : resolve(p)))); + const filePath = path.join(noRepo1, 'file.txt'); + await fs.writeFile(filePath, 'stuff\n', {encoding: 'utf8'}); + + project.setPaths([noRepo0, noRepo1]); + await workspace.open(filePath); + + const wrapper = shallow(app); + await wrapper.find('Command[command="github:initialize"]').prop('callback')(); + const req = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req.identifier, 'init'); + assert.strictEqual(req.getParams().dirPath, noRepo1); + }); + + it('defaults to the first project directory with no repository if one is present', async function() { + const withRepo = await cloneRepository(); + const noRepo0 = await new Promise((resolve, reject) => temp.mkdir({}, (err, p) => (err ? reject(err) : resolve(p)))); + const noRepo1 = await new Promise((resolve, reject) => temp.mkdir({}, (err, p) => (err ? reject(err) : resolve(p)))); + + project.setPaths([withRepo, noRepo0, noRepo1]); + + const wrapper = shallow(app); + await wrapper.find('Command[command="github:initialize"]').prop('callback')(); + const req = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req.identifier, 'init'); + assert.strictEqual(req.getParams().dirPath, noRepo0); + }); + + it('requests the init dialog from the git tab', async function() { const wrapper = shallow(app); const gitTabWrapper = wrapper .find('PaneItem[className="github-Git-root"]') .renderProp('children')({itemHolder: new RefHolder()}); - gitTabWrapper.find('GitTabItem').prop('openInitializeDialog')(path.join('/some/workdir')); + await gitTabWrapper.find('GitTabItem').prop('openInitializeDialog')(path.join('/some/workdir')); const req = wrapper.find('DialogsController').prop('request'); assert.strictEqual(req.identifier, 'init'); @@ -294,7 +325,7 @@ describe('RootController', function() { it('triggers the initialize callback on accept', async function() { const wrapper = shallow(app); - wrapper.find('Command[command="github:initialize"]').prop('callback')(); + await wrapper.find('Command[command="github:initialize"]').prop('callback')(); const req0 = wrapper.find('DialogsController').prop('request'); await req0.accept(path.join('/home/me/src')); @@ -304,9 +335,9 @@ describe('RootController', function() { assert.strictEqual(req1, dialogRequests.null); }); - it('dismisses the dialog with its cancel callback', function() { + it('dismisses the dialog with its cancel callback', async function() { const wrapper = shallow(app); - wrapper.find('Command[command="github:initialize"]').prop('callback')(); + await wrapper.find('Command[command="github:initialize"]').prop('callback')(); const req0 = wrapper.find('DialogsController').prop('request'); assert.notStrictEqual(req0, dialogRequests.null); From 98e6b5d6eaca8b284014c12e24470be9f9fca1a7 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 19 Jul 2019 09:03:39 -0400 Subject: [PATCH 27/41] Remove visible prop from Panel --- lib/atom/panel.js | 19 +------------------ test/atom/panel.test.js | 19 ------------------- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/lib/atom/panel.js b/lib/atom/panel.js index 9db1de1962..507744a0a5 100644 --- a/lib/atom/panel.js +++ b/lib/atom/panel.js @@ -22,14 +22,12 @@ export default class Panel extends React.Component { children: PropTypes.element.isRequired, options: PropTypes.object, onDidClosePanel: PropTypes.func, - visible: PropTypes.bool, itemHolder: RefHolderPropType, } static defaultProps = { options: {}, onDidClosePanel: panel => {}, - visible: true, } constructor(props) { @@ -46,21 +44,6 @@ export default class Panel extends React.Component { this.setupPanel(); } - shouldComponentUpdate(newProps) { - return this.props.visible !== newProps.visible; - } - - componentDidUpdate() { - if (this.didCloseItem) { - // eslint-disable-next-line no-console - console.error('Unexpected update in `Panel`: the contained panel has been destroyed'); - } - - if (this.panel) { - this.panel[this.props.visible ? 'show' : 'hide'](); - } - } - render() { return ReactDOM.createPortal( this.props.children, @@ -76,7 +59,7 @@ export default class Panel extends React.Component { const methodName = `add${location}Panel`; const item = createItem(this.domNode, this.props.itemHolder); - const options = {...this.props.options, visible: this.props.visible, item}; + const options = {...this.props.options, item}; this.panel = this.props.workspace[methodName](options); this.subscriptions.add( this.panel.onDidDestroy(() => { diff --git a/test/atom/panel.test.js b/test/atom/panel.test.js index e8474f9e8a..afe0a8446c 100644 --- a/test/atom/panel.test.js +++ b/test/atom/panel.test.js @@ -51,7 +51,6 @@ describe('Panel', function() { assert.strictEqual(workspace.addLeftPanel.callCount, 1); const options = workspace.addLeftPanel.args[0][0]; assert.strictEqual(options.some, 'option'); - assert.isTrue(options.visible); assert.isDefined(options.item.getElement()); const panel = wrapper.instance().getPanel(); @@ -69,22 +68,4 @@ describe('Panel', function() { wrapper.instance().getPanel().destroy(); assert.strictEqual(onDidClosePanel.callCount, 1); }); - - describe('when updating the visible prop', function() { - it('shows or hides the panel', function() { - const wrapper = mount( - - - , - ); - - const panel = wrapper.instance().getPanel(); - - wrapper.setProps({visible: false}); - assert.strictEqual(panel.hide.callCount, 1); - - wrapper.setProps({visible: true}); - assert.strictEqual(panel.show.callCount, 1); - }); - }); }); From 466d4cbf07b813e0ded713295f5cf80bd5193ca2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 19 Jul 2019 09:16:11 -0400 Subject: [PATCH 28/41] Always render github-DialogInfo, even if it's empty --- lib/views/init-dialog.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/views/init-dialog.js b/lib/views/init-dialog.js index 15d69d070d..8c61f00eca 100644 --- a/lib/views/init-dialog.js +++ b/lib/views/init-dialog.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {Fragment} from 'react'; import PropTypes from 'prop-types'; import {TextBuffer} from 'atom'; @@ -57,17 +57,19 @@ export default class InitDialog extends React.Component {
- {this.props.inProgress && ( -
- - initializing... -
- )} - {this.props.error && ( -
    -
  • {this.props.error.userMessage || this.props.error.message}
    • -
- )} +
+ {this.props.inProgress && ( + + + initializing... + + )} + {this.props.error && ( +
    +
  • {this.props.error.userMessage || this.props.error.message}
    • +
+ )} +
diff --git a/lib/views/credential-dialog.js b/lib/views/credential-dialog.js index 36f623166f..dc5803f98a 100644 --- a/lib/views/credential-dialog.js +++ b/lib/views/credential-dialog.js @@ -72,13 +72,17 @@ export default class CredentialDialog extends React.Component { )}
- {this.props.error && ( -
    -
  • {this.props.error.userMessage || this.props.error.message}
    • -
- )} - - +
+ {this.props.error && ( +
    +
  • {this.props.error.userMessage || this.props.error.message}
    • +
+ )} +
+
+ + +
); diff --git a/lib/views/open-commit-dialog.js b/lib/views/open-commit-dialog.js index a43c0908c9..93d436e0a6 100644 --- a/lib/views/open-commit-dialog.js +++ b/lib/views/open-commit-dialog.js @@ -51,11 +51,13 @@ export default class OpenCommitDialog extends React.Component {
- {this.props.error && ( -
    -
  • {this.props.error.userMessage || this.props.error.message}
    • -
- )} +
+ {this.props.error && ( +
    +
  • {this.props.error.userMessage || this.props.error.message}
    • +
+ )} +
{params.includeRemember && ( -
); } + componentDidMount() { + this.autofocus.trigger(); + } + + recaptureFocus = () => this.autofocus.trigger(); + accept = () => { + if (!this.canSignIn()) { + return Promise.resolve(); + } + const request = this.props.request; const params = request.getParams(); @@ -112,4 +139,8 @@ export default class CredentialDialog extends React.Component { didChangeRemember = e => this.setState({remember: e.target.checked}); toggleShowPassword = () => this.setState({showPassword: !this.state.showPassword}); + + canSignIn() { + return !this.props.request.getParams().includeUsername || this.state.username.length > 0; + } } diff --git a/test/views/credential-dialog.test.js b/test/views/credential-dialog.test.js index 0efc4c3c95..44a8b1c5d8 100644 --- a/test/views/credential-dialog.test.js +++ b/test/views/credential-dialog.test.js @@ -112,4 +112,28 @@ describe('CredentialDialog', function() { assert.strictEqual(passwordInput.prop('type'), 'password'); }); }); + + describe('sign in button enablement', function() { + it('is always enabled when includeUsername is false', function() { + const request = dialogRequests.credential({includeUsername: false}); + const wrapper = shallow(buildApp({request})); + + assert.isFalse(wrapper.find('.btn-primary').prop('disabled')); + }); + + it('is disabled if includeUsername is true and the username is empty', function() { + const request = dialogRequests.credential({includeUsername: true}); + const wrapper = shallow(buildApp({request})); + + assert.isTrue(wrapper.find('.btn-primary').prop('disabled')); + }); + + it('is enabled if includeUsername is true and the username is populated', function() { + const request = dialogRequests.credential({includeUsername: true}); + const wrapper = shallow(buildApp({request})); + wrapper.find('.github-Credential-username').simulate('change', {target: {value: 'nonempty'}}); + + assert.isFalse(wrapper.find('.btn-primary').prop('disabled')); + }); + }); }); From b183168e91258fcc1cc1190a4b7ab33f6f48e2af Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 19 Jul 2019 10:49:10 -0400 Subject: [PATCH 33/41] CSS work --- styles/dialog.less | 47 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/styles/dialog.less b/styles/dialog.less index e2b91cfab2..4ad9176cbe 100644 --- a/styles/dialog.less +++ b/styles/dialog.less @@ -7,6 +7,7 @@ margin: @component-padding; text-align: center; font-size: 1.2em; + user-select: none; } &Form { @@ -21,17 +22,18 @@ position: relative; line-height: 2; - &Button { - position: absolute; - background: transparent; - right: .3em; - bottom: 0; - border: none; - color: @text-color-subtle; - cursor: pointer; + &--horizontal { + display: flex; + flex-direction: row; + justify-content: flex-end; + align-items: center; + + input { + margin: 0 @component-padding; + } - &:hover { - color: @text-color-highlight; + input[type="text"] , input[type="password"] { + width: 85%; } } @@ -84,6 +86,26 @@ } } + &--insetButton { + position: absolute; + background: transparent; + right: 1em; + top: 0; + bottom: 0; + border: none; + color: @text-color-subtle; + cursor: pointer; + + &:hover { + color: @text-color-highlight; + } + + &:focus { + border: solid 1px @button-background-color-selected; + border-radius: 4px; + } + } + &Spinner { display: flex; align-items: center; @@ -102,4 +124,9 @@ color: @text-color-subtle; } } + + &:not(:focus-within) { + padding-bottom: 1px; + transition: padding-bottom 0.01s; + } } From 45be3ed88299db2e74dcaf5e71e1196e92046491 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 19 Jul 2019 10:55:53 -0400 Subject: [PATCH 34/41] Close credentials dialog on cancel --- lib/controllers/root-controller.js | 7 +++++-- test/controllers/root-controller.test.js | 9 ++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index ade5b01b11..08ce137b8c 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -536,11 +536,14 @@ export default class RootController extends React.Component { openCredentialsDialog = query => { return new Promise((resolve, reject) => { const dialogRequest = dialogRequests.credential(query); - dialogRequest.onAccept(async result => { + dialogRequest.onProgressingAccept(async result => { resolve(result); await this.closeDialog(); }); - dialogRequest.onCancel(reject); + dialogRequest.onCancel(async () => { + reject(); + await this.closeDialog(); + }); this.setState({dialogRequest}); }); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index e5881ef17d..ee56e91dc0 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -531,7 +531,7 @@ describe('RootController', function() { }); const req0 = wrapper.find('DialogsController').prop('request'); - req0.accept({password: 'friend'}); + await req0.accept({password: 'friend'}); assert.deepEqual(await credentialPromise, {password: 'friend'}); const req1 = wrapper.find('DialogsController').prop('request'); @@ -546,9 +546,12 @@ describe('RootController', function() { }); wrapper.update(); - const req = wrapper.find('DialogsController').prop('request'); - req.cancel(new Error('cancelled')); + const req0 = wrapper.find('DialogsController').prop('request'); + await req0.cancel(new Error('cancelled')); await assert.isRejected(credentialPromise); + + const req1 = wrapper.find('DialogsController').prop('request'); + assert.strictEqual(req1, dialogRequests.null); }); }); From baa3c5ec047197b81beaa4601e0ba54511270f93 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 19 Jul 2019 11:19:59 -0400 Subject: [PATCH 35/41] Issueish dialog glitches --- lib/controllers/root-controller.js | 2 +- lib/views/open-issueish-dialog.js | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 08ce137b8c..9b77e89d6c 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -551,7 +551,7 @@ export default class RootController extends React.Component { openIssueishDialog = () => { const dialogRequest = dialogRequests.issueish(); - dialogRequest.onAccept(async url => { + dialogRequest.onProgressingAccept(async url => { await openIssueishItem(url, { workspace: this.props.workspace, workdir: this.props.repository.getWorkingDirectoryPath(), diff --git a/lib/views/open-issueish-dialog.js b/lib/views/open-issueish-dialog.js index 7c11e66259..a874ae6f59 100644 --- a/lib/views/open-issueish-dialog.js +++ b/lib/views/open-issueish-dialog.js @@ -3,8 +3,9 @@ import PropTypes from 'prop-types'; import {TextBuffer} from 'atom'; import Commands, {Command} from '../atom/commands'; -import {AtomTextEditor} from '../atom/atom-text-editor'; +import AtomTextEditor from '../atom/atom-text-editor'; import IssueishDetailItem from '../items/issueish-detail-item'; +import AutoFocus from '../autofocus'; import {addEvent} from '../reporter-proxy'; const ISSUEISH_URL_REGEX = /^(?:https?:\/\/)?(github.com)\/([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)/; @@ -33,6 +34,8 @@ export default class OpenIssueishDialog extends React.Component { }; this.sub = this.url.onDidChange(this.didChangeURL); + + this.autofocus = new AutoFocus(); } componentWillUnmount() { @@ -49,7 +52,12 @@ export default class OpenIssueishDialog extends React.Component {