From ab4ea9aa4719d13d4476379c4bc869eac07a6b92 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Wed, 23 Nov 2016 11:33:51 -0800 Subject: [PATCH 01/47] WIP: move main package code to GitPackageController --- lib/controllers/git-package-controller.js | 108 ++++++++++++++++++++++ lib/github-package.js | 43 ++++++--- 2 files changed, 140 insertions(+), 11 deletions(-) create mode 100644 lib/controllers/git-package-controller.js diff --git a/lib/controllers/git-package-controller.js b/lib/controllers/git-package-controller.js new file mode 100644 index 0000000000..e890952592 --- /dev/null +++ b/lib/controllers/git-package-controller.js @@ -0,0 +1,108 @@ +/** @babel */ + +import {CompositeDisposable} from 'atom' + +import React from 'react' + +import StatusBarTileController from './status-bar-tile-controller' + + +const ReactStatusBarTileController = EtchReactAdapter(StatusBarTileController) + +function EtchReactAdapter (EtchComponent) { + return class EtchWrapper extends React.Component { + componentDidMount () { + this.component = new EtchComponent(this.props) + this.container.appendChild(this.component.element) + } + + componentWillReceiveProps (newProps) { + this.component.update(newProps) + } + + shouldComponentUpdate () { + return false + } + + render () { + const {reactWrapperProps} = this.props + const ReactWrapperType = this.props.reactWrapperType + return this.container = c} /> + } + + componentWillUnmount () { + this.container.removeChild(this.component.element) + this.component.destroy() + } + } +} + +EtchReactAdapter.propTypes = { + reactWrapperType: React.PropTypes.string, + reactWrapperProps: React.PropTypes.object +} + +EtchReactAdapter.defaultProps = { + reactWrapperType: 'div' +} + + +class StatusBar extends React.Component { + componentDidMount () { + this.consumeStatusBar(this.props) + } + + componentDidUpdate () { + this.consumeStatusBar(this.props) + } + + render () { + return
this.container = c}>{this.props.children}
+ } + + consumeStatusBar (props) { + if (this.tile) return + if (!props.statusBar) return + + if (!props.legacy) { + props.statusBar.disableGitInfoTile() + } + const componentElement = this.container.children[0] + this.tile = props.statusBar.addRightTile({item: componentElement, priority: -50}) + } +} + +StatusBar.proptTypes = { + statusBar: React.PropTypes.object, + legacy: React.PropTypes.bool +} + + +export default class GitPackageController extends React.Component { + constructor (props, context) { + super(props, context) + + this.subscriptions = new CompositeDisposable() + + // this.changeObserver = process.platform === 'linux' + // ? new WorkspaceChangeObserver(window, props.workspace) + // : new FileSystemChangeObserver() + } + + render () { + console.log('rendering') + + return ( +
+ + + +
+ ) + } +} diff --git a/lib/github-package.js b/lib/github-package.js index d2018484ea..0f23d67072 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -6,10 +6,13 @@ import FileSystemChangeObserver from './models/file-system-change-observer' import WorkspaceChangeObserver from './models/workspace-change-observer' import GitPanelController from './controllers/git-panel-controller' import FilePatchController from './controllers/file-patch-controller' -import StatusBarTileController from './controllers/status-bar-tile-controller' +import GitPackageController from './controllers/git-package-controller' import path from 'path' +import React from 'react' +import ReactDom from 'react-dom' + export default class GithubPackage { constructor (workspace, project, commandRegistry, notificationManager) { this.workspace = workspace @@ -32,11 +35,11 @@ export default class GithubPackage { didChangeAmending: this.didChangeAmending.bind(this), focusFilePatchView: this.focusFilePatchView.bind(this) }) - this.statusBarTileController = new StatusBarTileController({ - workspace, - repository: null, - toggleGitPanel: this.toggleGitPanel.bind(this) - }) + // this.statusBarTileController = new StatusBarTileController({ + // workspace, + // repository: null, + // toggleGitPanel: this.toggleGitPanel.bind(this) + // }) this.filePatchController = null } @@ -52,6 +55,21 @@ export default class GithubPackage { ) } + rerender () { + if (!this.element) { + this.element = document.createElement('div') + } + + ReactDom.render( + , this.element + ) + } + async deactivate () { await this.changeObserver.stop() } @@ -59,13 +77,15 @@ export default class GithubPackage { // FIXME: Combine these 2 methods into a single consumeStatusBar method after // Atom 1.13 is on stable channel consumeStatusBar_1_0 (statusBar) { - const tile = statusBar.addRightTile({item: this.statusBarTileController, priority: -50}) - this.subscriptions.add(new Disposable(() => tile.destroy())) + this.statusBar = statusBar + this.legacyStatusBar = true + this.rerender() } consumeStatusBar_1_1 (statusBar) { - statusBar.disableGitInfoTile() - this.consumeStatusBar_1_0(statusBar) + this.statusBar = statusBar + this.legacyStatusBar = false + this.rerender() } async didChangeProjectPaths () { @@ -186,8 +206,9 @@ export default class GithubPackage { async setActiveRepository (repository) { this.activeRepository = repository await this.changeObserver.setActiveRepository(repository) - await this.statusBarTileController.update({repository}) + // await this.statusBarTileController.update({repository}) await this.gitPanelController.update({repository}) + this.rerender() } projectDirectoryForPath (path) { From a0ac19b335d342f41054ddd03bbfa0dcd18a61a1 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Wed, 23 Nov 2016 12:56:53 -0800 Subject: [PATCH 02/47] :art: Package controller refactor: * git-package-controller.js -> github-package-controller.js * move static props into `static prop =` inside class --- ...roller.js => github-package-controller.js} | 32 +++++++++---------- lib/github-package.js | 4 +-- 2 files changed, 17 insertions(+), 19 deletions(-) rename lib/controllers/{git-package-controller.js => github-package-controller.js} (85%) diff --git a/lib/controllers/git-package-controller.js b/lib/controllers/github-package-controller.js similarity index 85% rename from lib/controllers/git-package-controller.js rename to lib/controllers/github-package-controller.js index e890952592..ddb1a26d63 100644 --- a/lib/controllers/git-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -11,6 +11,15 @@ const ReactStatusBarTileController = EtchReactAdapter(StatusBarTileController) function EtchReactAdapter (EtchComponent) { return class EtchWrapper extends React.Component { + static propTypes = { + reactWrapperType: React.PropTypes.string, + reactWrapperProps: React.PropTypes.object + } + + static defaultProps = { + reactWrapperType: 'div' + } + componentDidMount () { this.component = new EtchComponent(this.props) this.container.appendChild(this.component.element) @@ -37,17 +46,13 @@ function EtchReactAdapter (EtchComponent) { } } -EtchReactAdapter.propTypes = { - reactWrapperType: React.PropTypes.string, - reactWrapperProps: React.PropTypes.object -} - -EtchReactAdapter.defaultProps = { - reactWrapperType: 'div' -} - class StatusBar extends React.Component { + static propTypes = { + statusBar: React.PropTypes.object, + legacy: React.PropTypes.bool + } + componentDidMount () { this.consumeStatusBar(this.props) } @@ -72,13 +77,8 @@ class StatusBar extends React.Component { } } -StatusBar.proptTypes = { - statusBar: React.PropTypes.object, - legacy: React.PropTypes.bool -} - -export default class GitPackageController extends React.Component { +export default class GithubPackageController extends React.Component { constructor (props, context) { super(props, context) @@ -90,8 +90,6 @@ export default class GitPackageController extends React.Component { } render () { - console.log('rendering') - return (
diff --git a/lib/github-package.js b/lib/github-package.js index 0f23d67072..8942cd4ecb 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -6,7 +6,7 @@ import FileSystemChangeObserver from './models/file-system-change-observer' import WorkspaceChangeObserver from './models/workspace-change-observer' import GitPanelController from './controllers/git-panel-controller' import FilePatchController from './controllers/file-patch-controller' -import GitPackageController from './controllers/git-package-controller' +import GithubPackageController from './controllers/github-package-controller' import path from 'path' @@ -61,7 +61,7 @@ export default class GithubPackage { } ReactDom.render( - Date: Wed, 23 Nov 2016 14:30:40 -0800 Subject: [PATCH 03/47] Replace EtchReactAdapter function with EtchWrapper component Previously, rendering an Etch component with React required creating a new class by passing the Etch component class to `EtchReactAdapter` and rendering what was returned with React. Instead, EtchReactAdapter allows you to specify which component to wrap and what props to pass it declaratively via JSX, and will manage the component lifecycle itself. --- lib/controllers/github-package-controller.js | 125 ++++++++++++++---- lib/controllers/status-bar-tile-controller.js | 3 +- 2 files changed, 98 insertions(+), 30 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index ddb1a26d63..2a224dd67e 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -7,42 +7,108 @@ import React from 'react' import StatusBarTileController from './status-bar-tile-controller' -const ReactStatusBarTileController = EtchReactAdapter(StatusBarTileController) +/** + * `EtchWrapper` is a React component that renders Etch components + * and correctly manages their lifecycles as the application progresses. + * + * + * + * + * + * The `type` property specifies the DOM node type to wrap around the + * Etch component's element, and defaults to 'div'. Any other props you + * pass to the wrapper component will be applied to the DOM node. + * + * The component takes a single JSX child, which describes the type and props + * of the Etch component to render. Any time this changes, the wrapper will + * update (or destroy and recreate) the Etch component as necessary. + * + * Note that the component cleans up its own DOM node, and calls + * `component.destroy(false)` (if your component has a `destroy` method) + * and you should pass the `false` as the second argument to + * `etch.destroy(this)` (e.g. `etch.destroy(this, false)`) inside your + * component instance. + */ +class EtchWrapper extends React.Component { + static propTypes = { + children: React.PropTypes.element.isRequired, + type: React.PropTypes.string, + } + + static defaultProps = { + type: 'div', + } -function EtchReactAdapter (EtchComponent) { - return class EtchWrapper extends React.Component { - static propTypes = { - reactWrapperType: React.PropTypes.string, - reactWrapperProps: React.PropTypes.object + componentDidMount () { + this.createComponent(this.getWrappedComponentDetails(this.props.children)) + } + + componentWillReceiveProps (newProps) { + const oldDetails = this.getWrappedComponentDetails(this.props.children) + const newDetails = this.getWrappedComponentDetails(newProps.children) + if (oldDetails.type !== newDetails.type) { + // The wrapped component type changed, so we need to destroy the old + // component and create a new one of the new type. + this.destroyComponent() + this.createComponent(newDetails) } + } + + componentDidUpdate (prevProps) { + const oldDetails = this.getWrappedComponentDetails(prevProps.children) + const newDetails = this.getWrappedComponentDetails(this.props.children) - static defaultProps = { - reactWrapperType: 'div' + if (oldDetails.type === newDetails.type) { + // We didn't change the wrapped (Etch) component type, + // so we need to update the instance with the new props. + this.updateComponent(this.getWrappedComponentDetails(this.props.children)) } - componentDidMount () { - this.component = new EtchComponent(this.props) + // If we just recreated our DOM node by changing the node type, we + // need to reattach the wrapped component's element. + if (!this.container.contains(this.component.element)) { this.container.appendChild(this.component.element) } + } - componentWillReceiveProps (newProps) { - this.component.update(newProps) - } + render () { + const Type = this.props.type + const {type, thing, children, ...props} = this.props + return this.container = c}>{thing} + } - shouldComponentUpdate () { - return false - } + componentWillUnmount () { + this.destroyComponent() + } - render () { - const {reactWrapperProps} = this.props - const ReactWrapperType = this.props.reactWrapperType - return this.container = c} /> - } + getWrappedComponentDetails (ourChildren) { + // e.g. Hi + const etchElement = React.Children.toArray(ourChildren)[0] + // etchElement === {type: EtchChild, props: {prop: 1, other: 2, children: 'Hi'}} + const {type, props} = etchElement + // type === EtchChild, props === {prop: 1, other: 2, children: 'Hi'} + const {children, ...remainingProps} = props + // children === 'Hi', remainingProps === {prop: 1, other: 2} + return {type, children, props: remainingProps} + } + + // Etch component interactions + + createComponent ({type, props, children}) { + this.component = new type(props, children) + this.container.appendChild(this.component.element) + } + + updateComponent ({props, children}) { + this.component.update(props, children) + } - componentWillUnmount () { + destroyComponent () { + if (this.container.contains(this.component.element)) { this.container.removeChild(this.component.element) - this.component.destroy() } + this.component.destroy && this.component.destroy(false) + delete this.component } } @@ -93,12 +159,13 @@ export default class GithubPackageController extends React.Component { return (
- + + +
) diff --git a/lib/controllers/status-bar-tile-controller.js b/lib/controllers/status-bar-tile-controller.js index 3518315c35..99ccfaacb4 100644 --- a/lib/controllers/status-bar-tile-controller.js +++ b/lib/controllers/status-bar-tile-controller.js @@ -191,9 +191,10 @@ export default class StatusBarTileController { this.showingPushPullMenuView = true } - destroy () { + destroy (removeDomNode) { this.branchTooltipDisposable && this.branchTooltipDisposable.dispose() this.pushPullTooltipDisposable && this.pushPullTooltipDisposable.dispose() this.repositoryObserver.destroy() + etch.destroy(this, removeDomNode) } } From 4131d190d101cfb34749a15f5538e8a81d8f9e5e Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Wed, 23 Nov 2016 14:42:14 -0800 Subject: [PATCH 04/47] :art: --- lib/controllers/github-package-controller.js | 13 ++++++++++++- lib/github-package.js | 4 ++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index 2a224dd67e..a9fbcfd3a3 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -116,7 +116,7 @@ class EtchWrapper extends React.Component { class StatusBar extends React.Component { static propTypes = { statusBar: React.PropTypes.object, - legacy: React.PropTypes.bool + legacy: React.PropTypes.bool, } componentDidMount () { @@ -145,6 +145,17 @@ class StatusBar extends React.Component { export default class GithubPackageController extends React.Component { + static propTypes = { + workspace: React.PropTypes.object.isRequired, + repository: React.PropTypes.object, + statusBar: React.PropTypes.object, + legacyStatusBar: React.PropTypes.bool, + } + + static defaultProps = { + legacyStatusBar: false, + } + constructor (props, context) { super(props, context) diff --git a/lib/github-package.js b/lib/github-package.js index 8942cd4ecb..295feeadf5 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -62,10 +62,10 @@ export default class GithubPackage { ReactDom.render( , this.element ) } From 2a35d86519146654a2c8e9d5dbfdc4aa74e5f4cd Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Wed, 23 Nov 2016 14:42:25 -0800 Subject: [PATCH 05/47] Render on initial package activation --- lib/github-package.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/github-package.js b/lib/github-package.js index 295feeadf5..85eb9142f7 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -53,6 +53,7 @@ export default class GithubPackage { this.commandRegistry.add('atom-workspace', {'git:toggle-git-panel': this.toggleGitPanel.bind(this)}), this.commandRegistry.add('atom-workspace', {'git:focus-git-panel': this.focusGitPanel.bind(this)}) ) + this.rerender() } rerender () { From 4a6f7ba65495403a69ba749fd56ce940d9838a56 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 05:32:55 -0800 Subject: [PATCH 06/47] Add reattachDomNode prop to EtchWrapper component --- lib/controllers/github-package-controller.js | 35 ++++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index a9fbcfd3a3..a01e5ac5fd 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -11,7 +11,7 @@ import StatusBarTileController from './status-bar-tile-controller' * `EtchWrapper` is a React component that renders Etch components * and correctly manages their lifecycles as the application progresses. * - * + * * * * @@ -19,6 +19,12 @@ import StatusBarTileController from './status-bar-tile-controller' * Etch component's element, and defaults to 'div'. Any other props you * pass to the wrapper component will be applied to the DOM node. * + * `reattachDomNode` determines whether or not to place the wrapped component + * element back in the React component's DOM node if we find it's missing; + * this could happen due to changing the `type` property. If you pass the + * wrapped component element into a method that moves the element, you should + * specify `false` for this option. + * * The component takes a single JSX child, which describes the type and props * of the Etch component to render. Any time this changes, the wrapper will * update (or destroy and recreate) the Etch component as necessary. @@ -28,15 +34,21 @@ import StatusBarTileController from './status-bar-tile-controller' * and you should pass the `false` as the second argument to * `etch.destroy(this)` (e.g. `etch.destroy(this, false)`) inside your * component instance. + * + * The component instance is available at `this.getWrappedComponent` if you need + * to call methods on it from the outside (though you should really consider + * setting a prop instead. ;) */ class EtchWrapper extends React.Component { static propTypes = { children: React.PropTypes.element.isRequired, type: React.PropTypes.string, + reattachDomNode: React.PropTypes.bool, } static defaultProps = { type: 'div', + reattachDomNode: true, } componentDidMount () { @@ -54,27 +66,27 @@ class EtchWrapper extends React.Component { } } - componentDidUpdate (prevProps) { + async componentDidUpdate (prevProps) { const oldDetails = this.getWrappedComponentDetails(prevProps.children) const newDetails = this.getWrappedComponentDetails(this.props.children) if (oldDetails.type === newDetails.type) { // We didn't change the wrapped (Etch) component type, // so we need to update the instance with the new props. - this.updateComponent(this.getWrappedComponentDetails(this.props.children)) + await this.updateComponent(this.getWrappedComponentDetails(this.props.children)) } // If we just recreated our DOM node by changing the node type, we // need to reattach the wrapped component's element. - if (!this.container.contains(this.component.element)) { + if (this.props.reattachDomNode && !this.container.contains(this.component.element)) { this.container.appendChild(this.component.element) } } render () { const Type = this.props.type - const {type, thing, children, ...props} = this.props - return this.container = c}>{thing} + const {type, children, reattachDomNode, ...props} = this.props + return this.container = c} /> } componentWillUnmount () { @@ -92,15 +104,24 @@ class EtchWrapper extends React.Component { return {type, children, props: remainingProps} } + // For compatability with Atom's ViewProvider + getElement () { + return this.container + } + // Etch component interactions + getWrappedComponent () { + return this.component + } + createComponent ({type, props, children}) { this.component = new type(props, children) this.container.appendChild(this.component.element) } updateComponent ({props, children}) { - this.component.update(props, children) + return this.component.update(props, children) } destroyComponent () { From cafc9d48c33d10be6d271e29d4b18cb2f9120e0a Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 05:39:21 -0800 Subject: [PATCH 07/47] Add Portal component --- lib/controllers/github-package-controller.js | 69 ++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index a01e5ac5fd..a5dbd866f6 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -3,6 +3,7 @@ import {CompositeDisposable} from 'atom' import React from 'react' +import ReactDom from 'react-dom' import StatusBarTileController from './status-bar-tile-controller' @@ -165,6 +166,74 @@ class StatusBar extends React.Component { } +/** + * `Portal` is a mechanism for rendering a React subtree at a different place + * in the DOM. + * + * + * + * + * + * Given the above example, there will be a span with the class "portal-class" + * created and appended to the document body, and then `` will be + * rendered into it. Note that this uses `unstable_renderSubtreeIntoContainer` + * to preserve context in the subtree. + * + * `getComponent()` allows access to the rendered subtree instance + * (`Stuff` in the example above). + * + * Pass `false` (the default) to `appendNode` to skip adding the node to the + * DOM. `type` defaults to "div" and `className` defaults to + * "react-atom-portal". + */ +class Portal extends React.Component { + static propTypes = { + type: React.PropTypes.string, + className: React.PropTypes.string, + appendNode: React.PropTypes.bool, + } + + static defaultProps = { + type: 'div', + className: 'react-atom-portal', + appendNode: false, + } + + componentDidMount () { + this.node = document.createElement(this.props.type) + this.node.className = this.props.className + if (this.props.appendNode) { + document.body.appendChild(this.node) + } + this.renderPortal(this.props) + } + + componentWillReceiveProps (newProps) { + this.renderPortal(newProps) + } + + componentWillUnmount () { + ReactDom.unmountComponentAtNode(this.node) + if (this.props.appendNode) { + document.body.removeChild(this.node) + } + } + + renderPortal (props) { + this.portal = ReactDom.unstable_renderSubtreeIntoContainer( + this, props.children, this.node + ) + } + + render () { + return null + } + + getComponent () { + return this.portal + } +} + export default class GithubPackageController extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, From 1303b1daa6fc46b4796c73ce682eda48ccf58d46 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 05:44:53 -0800 Subject: [PATCH 08/47] Add PaneItem component --- lib/controllers/github-package-controller.js | 73 ++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index a5dbd866f6..198eefd365 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -234,6 +234,79 @@ class Portal extends React.Component { } } +/** + * `PaneItem` adds its child to the current Atom pane when rendered. + * When the pane is closed, the component's `onDidCloseItem` is called. + * You should use this callback to set state so that the `PaneItem` is no + * longer rendered; you will get an error in your console if you forget. + * + * The optional `getItem` property is a function that gets passed the wrapped + * React component instance, and you should return a valid Atom item. + * + * Unmounting the component when the item is open will close the item. + */ +class PaneItem extends React.Component { + static propTypes = { + workspace: React.PropTypes.object.isRequired, + getItem: React.PropTypes.func, + onDidCloseItem: React.PropTypes.func, + } + + static defaultProps = { + getItem: (c) => c, + onDidCloseItem: () => {}, + } + + componentDidMount () { + this.setupPaneItem(this.props) + } + + componentWillReceiveProps () { + if (this.didCloseItem) { + console.error( + `Unexpected update in \`PaneItem\`: the contained item has been closed` + ) + } + } + + render () { + return this.portal = c}>{this.props.children} + } + + setupPaneItem (props) { + if (this.paneItem) return + + const item = props.getItem(this.portal.getComponent()) + this.paneItem = this.props.workspace.getActivePane().addItem(item) + this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => { + if (item === this.paneItem) { + this.didCloseItem = true + this.props.onDidCloseItem() + } + }) + } + + componentWillUnmount () { + this.subscriptions && this.subscriptions.dispose() + if (this.paneItem && !this.didCloseItem) { + this.paneItem.destroy() + } + } + + activate () { + setTimeout(() => { + if (!this.paneItem) return + + const pane = this.props.workspace.paneForItem(this.paneItem) + if (pane) { + pane.activateItem(this.paneItem) + } else { + throw new Error(`Cannot find pane for item in \`PaneItem#activate\``) + } + }) + } +} + export default class GithubPackageController extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, From 252d6a035e4963a6dfdf7a464a9ec777ef569398 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 06:00:21 -0800 Subject: [PATCH 09/47] Add Panel component --- lib/controllers/github-package-controller.js | 52 ++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index 198eefd365..0ae750de0a 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -166,6 +166,58 @@ class StatusBar extends React.Component { } +/** + * `Panel` renders a React component into an Atom panel. Specify the + * location via the `location` prop, and any additional options to the + * `addXPanel` method in the `options` prop. + * + * You can get the underlying Atom panel via `getPanel()`, but you should + * consider controlling the panel via React and the Panel component instead. + */ +class Panel extends React.Component { + static propTypes = { + workspace: React.PropTypes.object.isRequired, + location: React.PropTypes.oneOf([ + 'top', 'bottom', 'left', 'right', 'header', 'footer', 'modal' + ]).isRequired, + options: React.PropTypes.object, + } + + static defaultProps = { + options: {}, + } + + componentDidMount () { + this.setupPanel(this.props) + } + + render () { + return this.portal = c}>{this.props.children} + } + + setupPanel (props) { + if (this.panel) return + + const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) + const methodName = `add${location}Panel` + + const item = this.portal.getComponent() + const options = Object.assign({}, this.props.options, { item }) + this.panel = this.props.workspace[methodName](options) + } + + componentWillUnmount () { + if (this.panel) { + this.panel.destroy() + } + } + + getPanel () { + return this.panel + } +} + + /** * `Portal` is a mechanism for rendering a React subtree at a different place * in the DOM. From f5f0f22957a2d2eb09ab09c3590780cde27fabd7 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 06:06:37 -0800 Subject: [PATCH 10/47] Render GitPanelController with React --- lib/controllers/github-package-controller.js | 29 ++++++++++++++++++ lib/github-package.js | 32 +++++++------------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index 0ae750de0a..b8451a8637 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -5,6 +5,7 @@ import {CompositeDisposable} from 'atom' import React from 'react' import ReactDom from 'react-dom' +import GitPanelController from './git-panel-controller' import StatusBarTileController from './status-bar-tile-controller' @@ -362,6 +363,8 @@ class PaneItem extends React.Component { export default class GithubPackageController extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, + commandRegistry: React.PropTypes.object.isRequired, + notificationManager: React.PropTypes.object.isRequired, repository: React.PropTypes.object, statusBar: React.PropTypes.object, legacyStatusBar: React.PropTypes.bool, @@ -393,7 +396,33 @@ export default class GithubPackageController extends React.Component { /> + {this.props.gitPanelActive ? this.renderGitPanel() : null}
) } + + renderGitPanel () { + return ( + + this.gitPanelController = c}> + + + + ) + } + async showFilePatchForPath (filePath, stagingStatus, {activate, amending} = {}) { + } + + showMergeConflictFileForPath() {} + didChangeAmending() {} + focusFilePatchView() {} } diff --git a/lib/github-package.js b/lib/github-package.js index 85eb9142f7..d8e851d3bc 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -25,16 +25,6 @@ export default class GithubPackage { this.activeRepository = null this.repositoriesByProjectDirectory = new Map() this.subscriptions = new CompositeDisposable() - this.gitPanelController = new GitPanelController({ - repository: null, - workspace, - commandRegistry, - notificationManager, - didSelectFilePath: this.showFilePatchForPath.bind(this), - didSelectMergeConflictFile: this.showMergeConflictFileForPath.bind(this), - didChangeAmending: this.didChangeAmending.bind(this), - focusFilePatchView: this.focusFilePatchView.bind(this) - }) // this.statusBarTileController = new StatusBarTileController({ // workspace, // repository: null, @@ -64,9 +54,12 @@ export default class GithubPackage { ReactDom.render( , this.element ) } @@ -152,17 +145,17 @@ export default class GithubPackage { } toggleGitPanel () { - if (this.workspaceGitPanel) { - this.workspaceGitPanel.destroy() - this.workspaceGitPanel = null - } else { - this.focusGitPanel() - } + this.gitPanelActive = !this.gitPanelActive + this.rerender() } focusGitPanel () { - this.workspaceGitPanel = this.workspace.addRightPanel({item: this.gitPanelController}) - this.gitPanelController.focus() + if (!this.gitPanelActive) { + this.toggleGitPanel() + } + // TODO: why doesn't a setTimeout of 0 or even 100 work here? + // Suspect it has to do with the command palette closing... + setTimeout(() => this.controller.gitPanelController.component.focus(), 200) } getActiveRepository () { @@ -208,7 +201,6 @@ export default class GithubPackage { this.activeRepository = repository await this.changeObserver.setActiveRepository(repository) // await this.statusBarTileController.update({repository}) - await this.gitPanelController.update({repository}) this.rerender() } @@ -242,8 +234,6 @@ export default class GithubPackage { destroy () { this.subscriptions.dispose() if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() - this.gitPanelController.destroy() - this.statusBarTileController.destroy() if (this.filePatchController) this.filePatchController.destroy() } } From b6d5867409a0da7ab68445a8ce163d1cf9405e72 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 06:08:40 -0800 Subject: [PATCH 11/47] Basic FilePatchController rendering --- lib/controllers/github-package-controller.js | 60 ++++++++++++++++++++ lib/github-package.js | 27 --------- 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index b8451a8637..32ea5a9056 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -5,6 +5,7 @@ import {CompositeDisposable} from 'atom' import React from 'react' import ReactDom from 'react-dom' +import FilePatchController from './file-patch-controller' import GitPanelController from './git-panel-controller' import StatusBarTileController from './status-bar-tile-controller' @@ -376,7 +377,16 @@ export default class GithubPackageController extends React.Component { constructor (props, context) { super(props, context) + this.state = { + filePath: null, + filePatch: null, + stagingStatus: null, + amending: null, + } + this.showFilePatchForPath = this.showFilePatchForPath.bind(this) + this.showMergeConflictFileForPath = this.showMergeConflictFileForPath.bind(this) + this.onRepoRefresh = this.onRepoRefresh.bind(this) this.subscriptions = new CompositeDisposable() // this.changeObserver = process.platform === 'linux' @@ -397,6 +407,7 @@ export default class GithubPackageController extends React.Component { {this.props.gitPanelActive ? this.renderGitPanel() : null} + {this.state.filePatch ? this.renderFilePatchController() : null} ) } @@ -419,7 +430,56 @@ export default class GithubPackageController extends React.Component { ) } + + renderFilePatchController () { + return ( + container.getWrappedComponent()} + ref={c => this.filePatchControllerPane = c} + onDidCloseItem={() => this.setState({filePatch: null})} + > + this.filePatchController = c} reattachDomNode={false}> + + + + ) + } + async showFilePatchForPath (filePath, stagingStatus, {activate, amending} = {}) { + const repository = this.props.repository + if (!repository) return + + const filePatch = await repository.getFilePatchForPath(filePath, {staged: stagingStatus === 'staged', amending}) + if (filePatch) { + this.setState({ + filePath: filePath, + filePatch: filePatch, + stagingStatus: stagingStatus, + amending: amending, + }, () => { + // TODO: can be better done w/ a prop? + if (activate) { + this.filePatchControllerPane.activate() + } + }) + } else { + this.setState({ + filePath: null, + filePatch: null, + stagingStatus: null, + amending: null, + }) + } + } + + onRepoRefresh () { + this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: this.state.amending}) } showMergeConflictFileForPath() {} diff --git a/lib/github-package.js b/lib/github-package.js index d8e851d3bc..c6c5cd75b7 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -97,33 +97,6 @@ export default class GithubPackage { } } - async showFilePatchForPath (filePath, stagingStatus, {activate, amending} = {}) { - const repository = this.getActiveRepository() - const filePatch = await repository.getFilePatchForPath(filePath, {staged: stagingStatus === 'staged', amending}) - if (filePatch) { - let containingPane - const onRepoRefresh = () => { - this.showFilePatchForPath(filePath, stagingStatus, {amending}) - } - if (this.filePatchController) { - this.filePatchController.update({filePatch, repository, stagingStatus, onRepoRefresh}) - containingPane = this.workspace.paneForItem(this.filePatchController) - } else { - this.filePatchController = new FilePatchController({filePatch, repository, stagingStatus, onRepoRefresh}) - this.filePatchController.onDidDestroy(() => { this.filePatchController = null }) - containingPane = this.workspace.getActivePane() - } - if (activate) { - containingPane.activateItem(this.filePatchController) - } - } else { - if (this.filePatchController) { - this.filePatchController.destroy() - this.filePatchController = null - } - } - } - focusFilePatchView () { this.filePatchController.refs.filePatchView.element.focus() } From 45dce22cff07303ae02e578158b52ac004155618 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 06:10:53 -0800 Subject: [PATCH 12/47] Tear down React app on package deactivation --- lib/github-package.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/github-package.js b/lib/github-package.js index c6c5cd75b7..e0de03e852 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -208,5 +208,9 @@ export default class GithubPackage { this.subscriptions.dispose() if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() if (this.filePatchController) this.filePatchController.destroy() + if (this.element) { + ReactDom.unmountComponentAtNode(this.element) + delete this.element + } } } From ffea917e826bd1bf7efb14c5c8ff8a7feeb3e574 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 06:20:51 -0800 Subject: [PATCH 13/47] :art: Extract Atom/Etch helper components into separate files --- lib/controllers/github-package-controller.js | 356 +------------------ lib/views/etch-wrapper.js | 129 +++++++ lib/views/pane-item.js | 78 ++++ lib/views/panel.js | 58 +++ lib/views/portal.js | 72 ++++ lib/views/status-bar.js | 34 ++ 6 files changed, 375 insertions(+), 352 deletions(-) create mode 100644 lib/views/etch-wrapper.js create mode 100644 lib/views/pane-item.js create mode 100644 lib/views/panel.js create mode 100644 lib/views/portal.js create mode 100644 lib/views/status-bar.js diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index 32ea5a9056..e84f9decd4 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -5,362 +5,14 @@ import {CompositeDisposable} from 'atom' import React from 'react' import ReactDom from 'react-dom' +import EtchWrapper from '../views/etch-wrapper' +import StatusBar from '../views/status-bar' +import Panel from '../views/panel' +import PaneItem from '../views/pane-item' import FilePatchController from './file-patch-controller' import GitPanelController from './git-panel-controller' import StatusBarTileController from './status-bar-tile-controller' - -/** - * `EtchWrapper` is a React component that renders Etch components - * and correctly manages their lifecycles as the application progresses. - * - * - * - * - * - * The `type` property specifies the DOM node type to wrap around the - * Etch component's element, and defaults to 'div'. Any other props you - * pass to the wrapper component will be applied to the DOM node. - * - * `reattachDomNode` determines whether or not to place the wrapped component - * element back in the React component's DOM node if we find it's missing; - * this could happen due to changing the `type` property. If you pass the - * wrapped component element into a method that moves the element, you should - * specify `false` for this option. - * - * The component takes a single JSX child, which describes the type and props - * of the Etch component to render. Any time this changes, the wrapper will - * update (or destroy and recreate) the Etch component as necessary. - * - * Note that the component cleans up its own DOM node, and calls - * `component.destroy(false)` (if your component has a `destroy` method) - * and you should pass the `false` as the second argument to - * `etch.destroy(this)` (e.g. `etch.destroy(this, false)`) inside your - * component instance. - * - * The component instance is available at `this.getWrappedComponent` if you need - * to call methods on it from the outside (though you should really consider - * setting a prop instead. ;) - */ -class EtchWrapper extends React.Component { - static propTypes = { - children: React.PropTypes.element.isRequired, - type: React.PropTypes.string, - reattachDomNode: React.PropTypes.bool, - } - - static defaultProps = { - type: 'div', - reattachDomNode: true, - } - - componentDidMount () { - this.createComponent(this.getWrappedComponentDetails(this.props.children)) - } - - componentWillReceiveProps (newProps) { - const oldDetails = this.getWrappedComponentDetails(this.props.children) - const newDetails = this.getWrappedComponentDetails(newProps.children) - if (oldDetails.type !== newDetails.type) { - // The wrapped component type changed, so we need to destroy the old - // component and create a new one of the new type. - this.destroyComponent() - this.createComponent(newDetails) - } - } - - async componentDidUpdate (prevProps) { - const oldDetails = this.getWrappedComponentDetails(prevProps.children) - const newDetails = this.getWrappedComponentDetails(this.props.children) - - if (oldDetails.type === newDetails.type) { - // We didn't change the wrapped (Etch) component type, - // so we need to update the instance with the new props. - await this.updateComponent(this.getWrappedComponentDetails(this.props.children)) - } - - // If we just recreated our DOM node by changing the node type, we - // need to reattach the wrapped component's element. - if (this.props.reattachDomNode && !this.container.contains(this.component.element)) { - this.container.appendChild(this.component.element) - } - } - - render () { - const Type = this.props.type - const {type, children, reattachDomNode, ...props} = this.props - return this.container = c} /> - } - - componentWillUnmount () { - this.destroyComponent() - } - - getWrappedComponentDetails (ourChildren) { - // e.g. Hi - const etchElement = React.Children.toArray(ourChildren)[0] - // etchElement === {type: EtchChild, props: {prop: 1, other: 2, children: 'Hi'}} - const {type, props} = etchElement - // type === EtchChild, props === {prop: 1, other: 2, children: 'Hi'} - const {children, ...remainingProps} = props - // children === 'Hi', remainingProps === {prop: 1, other: 2} - return {type, children, props: remainingProps} - } - - // For compatability with Atom's ViewProvider - getElement () { - return this.container - } - - // Etch component interactions - - getWrappedComponent () { - return this.component - } - - createComponent ({type, props, children}) { - this.component = new type(props, children) - this.container.appendChild(this.component.element) - } - - updateComponent ({props, children}) { - return this.component.update(props, children) - } - - destroyComponent () { - if (this.container.contains(this.component.element)) { - this.container.removeChild(this.component.element) - } - this.component.destroy && this.component.destroy(false) - delete this.component - } -} - - -class StatusBar extends React.Component { - static propTypes = { - statusBar: React.PropTypes.object, - legacy: React.PropTypes.bool, - } - - componentDidMount () { - this.consumeStatusBar(this.props) - } - - componentDidUpdate () { - this.consumeStatusBar(this.props) - } - - render () { - return
this.container = c}>{this.props.children}
- } - - consumeStatusBar (props) { - if (this.tile) return - if (!props.statusBar) return - - if (!props.legacy) { - props.statusBar.disableGitInfoTile() - } - const componentElement = this.container.children[0] - this.tile = props.statusBar.addRightTile({item: componentElement, priority: -50}) - } -} - - -/** - * `Panel` renders a React component into an Atom panel. Specify the - * location via the `location` prop, and any additional options to the - * `addXPanel` method in the `options` prop. - * - * You can get the underlying Atom panel via `getPanel()`, but you should - * consider controlling the panel via React and the Panel component instead. - */ -class Panel extends React.Component { - static propTypes = { - workspace: React.PropTypes.object.isRequired, - location: React.PropTypes.oneOf([ - 'top', 'bottom', 'left', 'right', 'header', 'footer', 'modal' - ]).isRequired, - options: React.PropTypes.object, - } - - static defaultProps = { - options: {}, - } - - componentDidMount () { - this.setupPanel(this.props) - } - - render () { - return this.portal = c}>{this.props.children} - } - - setupPanel (props) { - if (this.panel) return - - const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) - const methodName = `add${location}Panel` - - const item = this.portal.getComponent() - const options = Object.assign({}, this.props.options, { item }) - this.panel = this.props.workspace[methodName](options) - } - - componentWillUnmount () { - if (this.panel) { - this.panel.destroy() - } - } - - getPanel () { - return this.panel - } -} - - -/** - * `Portal` is a mechanism for rendering a React subtree at a different place - * in the DOM. - * - * - * - * - * - * Given the above example, there will be a span with the class "portal-class" - * created and appended to the document body, and then `` will be - * rendered into it. Note that this uses `unstable_renderSubtreeIntoContainer` - * to preserve context in the subtree. - * - * `getComponent()` allows access to the rendered subtree instance - * (`Stuff` in the example above). - * - * Pass `false` (the default) to `appendNode` to skip adding the node to the - * DOM. `type` defaults to "div" and `className` defaults to - * "react-atom-portal". - */ -class Portal extends React.Component { - static propTypes = { - type: React.PropTypes.string, - className: React.PropTypes.string, - appendNode: React.PropTypes.bool, - } - - static defaultProps = { - type: 'div', - className: 'react-atom-portal', - appendNode: false, - } - - componentDidMount () { - this.node = document.createElement(this.props.type) - this.node.className = this.props.className - if (this.props.appendNode) { - document.body.appendChild(this.node) - } - this.renderPortal(this.props) - } - - componentWillReceiveProps (newProps) { - this.renderPortal(newProps) - } - - componentWillUnmount () { - ReactDom.unmountComponentAtNode(this.node) - if (this.props.appendNode) { - document.body.removeChild(this.node) - } - } - - renderPortal (props) { - this.portal = ReactDom.unstable_renderSubtreeIntoContainer( - this, props.children, this.node - ) - } - - render () { - return null - } - - getComponent () { - return this.portal - } -} - -/** - * `PaneItem` adds its child to the current Atom pane when rendered. - * When the pane is closed, the component's `onDidCloseItem` is called. - * You should use this callback to set state so that the `PaneItem` is no - * longer rendered; you will get an error in your console if you forget. - * - * The optional `getItem` property is a function that gets passed the wrapped - * React component instance, and you should return a valid Atom item. - * - * Unmounting the component when the item is open will close the item. - */ -class PaneItem extends React.Component { - static propTypes = { - workspace: React.PropTypes.object.isRequired, - getItem: React.PropTypes.func, - onDidCloseItem: React.PropTypes.func, - } - - static defaultProps = { - getItem: (c) => c, - onDidCloseItem: () => {}, - } - - componentDidMount () { - this.setupPaneItem(this.props) - } - - componentWillReceiveProps () { - if (this.didCloseItem) { - console.error( - `Unexpected update in \`PaneItem\`: the contained item has been closed` - ) - } - } - - render () { - return this.portal = c}>{this.props.children} - } - - setupPaneItem (props) { - if (this.paneItem) return - - const item = props.getItem(this.portal.getComponent()) - this.paneItem = this.props.workspace.getActivePane().addItem(item) - this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => { - if (item === this.paneItem) { - this.didCloseItem = true - this.props.onDidCloseItem() - } - }) - } - - componentWillUnmount () { - this.subscriptions && this.subscriptions.dispose() - if (this.paneItem && !this.didCloseItem) { - this.paneItem.destroy() - } - } - - activate () { - setTimeout(() => { - if (!this.paneItem) return - - const pane = this.props.workspace.paneForItem(this.paneItem) - if (pane) { - pane.activateItem(this.paneItem) - } else { - throw new Error(`Cannot find pane for item in \`PaneItem#activate\``) - } - }) - } -} - export default class GithubPackageController extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, diff --git a/lib/views/etch-wrapper.js b/lib/views/etch-wrapper.js new file mode 100644 index 0000000000..6fcc805878 --- /dev/null +++ b/lib/views/etch-wrapper.js @@ -0,0 +1,129 @@ +/** @babel */ + +import React from 'react' + +/** + * `EtchWrapper` is a React component that renders Etch components + * and correctly manages their lifecycles as the application progresses. + * + * + * + * + * + * The `type` property specifies the DOM node type to wrap around the + * Etch component's element, and defaults to 'div'. Any other props you + * pass to the wrapper component will be applied to the DOM node. + * + * `reattachDomNode` determines whether or not to place the wrapped component + * element back in the React component's DOM node if we find it's missing; + * this could happen due to changing the `type` property. If you pass the + * wrapped component element into a method that moves the element, you should + * specify `false` for this option. + * + * The component takes a single JSX child, which describes the type and props + * of the Etch component to render. Any time this changes, the wrapper will + * update (or destroy and recreate) the Etch component as necessary. + * + * Note that the component cleans up its own DOM node, and calls + * `component.destroy(false)` (if your component has a `destroy` method) + * and you should pass the `false` as the second argument to + * `etch.destroy(this)` (e.g. `etch.destroy(this, false)`) inside your + * component instance. + * + * The component instance is available at `this.getWrappedComponent` if you need + * to call methods on it from the outside (though you should really consider + * setting a prop instead. ;) + */ +export default class EtchWrapper extends React.Component { + static propTypes = { + children: React.PropTypes.element.isRequired, + type: React.PropTypes.string, + reattachDomNode: React.PropTypes.bool, + } + + static defaultProps = { + type: 'div', + reattachDomNode: true, + } + + componentDidMount () { + this.createComponent(this.getWrappedComponentDetails(this.props.children)) + } + + componentWillReceiveProps (newProps) { + const oldDetails = this.getWrappedComponentDetails(this.props.children) + const newDetails = this.getWrappedComponentDetails(newProps.children) + if (oldDetails.type !== newDetails.type) { + // The wrapped component type changed, so we need to destroy the old + // component and create a new one of the new type. + this.destroyComponent() + this.createComponent(newDetails) + } + } + + async componentDidUpdate (prevProps) { + const oldDetails = this.getWrappedComponentDetails(prevProps.children) + const newDetails = this.getWrappedComponentDetails(this.props.children) + + if (oldDetails.type === newDetails.type) { + // We didn't change the wrapped (Etch) component type, + // so we need to update the instance with the new props. + await this.updateComponent(this.getWrappedComponentDetails(this.props.children)) + } + + // If we just recreated our DOM node by changing the node type, we + // need to reattach the wrapped component's element. + if (this.props.reattachDomNode && !this.container.contains(this.component.element)) { + this.container.appendChild(this.component.element) + } + } + + render () { + const Type = this.props.type + const {type, children, reattachDomNode, ...props} = this.props + return this.container = c} /> + } + + componentWillUnmount () { + this.destroyComponent() + } + + getWrappedComponentDetails (ourChildren) { + // e.g. Hi + const etchElement = React.Children.toArray(ourChildren)[0] + // etchElement === {type: EtchChild, props: {prop: 1, other: 2, children: 'Hi'}} + const {type, props} = etchElement + // type === EtchChild, props === {prop: 1, other: 2, children: 'Hi'} + const {children, ...remainingProps} = props + // children === 'Hi', remainingProps === {prop: 1, other: 2} + return {type, children, props: remainingProps} + } + + // For compatability with Atom's ViewProvider + getElement () { + return this.container + } + + // Etch component interactions + + getWrappedComponent () { + return this.component + } + + createComponent ({type, props, children}) { + this.component = new type(props, children) + this.container.appendChild(this.component.element) + } + + updateComponent ({props, children}) { + return this.component.update(props, children) + } + + destroyComponent () { + if (this.container.contains(this.component.element)) { + this.container.removeChild(this.component.element) + } + this.component.destroy && this.component.destroy(false) + delete this.component + } +} diff --git a/lib/views/pane-item.js b/lib/views/pane-item.js new file mode 100644 index 0000000000..c42653fe21 --- /dev/null +++ b/lib/views/pane-item.js @@ -0,0 +1,78 @@ +/** @babel */ + +import React from 'react' + +import Portal from './portal' + +/** + * `PaneItem` adds its child to the current Atom pane when rendered. + * When the pane is closed, the component's `onDidCloseItem` is called. + * You should use this callback to set state so that the `PaneItem` is no + * longer rendered; you will get an error in your console if you forget. + * + * The optional `getItem` property is a function that gets passed the wrapped + * React component instance, and you should return a valid Atom item. + * + * Unmounting the component when the item is open will close the item. + */ +export default class PaneItem extends React.Component { + static propTypes = { + workspace: React.PropTypes.object.isRequired, + getItem: React.PropTypes.func, + onDidCloseItem: React.PropTypes.func, + } + + static defaultProps = { + getItem: (c) => c, + onDidCloseItem: () => {}, + } + + componentDidMount () { + this.setupPaneItem(this.props) + } + + componentWillReceiveProps () { + if (this.didCloseItem) { + console.error( + `Unexpected update in \`PaneItem\`: the contained item has been closed` + ) + } + } + + render () { + return this.portal = c}>{this.props.children} + } + + setupPaneItem (props) { + if (this.paneItem) return + + const item = props.getItem(this.portal.getComponent()) + this.paneItem = this.props.workspace.getActivePane().addItem(item) + this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => { + if (item === this.paneItem) { + this.didCloseItem = true + this.props.onDidCloseItem() + } + }) + } + + componentWillUnmount () { + this.subscriptions && this.subscriptions.dispose() + if (this.paneItem && !this.didCloseItem) { + this.paneItem.destroy() + } + } + + activate () { + setTimeout(() => { + if (!this.paneItem) return + + const pane = this.props.workspace.paneForItem(this.paneItem) + if (pane) { + pane.activateItem(this.paneItem) + } else { + throw new Error(`Cannot find pane for item in \`PaneItem#activate\``) + } + }) + } +} diff --git a/lib/views/panel.js b/lib/views/panel.js new file mode 100644 index 0000000000..9842a68824 --- /dev/null +++ b/lib/views/panel.js @@ -0,0 +1,58 @@ +/** @babel */ + +import React from 'react' + +import Portal from './portal' + +/** + * `Panel` renders a React component into an Atom panel. Specify the + * location via the `location` prop, and any additional options to the + * `addXPanel` method in the `options` prop. + * + * You can get the underlying Atom panel via `getPanel()`, but you should + * consider controlling the panel via React and the Panel component instead. + */ +// TODO: all portals that wrap atom classes should watch for the underlying +// model to be mutated/destroyed and call callbacks appropriately +export default class Panel extends React.Component { + static propTypes = { + workspace: React.PropTypes.object.isRequired, + location: React.PropTypes.oneOf([ + 'top', 'bottom', 'left', 'right', 'header', 'footer', 'modal' + ]).isRequired, + options: React.PropTypes.object, + } + + static defaultProps = { + options: {}, + } + + componentDidMount () { + this.setupPanel(this.props) + } + + render () { + return this.portal = c}>{this.props.children} + } + + setupPanel (props) { + if (this.panel) return + + const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) + const methodName = `add${location}Panel` + + const item = this.portal.getComponent() + const options = Object.assign({}, this.props.options, { item }) + this.panel = this.props.workspace[methodName](options) + } + + componentWillUnmount () { + if (this.panel) { + this.panel.destroy() + } + } + + getPanel () { + return this.panel + } +} diff --git a/lib/views/portal.js b/lib/views/portal.js new file mode 100644 index 0000000000..9f9c570fb5 --- /dev/null +++ b/lib/views/portal.js @@ -0,0 +1,72 @@ +/** @babel */ + +import React from 'react' +import ReactDom from 'react-dom' + +/** + * `Portal` is a mechanism for rendering a React subtree at a different place + * in the DOM. + * + * + * + * + * + * Given the above example, there will be a span with the class "portal-class" + * created and appended to the document body, and then `` will be + * rendered into it. Note that this uses `unstable_renderSubtreeIntoContainer` + * to preserve context in the subtree. + * + * `getComponent()` allows access to the rendered subtree instance + * (`Stuff` in the example above). + * + * Pass `false` (the default) to `appendNode` to skip adding the node to the + * DOM. `type` defaults to "div" and `className` defaults to + * "react-atom-portal". + */ +export default class Portal extends React.Component { + static propTypes = { + type: React.PropTypes.string, + className: React.PropTypes.string, + appendNode: React.PropTypes.bool, + } + + static defaultProps = { + type: 'div', + className: 'react-atom-portal', + appendNode: false, + } + + componentDidMount () { + this.node = document.createElement(this.props.type) + this.node.className = this.props.className + if (this.props.appendNode) { + document.body.appendChild(this.node) + } + this.renderPortal(this.props) + } + + componentWillReceiveProps (newProps) { + this.renderPortal(newProps) + } + + componentWillUnmount () { + ReactDom.unmountComponentAtNode(this.node) + if (this.props.appendNode) { + document.body.removeChild(this.node) + } + } + + renderPortal (props) { + this.portal = ReactDom.unstable_renderSubtreeIntoContainer( + this, props.children, this.node + ) + } + + render () { + return null + } + + getComponent () { + return this.portal + } +} diff --git a/lib/views/status-bar.js b/lib/views/status-bar.js new file mode 100644 index 0000000000..1760a2fdd4 --- /dev/null +++ b/lib/views/status-bar.js @@ -0,0 +1,34 @@ +/** @babel */ + +import React from 'react' + +// TODO: updates, unmount +export default class StatusBar extends React.Component { + static propTypes = { + statusBar: React.PropTypes.object, + legacy: React.PropTypes.bool, + } + + componentDidMount () { + this.consumeStatusBar(this.props) + } + + componentDidUpdate () { + this.consumeStatusBar(this.props) + } + + render () { + return
this.container = c}>{this.props.children}
+ } + + consumeStatusBar (props) { + if (this.tile) return + if (!props.statusBar) return + + if (!props.legacy) { + props.statusBar.disableGitInfoTile() + } + const componentElement = this.container.children[0] + this.tile = props.statusBar.addRightTile({item: componentElement, priority: -50}) + } +} From 40dce6aee7cca44d1f928da0e016c1809d17fa27 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 12:58:20 -0800 Subject: [PATCH 14/47] Improve FilePatch rendering management --- lib/controllers/github-package-controller.js | 23 ++++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index e84f9decd4..c81222b373 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -13,6 +13,13 @@ import FilePatchController from './file-patch-controller' import GitPanelController from './git-panel-controller' import StatusBarTileController from './status-bar-tile-controller' +const nullFilePatchState = { + filePath: null, + filePatch: null, + stagingStatus: null, + amending: null, +} + export default class GithubPackageController extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, @@ -30,10 +37,7 @@ export default class GithubPackageController extends React.Component { constructor (props, context) { super(props, context) this.state = { - filePath: null, - filePatch: null, - stagingStatus: null, - amending: null, + ...nullFilePatchState } this.showFilePatchForPath = this.showFilePatchForPath.bind(this) @@ -59,7 +63,7 @@ export default class GithubPackageController extends React.Component { {this.props.gitPanelActive ? this.renderGitPanel() : null} - {this.state.filePatch ? this.renderFilePatchController() : null} + {(this.state.filePath && this.state.filePatch) ? this.renderFilePatchController() : null} ) } @@ -89,7 +93,7 @@ export default class GithubPackageController extends React.Component { workspace={this.props.workspace} getItem={container => container.getWrappedComponent()} ref={c => this.filePatchControllerPane = c} - onDidCloseItem={() => this.setState({filePatch: null})} + onDidCloseItem={() => this.setState({...nullFilePatchState})} > this.filePatchController = c} reattachDomNode={false}> Date: Fri, 25 Nov 2016 12:58:40 -0800 Subject: [PATCH 15/47] Remove unused comments --- lib/controllers/github-package-controller.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index c81222b373..c5d24db854 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -44,10 +44,6 @@ export default class GithubPackageController extends React.Component { this.showMergeConflictFileForPath = this.showMergeConflictFileForPath.bind(this) this.onRepoRefresh = this.onRepoRefresh.bind(this) this.subscriptions = new CompositeDisposable() - - // this.changeObserver = process.platform === 'linux' - // ? new WorkspaceChangeObserver(window, props.workspace) - // : new FileSystemChangeObserver() } render () { From 9ab386d37b924e3ae39b154da53a37a7c635c24b Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 13:07:35 -0800 Subject: [PATCH 16/47] Change up GitPanel focus logic --- lib/github-package.js | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index e0de03e852..58ada45708 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -4,7 +4,6 @@ import {CompositeDisposable, Disposable, File} from 'atom' import Repository from './models/repository' import FileSystemChangeObserver from './models/file-system-change-observer' import WorkspaceChangeObserver from './models/workspace-change-observer' -import GitPanelController from './controllers/git-panel-controller' import FilePatchController from './controllers/file-patch-controller' import GithubPackageController from './controllers/github-package-controller' @@ -25,11 +24,6 @@ export default class GithubPackage { this.activeRepository = null this.repositoriesByProjectDirectory = new Map() this.subscriptions = new CompositeDisposable() - // this.statusBarTileController = new StatusBarTileController({ - // workspace, - // repository: null, - // toggleGitPanel: this.toggleGitPanel.bind(this) - // }) this.filePatchController = null } @@ -46,13 +40,14 @@ export default class GithubPackage { this.rerender() } - rerender () { + rerender (callback = null) { if (!this.element) { this.element = document.createElement('div') } ReactDom.render( this.controller = c} workspace={this.workspace} commandRegistry={this.commandRegistry} notificationManager={this.notificationManager} @@ -60,7 +55,7 @@ export default class GithubPackage { statusBar={this.statusBar} legacyStatusBar={this.legacyStatusBar} gitPanelActive={this.gitPanelActive} - />, this.element + />, this.element, typeof callback === 'function' ? callback : null ) } @@ -117,18 +112,19 @@ export default class GithubPackage { } } - toggleGitPanel () { + toggleGitPanel (callback) { this.gitPanelActive = !this.gitPanelActive - this.rerender() + this.rerender(callback) } focusGitPanel () { if (!this.gitPanelActive) { - this.toggleGitPanel() + this.toggleGitPanel(() => { + // TODO: why doesn't a setTimeout of 0 or even 100 work here? + // Suspect it has to do with the command palette closing... + setTimeout(() => this.controller.gitPanelController.getWrappedComponent().focus(), 200) + }) } - // TODO: why doesn't a setTimeout of 0 or even 100 work here? - // Suspect it has to do with the command palette closing... - setTimeout(() => this.controller.gitPanelController.component.focus(), 200) } getActiveRepository () { @@ -173,7 +169,6 @@ export default class GithubPackage { async setActiveRepository (repository) { this.activeRepository = repository await this.changeObserver.setActiveRepository(repository) - // await this.statusBarTileController.update({repository}) this.rerender() } From a735b1e9a824fe0447b48034495ffb700c05c6b5 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 25 Nov 2016 14:49:10 -0800 Subject: [PATCH 17/47] :art: --- lib/controllers/github-package-controller.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index c5d24db854..5c86710755 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -109,12 +109,7 @@ export default class GithubPackageController extends React.Component { const filePatch = await repository.getFilePatchForPath(filePath, {staged: stagingStatus === 'staged', amending}) if (filePatch) { - this.setState({ - filePath: filePath, - filePatch: filePatch, - stagingStatus: stagingStatus, - amending: amending, - }, () => { + this.setState({ filePath, filePatch, stagingStatus, amending }, () => { // TODO: can be better done w/ a prop? if (activate) { this.filePatchControllerPane.activate() @@ -130,6 +125,8 @@ export default class GithubPackageController extends React.Component { } showMergeConflictFileForPath() {} + didChangeAmending() {} + focusFilePatchView() {} } From 5551ff0813493782398fa7d7d623485a06355669 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 28 Nov 2016 12:56:03 -0800 Subject: [PATCH 18/47] Handle amending --- lib/controllers/git-panel-controller.js | 2 +- lib/controllers/github-package-controller.js | 8 ++++++-- lib/models/repository.js | 8 ++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index 2cb34cb928..cf71a43040 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -157,7 +157,7 @@ export default class GitPanelController { } setAmending (isAmending) { - if (this.props.didChangeAmending) this.props.didChangeAmending() + if (this.props.didChangeAmending) this.props.didChangeAmending(isAmending) this.isAmending = isAmending return this.repositoryObserver.refreshModelData(this.getActiveRepository()) } diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/github-package-controller.js index 5c86710755..01ee8f72d6 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/github-package-controller.js @@ -42,6 +42,7 @@ export default class GithubPackageController extends React.Component { this.showFilePatchForPath = this.showFilePatchForPath.bind(this) this.showMergeConflictFileForPath = this.showMergeConflictFileForPath.bind(this) + this.didChangeAmending = this.didChangeAmending.bind(this) this.onRepoRefresh = this.onRepoRefresh.bind(this) this.subscriptions = new CompositeDisposable() } @@ -104,6 +105,7 @@ export default class GithubPackageController extends React.Component { } async showFilePatchForPath (filePath, stagingStatus, {activate, amending} = {}) { + if (!filePath) return const repository = this.props.repository if (!repository) return @@ -126,7 +128,9 @@ export default class GithubPackageController extends React.Component { showMergeConflictFileForPath() {} - didChangeAmending() {} - + didChangeAmending (isAmending) { + this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: isAmending}) + } + focusFilePatchView() {} } diff --git a/lib/models/repository.js b/lib/models/repository.js index cc9b5b63c7..6ed98ca581 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -149,8 +149,8 @@ export default class Repository { this.unstagedFilePatchesByPath = new Map() } - getCachedFilePatchForPath (filePath, {staged, amend} = {}) { - if (staged && amend) { + getCachedFilePatchForPath (filePath, {staged, amending} = {}) { + if (staged && amending) { return this.stagedFilePatchesSinceParentCommitByPath.get(filePath) } else if (staged) { return this.stagedFilePatchesByPath.get(filePath) @@ -159,9 +159,9 @@ export default class Repository { } } - cacheFilePatchForPath (filePath, filePatch, {staged, amend} = {}) { + cacheFilePatchForPath (filePath, filePatch, {staged, amending} = {}) { let cachedFilePatch - if (staged && amend) { + if (staged && amending) { this.stagedFilePatchesSinceParentCommitByPath.set(filePath, filePatch) } else if (staged) { this.stagedFilePatchesByPath.set(filePath, filePatch) From 1af71fd6bc5a8d6e393b777da7abe652d7c71955 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 28 Nov 2016 13:00:29 -0800 Subject: [PATCH 19/47] GithubPackageController :arrow_right: GitController --- .../{github-package-controller.js => git-controller.js} | 2 +- lib/github-package.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename lib/controllers/{github-package-controller.js => git-controller.js} (98%) diff --git a/lib/controllers/github-package-controller.js b/lib/controllers/git-controller.js similarity index 98% rename from lib/controllers/github-package-controller.js rename to lib/controllers/git-controller.js index 01ee8f72d6..1a903c04d4 100644 --- a/lib/controllers/github-package-controller.js +++ b/lib/controllers/git-controller.js @@ -20,7 +20,7 @@ const nullFilePatchState = { amending: null, } -export default class GithubPackageController extends React.Component { +export default class GitController extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, commandRegistry: React.PropTypes.object.isRequired, diff --git a/lib/github-package.js b/lib/github-package.js index 58ada45708..1ec5cdc4b6 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -5,7 +5,7 @@ import Repository from './models/repository' import FileSystemChangeObserver from './models/file-system-change-observer' import WorkspaceChangeObserver from './models/workspace-change-observer' import FilePatchController from './controllers/file-patch-controller' -import GithubPackageController from './controllers/github-package-controller' +import GitController from './controllers/git-controller' import path from 'path' @@ -46,7 +46,7 @@ export default class GithubPackage { } ReactDom.render( - this.controller = c} workspace={this.workspace} commandRegistry={this.commandRegistry} From 2c5c6f944f44cb31c512e5db4803805d4b5e6f5d Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 28 Nov 2016 18:15:45 -0800 Subject: [PATCH 20/47] Fix edge cases around updating wrapped components moved into panels --- lib/controllers/git-controller.js | 8 ++++++-- lib/controllers/git-panel-controller.js | 5 ++++- lib/github-package.js | 14 ++++++-------- lib/views/panel.js | 4 +++- lib/views/portal.js | 4 ++++ 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 1a903c04d4..8a57591194 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -67,8 +67,12 @@ export default class GitController extends React.Component { renderGitPanel () { return ( - - this.gitPanelController = c}> + container.getWrappedComponent()} + > + this.gitPanelController = c} reattachDomNode={false}> c, } componentDidMount () { @@ -41,7 +43,7 @@ export default class Panel extends React.Component { const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) const methodName = `add${location}Panel` - const item = this.portal.getComponent() + const item = props.getItem(this.portal.getComponent()) const options = Object.assign({}, this.props.options, { item }) this.panel = this.props.workspace[methodName](options) } diff --git a/lib/views/portal.js b/lib/views/portal.js index 9f9c570fb5..469047bd6f 100644 --- a/lib/views/portal.js +++ b/lib/views/portal.js @@ -62,6 +62,10 @@ export default class Portal extends React.Component { ) } + shouldComponentUpdate () { + return false + } + render () { return null } From bb577eada2e07c5cb2ae9fc5dcb1a5a2669debf4 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 28 Nov 2016 18:18:52 -0800 Subject: [PATCH 21/47] :fire: unused GithubPackage#didChangeAmending --- lib/github-package.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 9ecb62904b..c2c37d19f7 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -105,13 +105,6 @@ export default class GithubPackage { } } - didChangeAmending () { - if (this.filePatchController && this.filePatchController.props.stagingStatus === 'staged') { - this.filePatchController.destroy() - this.filePatchController = null - } - } - toggleGitPanel (callback) { this.gitPanelActive = !this.gitPanelActive this.rerender(callback) From abfbb867869983e2ab14434403111cf52801a8b1 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 28 Nov 2016 18:20:01 -0800 Subject: [PATCH 22/47] :fire: unused methods in GithubPackage --- lib/github-package.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index c2c37d19f7..47310e818d 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -24,7 +24,6 @@ export default class GithubPackage { this.activeRepository = null this.repositoriesByProjectDirectory = new Map() this.subscriptions = new CompositeDisposable() - this.filePatchController = null } async activate () { @@ -93,6 +92,7 @@ export default class GithubPackage { } focusFilePatchView () { + // TODO: migrate to ref this.filePatchController.refs.filePatchView.element.focus() } @@ -193,7 +193,6 @@ export default class GithubPackage { destroy () { this.subscriptions.dispose() if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() - if (this.filePatchController) this.filePatchController.destroy() if (this.element) { ReactDom.unmountComponentAtNode(this.element) delete this.element From 61ed981ffbd48b21866259adc3002dca9abae918 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 09:54:01 -0800 Subject: [PATCH 23/47] Change Portal and related components' `getItem` props --- lib/controllers/git-controller.js | 4 ++-- lib/views/pane-item.js | 12 ++++++++---- lib/views/panel.js | 11 +++++++++-- lib/views/portal.js | 9 +++++++-- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 8a57591194..83c7d4a408 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -70,7 +70,7 @@ export default class GitController extends React.Component { container.getWrappedComponent()} + getItem={({subtree}) => subtree.getWrappedComponent()} > this.gitPanelController = c} reattachDomNode={false}> container.getWrappedComponent()} + getItem={({subtree}) => subtree.getWrappedComponent()} ref={c => this.filePatchControllerPane = c} onDidCloseItem={() => this.setState({...nullFilePatchState})} > diff --git a/lib/views/pane-item.js b/lib/views/pane-item.js index c42653fe21..b470629249 100644 --- a/lib/views/pane-item.js +++ b/lib/views/pane-item.js @@ -10,8 +10,12 @@ import Portal from './portal' * You should use this callback to set state so that the `PaneItem` is no * longer rendered; you will get an error in your console if you forget. * - * The optional `getItem` property is a function that gets passed the wrapped - * React component instance, and you should return a valid Atom item. + * You may pass a `getItem` function that takes an object with `portal` and + * `subtree` properties. `getItem` should return an item to be added to the + * Panel. `portal` is an instance of th Portal component, and `subtree` is the + * rendered subtree component built from the `children` prop. The default + * implementation simply returns the Portal instance, which contains a + * `getElement` method (to be compatible with Atom's view system). * * Unmounting the component when the item is open will close the item. */ @@ -23,7 +27,7 @@ export default class PaneItem extends React.Component { } static defaultProps = { - getItem: (c) => c, + getItem: ({portal, subtree}) => portal, onDidCloseItem: () => {}, } @@ -46,7 +50,7 @@ export default class PaneItem extends React.Component { setupPaneItem (props) { if (this.paneItem) return - const item = props.getItem(this.portal.getComponent()) + const item = props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) this.paneItem = this.props.workspace.getActivePane().addItem(item) this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => { if (item === this.paneItem) { diff --git a/lib/views/panel.js b/lib/views/panel.js index ef850b2371..e99efaada3 100644 --- a/lib/views/panel.js +++ b/lib/views/panel.js @@ -9,6 +9,13 @@ import Portal from './portal' * location via the `location` prop, and any additional options to the * `addXPanel` method in the `options` prop. * + * You may pass a `getItem` function that takes an object with `portal` and + * `subtree` properties. `getItem` should return an item to be added to the + * Panel. `portal` is an instance of th Portal component, and `subtree` is the + * rendered subtree component built from the `children` prop. The default + * implementation simply returns the Portal instance, which contains a + * `getElement` method (to be compatible with Atom's view system). + * * You can get the underlying Atom panel via `getPanel()`, but you should * consider controlling the panel via React and the Panel component instead. */ @@ -26,7 +33,7 @@ export default class Panel extends React.Component { static defaultProps = { options: {}, - getItem: (c) => c, + getItem: ({portal, subtree}) => portal, } componentDidMount () { @@ -43,7 +50,7 @@ export default class Panel extends React.Component { const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) const methodName = `add${location}Panel` - const item = props.getItem(this.portal.getComponent()) + const item = props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) const options = Object.assign({}, this.props.options, { item }) this.panel = this.props.workspace[methodName](options) } diff --git a/lib/views/portal.js b/lib/views/portal.js index 469047bd6f..974d1b14b3 100644 --- a/lib/views/portal.js +++ b/lib/views/portal.js @@ -16,7 +16,8 @@ import ReactDom from 'react-dom' * rendered into it. Note that this uses `unstable_renderSubtreeIntoContainer` * to preserve context in the subtree. * - * `getComponent()` allows access to the rendered subtree instance + * `getElement()` allows acccess to the React subtree container element. + * `getRenderedSubtree()` allows access to the rendered subtree instance * (`Stuff` in the example above). * * Pass `false` (the default) to `appendNode` to skip adding the node to the @@ -70,7 +71,11 @@ export default class Portal extends React.Component { return null } - getComponent () { + getRenderedSubtree () { return this.portal } + + getElement () { + return this.node + } } From e1bc7f594efdba8f69b330ed82c4b4a2966a19fa Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 10:50:07 -0800 Subject: [PATCH 24/47] Implement showing merge conflict files on GitPanel click --- lib/controllers/git-controller.js | 13 +++++++++++-- lib/github-package.js | 9 --------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 83c7d4a408..06b780f785 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -1,6 +1,8 @@ /** @babel */ -import {CompositeDisposable} from 'atom' +import path from 'path' + +import {CompositeDisposable, File} from 'atom' import React from 'react' import ReactDom from 'react-dom' @@ -130,7 +132,14 @@ export default class GitController extends React.Component { this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: this.state.amending}) } - showMergeConflictFileForPath() {} + async showMergeConflictFileForPath (relativeFilePath, {focus} = {}) { + const absolutePath = path.join(this.props.repository.getWorkingDirectoryPath(), relativeFilePath) + if (await new File(absolutePath).exists()) { + return this.props.workspace.open(absolutePath, {activatePane: Boolean(focus), pending: true}) + } else { + this.props.notificationManager.addInfo('File has been deleted.') + } + } didChangeAmending (isAmending) { this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: isAmending}) diff --git a/lib/github-package.js b/lib/github-package.js index 47310e818d..a361347df1 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -96,15 +96,6 @@ export default class GithubPackage { this.filePatchController.refs.filePatchView.element.focus() } - async showMergeConflictFileForPath (relativeFilePath, {focus} = {}) { - const absolutePath = path.join(this.getActiveRepository().getWorkingDirectoryPath(), relativeFilePath) - if (await new File(absolutePath).exists()) { - return this.workspace.open(absolutePath, {activatePane: Boolean(focus)}) - } else { - this.notificationManager.addInfo('File has been deleted.') - } - } - toggleGitPanel (callback) { this.gitPanelActive = !this.gitPanelActive this.rerender(callback) From 59eecb3d388bca791ca509b6cca6c1e5e4935e88 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 14:21:17 -0800 Subject: [PATCH 25/47] Fix leaking subscriptions when deactivating and reactivating package --- lib/github-package.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index a361347df1..caf4a335a2 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -23,13 +23,14 @@ export default class GithubPackage { : new FileSystemChangeObserver() this.activeRepository = null this.repositoriesByProjectDirectory = new Map() - this.subscriptions = new CompositeDisposable() } async activate () { + this.subscriptions = new CompositeDisposable() await this.updateActiveRepository() await this.changeObserver.start() this.subscriptions.add( + new Disposable(() => this.changeObserver.stop()), this.project.onDidChangePaths(this.didChangeProjectPaths.bind(this)), this.workspace.onDidChangeActivePaneItem(this.didChangeActivePaneItem.bind(this)), this.changeObserver.onDidChange(this.refreshActiveRepository.bind(this)), @@ -42,6 +43,10 @@ export default class GithubPackage { rerender (callback = null) { if (!this.element) { this.element = document.createElement('div') + this.subscriptions.add(new Disposable(() => { + ReactDom.unmountComponentAtNode(this.element) + delete this.element + })) } ReactDom.render( @@ -59,7 +64,8 @@ export default class GithubPackage { } async deactivate () { - await this.changeObserver.stop() + this.subscriptions.dispose() + if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() } // FIXME: Combine these 2 methods into a single consumeStatusBar method after @@ -180,13 +186,4 @@ export default class GithubPackage { } } } - - destroy () { - this.subscriptions.dispose() - if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() - if (this.element) { - ReactDom.unmountComponentAtNode(this.element) - delete this.element - } - } } From 794f1bc081f9ebfe98c42679f8e767e29653eaf2 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 14:21:40 -0800 Subject: [PATCH 26/47] :art: activeRepository -> nextActiveRepository --- lib/github-package.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index caf4a335a2..80739cb0d6 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -122,34 +122,36 @@ export default class GithubPackage { } async updateActiveRepository () { - let activeRepository + let nextActiveRepository let activeItem = this.workspace.getActivePaneItem() if (activeItem && typeof activeItem.getPath === 'function') { let projectDirectory = this.projectDirectoryForPath(activeItem.getPath()) if (projectDirectory) { - activeRepository = await this.repositoryForProjectDirectory(projectDirectory) + nextActiveRepository = await this.repositoryForProjectDirectory(projectDirectory) } } else { - activeRepository = this.activeRepository + nextActiveRepository = this.activeRepository } if (activeItem instanceof FilePatchController) { if (!activeItem.props.repository.isDestroyed()) { - activeRepository = activeItem.props.repository + nextActiveRepository = activeItem.props.repository } } - if (activeRepository && activeRepository !== this.activeRepository) { - await this.setActiveRepository(activeRepository) + if (nextActiveRepository && nextActiveRepository !== this.activeRepository) { + await this.setActiveRepository(nextActiveRepository) if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() - this.destroyedRepositorySubscription = activeRepository.onDidDestroy(() => { - this.setActiveRepository(null) + this.destroyedRepositorySubscription = nextActiveRepository.onDidDestroy(() => { + if (nextActiveRepository === this.activeRepository) { + this.setActiveRepository(null) + } this.destroyedRepositorySubscription.dispose() }) } - if (!activeRepository) { + if (!nextActiveRepository) { await this.setActiveRepository(null) } } From 6085991433741db352db8acb5bab6f308d11ceb6 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 14:23:47 -0800 Subject: [PATCH 27/47] Serialize and restore GitPanel state --- lib/github-package.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 80739cb0d6..45fd8e1b5b 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -1,6 +1,6 @@ /** @babel */ -import {CompositeDisposable, Disposable, File} from 'atom' +import {CompositeDisposable, Disposable, Directory, File} from 'atom' import Repository from './models/repository' import FileSystemChangeObserver from './models/file-system-change-observer' import WorkspaceChangeObserver from './models/workspace-change-observer' @@ -23,9 +23,10 @@ export default class GithubPackage { : new FileSystemChangeObserver() this.activeRepository = null this.repositoriesByProjectDirectory = new Map() + this.gitPanelActive = false } - async activate () { + async activate (state) { this.subscriptions = new CompositeDisposable() await this.updateActiveRepository() await this.changeObserver.start() @@ -37,9 +38,25 @@ export default class GithubPackage { this.commandRegistry.add('atom-workspace', {'git:toggle-git-panel': this.toggleGitPanel.bind(this)}), this.commandRegistry.add('atom-workspace', {'git:focus-git-panel': this.focusGitPanel.bind(this)}) ) + this.gitPanelActive = !!state.gitPanelActive + if (state.activeRepositoryDir) { + const dir = new Directory(state.activeRepositoryDir) + const repo = await this.repositoryForProjectDirectory(dir) + if (repo) { + await this.setActiveRepository(repo) + } + } this.rerender() } + serialize () { + const activeRepository = this.getActiveRepository() + return { + gitPanelActive: this.gitPanelActive, + activeRepositoryDir: activeRepository ? activeRepository.getWorkingDirectoryPath() : null, + } + } + rerender (callback = null) { if (!this.element) { this.element = document.createElement('div') From 672ae3fcbddda4025c52dc3638fe5fa2e2561e91 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 14:38:40 -0800 Subject: [PATCH 28/47] Clean up status bar consumption --- lib/controllers/git-controller.js | 13 +++++++------ lib/github-package.js | 12 +----------- lib/views/status-bar.js | 10 ++++++---- package.json | 3 +-- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 06b780f785..716ec5560d 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -29,11 +29,6 @@ export default class GitController extends React.Component { notificationManager: React.PropTypes.object.isRequired, repository: React.PropTypes.object, statusBar: React.PropTypes.object, - legacyStatusBar: React.PropTypes.bool, - } - - static defaultProps = { - legacyStatusBar: false, } constructor (props, context) { @@ -52,7 +47,7 @@ export default class GitController extends React.Component { render () { return (
- + this.onConsumeStatusBar(sb)}> , this.element, typeof callback === 'function' ? callback : null ) @@ -85,17 +84,8 @@ export default class GithubPackage { if (this.destroyedRepositorySubscription) this.destroyedRepositorySubscription.dispose() } - // FIXME: Combine these 2 methods into a single consumeStatusBar method after - // Atom 1.13 is on stable channel - consumeStatusBar_1_0 (statusBar) { + consumeStatusBar (statusBar) { this.statusBar = statusBar - this.legacyStatusBar = true - this.rerender() - } - - consumeStatusBar_1_1 (statusBar) { - this.statusBar = statusBar - this.legacyStatusBar = false this.rerender() } diff --git a/lib/views/status-bar.js b/lib/views/status-bar.js index 1760a2fdd4..727b2e06c1 100644 --- a/lib/views/status-bar.js +++ b/lib/views/status-bar.js @@ -6,7 +6,11 @@ import React from 'react' export default class StatusBar extends React.Component { static propTypes = { statusBar: React.PropTypes.object, - legacy: React.PropTypes.bool, + onConsumeStatusBar: React.PropTypes.func, + } + + static defaultProps = { + onConsumeStatusBar: (statusBar) => {}, } componentDidMount () { @@ -25,10 +29,8 @@ export default class StatusBar extends React.Component { if (this.tile) return if (!props.statusBar) return - if (!props.legacy) { - props.statusBar.disableGitInfoTile() - } const componentElement = this.container.children[0] this.tile = props.statusBar.addRightTile({item: componentElement, priority: -50}) + this.props.onConsumeStatusBar(props.statusBar) } } diff --git a/package.json b/package.json index 5c3007e7d4..54657ef9ab 100644 --- a/package.json +++ b/package.json @@ -49,8 +49,7 @@ "consumedServices": { "status-bar": { "versions": { - "1.0.0": "consumeStatusBar_1_0", - "1.1.0": "consumeStatusBar_1_1" + "^1.0.0": "consumeStatusBar" } } } From 2c8520595b519f0fb88d36e26c22db4dad84a38e Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 15:21:23 -0800 Subject: [PATCH 29/47] First pass at fixing tests --- lib/github-package.js | 5 +- test/github-package.test.js | 432 +++++++++++++++++------------------- test/helpers.js | 54 +++++ 3 files changed, 258 insertions(+), 233 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 4229b1819b..1e3ff083ef 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -24,9 +24,10 @@ export default class GithubPackage { this.activeRepository = null this.repositoriesByProjectDirectory = new Map() this.gitPanelActive = false + this.subscriptions = new CompositeDisposable() } - async activate (state) { + async activate (state = {}) { this.subscriptions = new CompositeDisposable() await this.updateActiveRepository() await this.changeObserver.start() @@ -90,8 +91,8 @@ export default class GithubPackage { } async didChangeProjectPaths () { - await this.destroyRepositoriesForRemovedProjectFolders() await this.updateActiveRepository() + this.destroyRepositoriesForRemovedProjectFolders() } async didChangeActivePaneItem () { diff --git a/test/github-package.test.js b/test/github-package.test.js index 8df81a75e0..9e855cb05a 100644 --- a/test/github-package.test.js +++ b/test/github-package.test.js @@ -5,7 +5,7 @@ import path from 'path' import temp from 'temp' import etch from 'etch' import sinon from 'sinon' -import {cloneRepository, buildRepository} from './helpers' +import {cloneRepository, buildRepository, until} from './helpers' import FilePatch from '../lib/models/file-patch' import GithubPackage from '../lib/github-package' @@ -37,8 +37,6 @@ describe('GithubPackage', () => { await workspace.open(path.join(workdirPath1, 'a.txt')) await githubPackage.activate() assert.equal(githubPackage.getActiveRepository(), await githubPackage.repositoryForWorkdirPath(workdirPath1)) - assert.equal(githubPackage.gitPanelController.getActiveRepository(), githubPackage.getActiveRepository()) - assert.equal(githubPackage.statusBarTileController.getActiveRepository(), githubPackage.getActiveRepository()) }) }) @@ -51,34 +49,31 @@ describe('GithubPackage', () => { project.setPaths([workdirPath1, workdirPath2, nonRepositoryPath]) fs.writeFileSync(path.join(workdirPath1, 'a.txt'), 'change 1', 'utf8') + sinon.spy(githubPackage, 'rerender') await workspace.open(path.join(workdirPath1, 'a.txt')) await githubPackage.didChangeProjectPaths() assert.equal(githubPackage.getActiveRepository(), await githubPackage.repositoryForWorkdirPath(workdirPath1)) - assert.equal(githubPackage.gitPanelController.getActiveRepository(), githubPackage.getActiveRepository()) assert.equal(githubPackage.changeObserver.getActiveRepository(), githubPackage.getActiveRepository()) - assert.equal(githubPackage.statusBarTileController.getActiveRepository(), githubPackage.getActiveRepository()) + assert.equal(githubPackage.rerender.callCount, 1) // Remove repository for open file project.setPaths([workdirPath2, nonRepositoryPath]) await githubPackage.didChangeProjectPaths() assert.isNull(githubPackage.getActiveRepository()) assert.isNull(githubPackage.changeObserver.getActiveRepository()) - assert.isNull(githubPackage.gitPanelController.getActiveRepository()) - assert.isNull(githubPackage.statusBarTileController.getActiveRepository()) + assert.equal(githubPackage.rerender.callCount, 2) await workspace.open(path.join(workdirPath2, 'b.txt')) await githubPackage.didChangeProjectPaths() assert.equal(githubPackage.getActiveRepository(), await githubPackage.repositoryForWorkdirPath(workdirPath2)) assert.equal(githubPackage.changeObserver.getActiveRepository(), githubPackage.getActiveRepository()) - assert.equal(githubPackage.gitPanelController.getActiveRepository(), githubPackage.getActiveRepository()) - assert.equal(githubPackage.statusBarTileController.getActiveRepository(), githubPackage.getActiveRepository()) + assert.equal(githubPackage.rerender.callCount, 3) await workspace.open(path.join(nonRepositoryPath, 'c.txt')) await githubPackage.didChangeProjectPaths() assert.isNull(githubPackage.getActiveRepository()) assert.isNull(githubPackage.changeObserver.getActiveRepository()) - assert.isNull(githubPackage.gitPanelController.getActiveRepository()) - assert.isNull(githubPackage.statusBarTileController.getActiveRepository()) + assert.equal(githubPackage.rerender.callCount, 4) }) it('destroys all the repositories associated with the removed project folders', async () => { @@ -96,7 +91,7 @@ describe('GithubPackage', () => { project.removePath(workdirPath1) project.removePath(workdirPath3) - githubPackage.didChangeProjectPaths() + await githubPackage.didChangeProjectPaths() assert.notEqual(await githubPackage.repositoryForProjectDirectory(repository1.getWorkingDirectory()), repository1) assert.notEqual(await githubPackage.repositoryForProjectDirectory(repository3.getWorkingDirectory()), repository3) @@ -115,14 +110,10 @@ describe('GithubPackage', () => { await workspace.open(path.join(workdirPath1, 'a.txt')) await githubPackage.didChangeActivePaneItem() assert.equal(githubPackage.getActiveRepository(), await githubPackage.repositoryForWorkdirPath(workdirPath1)) - assert.equal(githubPackage.gitPanelController.getActiveRepository(), githubPackage.getActiveRepository()) - assert.equal(githubPackage.statusBarTileController.getActiveRepository(), githubPackage.getActiveRepository()) await workspace.open(path.join(workdirPath2, 'b.txt')) await githubPackage.didChangeActivePaneItem() assert.equal(githubPackage.getActiveRepository(), await githubPackage.repositoryForWorkdirPath(workdirPath2)) - assert.equal(githubPackage.gitPanelController.getActiveRepository(), githubPackage.getActiveRepository()) - assert.equal(githubPackage.statusBarTileController.getActiveRepository(), githubPackage.getActiveRepository()) }) }) @@ -168,234 +159,213 @@ describe('GithubPackage', () => { await githubPackage.updateActiveRepository() assert.isNull(githubPackage.getActiveRepository()) }) - - describe('when the active item is a FilePatchController', () => { - it('updates the active repository to be the one associated with the FilePatchController', async () => { - const workdirPath = await cloneRepository('three-files') - fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') - - project.setPaths([workdirPath]) - const repository = await githubPackage.repositoryForWorkdirPath(workdirPath) - - await workspace.open(path.join(workdirPath, 'b.txt')) - await githubPackage.updateActiveRepository() - assert.equal(githubPackage.getActiveRepository(), repository) - - await githubPackage.gitPanelController.props.didSelectFilePath('a.txt', 'unstaged', {activate: true}) - assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - assert.equal(githubPackage.filePatchController.props.repository, repository) - await githubPackage.updateActiveRepository() - assert.equal(githubPackage.getActiveRepository(), repository) - }) - }) - }) - - describe('showMergeConflictFileForPath(filePath)', () => { - it('opens the file as a pane item if it exsits', async () => { - const workdirPath = await cloneRepository('merge-conflict') - const repository = await buildRepository(workdirPath) - githubPackage.getActiveRepository = function () { return repository } - await githubPackage.gitPanelController.props.didSelectMergeConflictFile('added-to-both.txt') - assert.equal(workspace.getActivePaneItem().getPath(), path.join(workdirPath, 'added-to-both.txt')) - }) - - describe('when the file doesn\'t exist', () => { - it('shows an info notification and does not open the file', async () => { - const workdirPath = await cloneRepository('merge-conflict') - const repository = await buildRepository(workdirPath) - githubPackage.getActiveRepository = function () { return repository } - fs.unlinkSync(path.join(workdirPath, 'added-to-both.txt')) - - assert.equal(notificationManager.getNotifications().length, 0) - await githubPackage.gitPanelController.props.didSelectMergeConflictFile('added-to-both.txt') - assert.isUndefined(workspace.getActivePaneItem()) - assert.equal(notificationManager.getNotifications().length, 1) - }) - }) - }) - - describe('showFilePatchForPath(filePath, staged, {amending, activate})', () => { - describe('when a file is selected in the staging panel', () => { - it('shows a FilePatchView for the selected file as a pane item, always updating the existing pane item', async () => { - const workdirPath = await cloneRepository('three-files') - const repository = await buildRepository(workdirPath) - - fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') - fs.writeFileSync(path.join(workdirPath, 'd.txt'), 'new-file', 'utf8') - await repository.stageFiles(['d.txt']) - - githubPackage.getActiveRepository = function () { return repository } - - assert.isNull(githubPackage.filePatchController) - - await githubPackage.showFilePatchForPath('a.txt', 'unstaged', {activate: true}) - assert(githubPackage.filePatchController) - assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'a.txt') - assert.equal(githubPackage.filePatchController.props.repository, repository) - assert.equal(githubPackage.filePatchController.props.stagingStatus, 'unstaged') - assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - - const existingFilePatchView = githubPackage.filePatchController - const originalPane = workspace.getActivePane() - originalPane.splitRight() // activate a different pane - assert.isUndefined(workspace.getActivePaneItem()) - - await githubPackage.showFilePatchForPath('d.txt', 'staged') - assert.equal(githubPackage.filePatchController, existingFilePatchView) - assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'd.txt') - assert.equal(githubPackage.filePatchController.props.repository, repository) - assert.equal(githubPackage.filePatchController.props.stagingStatus, 'staged') - assert.equal(originalPane.getActiveItem(), githubPackage.filePatchController) - - originalPane.getActiveItem().destroy() - assert.isUndefined(workspace.getActivePaneItem()) - assert.isNull(githubPackage.filePatchController) - - await githubPackage.showFilePatchForPath('d.txt', 'staged', {activate: true}) - assert.notEqual(githubPackage.filePatchController, existingFilePatchView) - assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'd.txt') - assert.equal(githubPackage.filePatchController.props.repository, repository) - assert.equal(githubPackage.filePatchController.props.stagingStatus, 'staged') - assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - }) - }) - - describe('when there is a change to the repo', () => { - it('updates the FilePatchController with the correct filePatch', async () => { - const workdirPath = await cloneRepository('multiple-commits') - const repository = await buildRepository(workdirPath) - - githubPackage.getActiveRepository = function () { return repository } - fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change', 'utf8') - await githubPackage.showFilePatchForPath('file.txt', 'unstaged', {activate: true}) - - let filePatchController = githubPackage.filePatchController - sinon.spy(filePatchController, 'update') - - // udpate unstaged file patch - fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nplus more changes', 'utf8') - await repository.refresh() - // repository refresh causes async actions so waiting for this is the best way to determine the update has finished - await etch.getScheduler().getNextUpdatePromise() - const unstagedFilePatch = await repository.getFilePatchForPath('file.txt') - assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, unstagedFilePatch) - - // update staged file patch - await repository.stageFiles(['file.txt']) - await githubPackage.showFilePatchForPath('file.txt', 'staged', {activate: true}) - fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nplus more changes\nand more!', 'utf8') - await repository.stageFiles(['file.txt']) - await repository.refresh() - // repository refresh causes async actions so waiting for this is the best way to determine the update has finished - await etch.getScheduler().getNextUpdatePromise() - const stagedFilePatch = await repository.getFilePatchForPath('file.txt', {staged: true}) - assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, stagedFilePatch) - - // update amended file patch - // didChangeAmending destroys current filePatchController - githubPackage.gitPanelController.props.didChangeAmending() - await githubPackage.showFilePatchForPath('file.txt', 'staged', {activate: true, amending: true}) - filePatchController = githubPackage.filePatchController - sinon.spy(filePatchController, 'update') - fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change file being amended', 'utf8') - await repository.stageFiles(['file.txt']) - await repository.refresh() - // repository refresh causes async actions so waiting for this is the best way to determine the update has finished - await etch.getScheduler().getNextUpdatePromise() - const amendedStagedFilePatch = await repository.getFilePatchForPath('file.txt', {staged: true, amending: true}) - assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, amendedStagedFilePatch) - }) - }) }) - describe('when amend mode is toggled in the staging panel while viewing a staged change', () => { - it('closes the file patch pane item', async () => { - const workdirPath = await cloneRepository('three-files') - const repository = await buildRepository(workdirPath) - fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') - await repository.stageFiles(['a.txt']) - - githubPackage.getActiveRepository = function () { return repository } - - await githubPackage.showFilePatchForPath('a.txt', 'staged', {activate: true}) - assert.isOk(githubPackage.filePatchController) - assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - - githubPackage.gitPanelController.props.didChangeAmending() - assert.isNull(githubPackage.filePatchController) - assert.isUndefined(workspace.getActivePaneItem()) - }) - }) - - describe('when the changed files label in the status bar is clicked', () => { - it('toggles the git panel', async () => { - const workdirPath = await cloneRepository('three-files') - project.setPaths([workdirPath]) - await workspace.open(path.join(workdirPath, 'a.txt')) - await githubPackage.updateActiveRepository() - - githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() - assert.equal(workspace.getRightPanels().length, 1) - - githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() - assert.equal(workspace.getRightPanels().length, 0) - - githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() - assert.equal(workspace.getRightPanels().length, 1) - }) - }) + // TODO: move into GitController tests + // describe('showMergeConflictFileForPath(filePath)', () => { + // it('opens the file as a pane item if it exsits', async () => { + // const workdirPath = await cloneRepository('merge-conflict') + // const repository = await buildRepository(workdirPath) + // githubPackage.getActiveRepository = function () { return repository } + // await githubPackage.gitPanelController.props.didSelectMergeConflictFile('added-to-both.txt') + // assert.equal(workspace.getActivePaneItem().getPath(), path.join(workdirPath, 'added-to-both.txt')) + // }) + // + // describe('when the file doesn\'t exist', () => { + // it('shows an info notification and does not open the file', async () => { + // const workdirPath = await cloneRepository('merge-conflict') + // const repository = await buildRepository(workdirPath) + // githubPackage.getActiveRepository = function () { return repository } + // fs.unlinkSync(path.join(workdirPath, 'added-to-both.txt')) + // + // assert.equal(notificationManager.getNotifications().length, 0) + // await githubPackage.gitPanelController.props.didSelectMergeConflictFile('added-to-both.txt') + // assert.isUndefined(workspace.getActivePaneItem()) + // assert.equal(notificationManager.getNotifications().length, 1) + // }) + // }) + // }) + + // TODO: move into GitController tests + // describe('showFilePatchForPath(filePath, staged, {amending, activate})', () => { + // describe('when a file is selected in the staging panel', () => { + // it('shows a FilePatchView for the selected file as a pane item, always updating the existing pane item', async () => { + // const workdirPath = await cloneRepository('three-files') + // const repository = await buildRepository(workdirPath) + // + // fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') + // fs.writeFileSync(path.join(workdirPath, 'd.txt'), 'new-file', 'utf8') + // await repository.stageFiles(['d.txt']) + // + // githubPackage.getActiveRepository = function () { return repository } + // + // assert.isNull(githubPackage.filePatchController) + // + // await githubPackage.showFilePatchForPath('a.txt', 'unstaged', {activate: true}) + // assert(githubPackage.filePatchController) + // assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'a.txt') + // assert.equal(githubPackage.filePatchController.props.repository, repository) + // assert.equal(githubPackage.filePatchController.props.stagingStatus, 'unstaged') + // assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) + // + // const existingFilePatchView = githubPackage.filePatchController + // const originalPane = workspace.getActivePane() + // originalPane.splitRight() // activate a different pane + // assert.isUndefined(workspace.getActivePaneItem()) + // + // await githubPackage.showFilePatchForPath('d.txt', 'staged') + // assert.equal(githubPackage.filePatchController, existingFilePatchView) + // assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'd.txt') + // assert.equal(githubPackage.filePatchController.props.repository, repository) + // assert.equal(githubPackage.filePatchController.props.stagingStatus, 'staged') + // assert.equal(originalPane.getActiveItem(), githubPackage.filePatchController) + // + // originalPane.getActiveItem().destroy() + // assert.isUndefined(workspace.getActivePaneItem()) + // assert.isNull(githubPackage.filePatchController) + // + // await githubPackage.showFilePatchForPath('d.txt', 'staged', {activate: true}) + // assert.notEqual(githubPackage.filePatchController, existingFilePatchView) + // assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'd.txt') + // assert.equal(githubPackage.filePatchController.props.repository, repository) + // assert.equal(githubPackage.filePatchController.props.stagingStatus, 'staged') + // assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) + // }) + // }) + + // TODO: Move into GitController tests + // describe('when there is a change to the repo', () => { + // it('updates the FilePatchController with the correct filePatch', async () => { + // const workdirPath = await cloneRepository('multiple-commits') + // const repository = await buildRepository(workdirPath) + // + // githubPackage.getActiveRepository = function () { return repository } + // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change', 'utf8') + // await githubPackage.showFilePatchForPath('file.txt', 'unstaged', {activate: true}) + // + // let filePatchController = githubPackage.filePatchController + // sinon.spy(filePatchController, 'update') + // + // // udpate unstaged file patch + // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nplus more changes', 'utf8') + // await repository.refresh() + // // repository refresh causes async actions so waiting for this is the best way to determine the update has finished + // await etch.getScheduler().getNextUpdatePromise() + // const unstagedFilePatch = await repository.getFilePatchForPath('file.txt') + // assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, unstagedFilePatch) + // + // // update staged file patch + // await repository.stageFiles(['file.txt']) + // await githubPackage.showFilePatchForPath('file.txt', 'staged', {activate: true}) + // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nplus more changes\nand more!', 'utf8') + // await repository.stageFiles(['file.txt']) + // await repository.refresh() + // // repository refresh causes async actions so waiting for this is the best way to determine the update has finished + // await etch.getScheduler().getNextUpdatePromise() + // const stagedFilePatch = await repository.getFilePatchForPath('file.txt', {staged: true}) + // assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, stagedFilePatch) + // + // // update amended file patch + // // didChangeAmending destroys current filePatchController + // githubPackage.gitPanelController.props.didChangeAmending() + // await githubPackage.showFilePatchForPath('file.txt', 'staged', {activate: true, amending: true}) + // filePatchController = githubPackage.filePatchController + // sinon.spy(filePatchController, 'update') + // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change file being amended', 'utf8') + // await repository.stageFiles(['file.txt']) + // await repository.refresh() + // // repository refresh causes async actions so waiting for this is the best way to determine the update has finished + // await etch.getScheduler().getNextUpdatePromise() + // const amendedStagedFilePatch = await repository.getFilePatchForPath('file.txt', {staged: true, amending: true}) + // assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, amendedStagedFilePatch) + // }) + // }) + // }) + + // TODO: Move into GitController tests + // describe('when amend mode is toggled in the staging panel while viewing a staged change', () => { + // it('closes the file patch pane item', async () => { + // const workdirPath = await cloneRepository('three-files') + // const repository = await buildRepository(workdirPath) + // fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') + // await repository.stageFiles(['a.txt']) + // + // githubPackage.getActiveRepository = function () { return repository } + // + // await githubPackage.showFilePatchForPath('a.txt', 'staged', {activate: true}) + // assert.isOk(githubPackage.filePatchController) + // assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) + // + // githubPackage.gitPanelController.props.didChangeAmending() + // assert.isNull(githubPackage.filePatchController) + // assert.isUndefined(workspace.getActivePaneItem()) + // }) + // }) + // + // describe('when the changed files label in the status bar is clicked', () => { + // it('toggles the git panel', async () => { + // const workdirPath = await cloneRepository('three-files') + // project.setPaths([workdirPath]) + // await workspace.open(path.join(workdirPath, 'a.txt')) + // await githubPackage.updateActiveRepository() + // + // githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() + // assert.equal(workspace.getRightPanels().length, 1) + // + // githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() + // assert.equal(workspace.getRightPanels().length, 0) + // + // githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() + // assert.equal(workspace.getRightPanels().length, 1) + // }) + // }) describe('toggleGitPanel()', () => { it('shows-and-focuses or hides the git panel', async () => { - const workspaceElement = viewRegistry.getView(workspace) - document.body.appendChild(workspaceElement) + sinon.stub(githubPackage, 'rerender') const workdirPath = await cloneRepository('three-files') project.setPaths([workdirPath]) await workspace.open(path.join(workdirPath, 'a.txt')) await githubPackage.activate() - assert.equal(workspace.getRightPanels().length, 0) - githubPackage.toggleGitPanel() - assert.equal(workspace.getRightPanels().length, 1) - assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) - assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - + githubPackage.rerender.reset() + assert.isFalse(githubPackage.gitPanelActive) githubPackage.toggleGitPanel() - assert.equal(workspace.getRightPanels().length, 0) - + assert.isTrue(githubPackage.gitPanelActive) + assert.equal(githubPackage.rerender.callCount, 1) + // TODO: fix this + // assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) githubPackage.toggleGitPanel() - assert.equal(workspace.getRightPanels().length, 1) - assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) - assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - - workspaceElement.remove() + assert.isFalse(githubPackage.gitPanelActive) + assert.equal(githubPackage.rerender.callCount, 2) }) }) - describe('focusGitPanel()', () => { - it('shows-and-focuses the git panel', async () => { - const workspaceElement = viewRegistry.getView(workspace) - document.body.appendChild(workspaceElement) - const workdirPath = await cloneRepository('three-files') - project.setPaths([workdirPath]) - await workspace.open(path.join(workdirPath, 'a.txt')) - await githubPackage.activate() - - assert.equal(workspace.getRightPanels().length, 0) - githubPackage.focusGitPanel() - assert.equal(workspace.getRightPanels().length, 1) - assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) - assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - - githubPackage.toggleGitPanel() - assert.equal(workspace.getRightPanels().length, 0) - - githubPackage.focusGitPanel() - assert.equal(workspace.getRightPanels().length, 1) - assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) - assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - - workspaceElement.remove() - }) - }) + // TODO: fix + // describe('focusGitPanel()', () => { + // it('shows-and-focuses the git panel', async () => { + // const workspaceElement = viewRegistry.getView(workspace) + // document.body.appendChild(workspaceElement) + // const workdirPath = await cloneRepository('three-files') + // project.setPaths([workdirPath]) + // await workspace.open(path.join(workdirPath, 'a.txt')) + // await githubPackage.activate() + // + // assert.equal(workspace.getRightPanels().length, 0) + // githubPackage.focusGitPanel() + // assert.equal(workspace.getRightPanels().length, 1) + // assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) + // assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) + // + // githubPackage.toggleGitPanel() + // assert.equal(workspace.getRightPanels().length, 0) + // + // githubPackage.focusGitPanel() + // assert.equal(workspace.getRightPanels().length, 1) + // assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) + // assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) + // + // workspaceElement.remove() + // }) + // }) }) diff --git a/test/helpers.js b/test/helpers.js index 65b5f74f10..52d8ab5aaf 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -95,3 +95,57 @@ export function assertEqualSortedArraysByKey (arr1, arr2, key) { const sortFn = (a, b) => a[key] < b[key] assert.deepEqual(arr1.sort(sortFn), arr2.sort(sortFn)) } + +// like `waitsFor` and `waitsForPromise` except you can `await` it +// to suspend/block an `async` test, and you don't have to use `runs`. +// Usage: `await until([description,] [timeout,] fn)` +// Arguments can be supplied in any order +// +// E.g.: +// +// await until('a thing happens', () => didThingHappen()) +// expect(thing).toBe(something) +export function until (...args) { + const start = new Date().getTime() + + let latchFunction = null + let message = null + let timeout = null + + if (args.length > 3) { + throw new Error('until only takes up to 3 args') + } + + for (let arg of args) { + switch (typeof arg) { + case 'function': + latchFunction = arg + break + case 'string': + message = arg + break + case 'number': + timeout = arg + break + } + } + + message = message || 'something happens' + timeout = timeout || 5000 + + return new Promise((resolve, reject) => { + const checker = () => { + const result = latchFunction() + if (result) return resolve(result) + + const now = new Date().getTime() + const delta = now - start + if (delta > timeout) { + return reject(new Error(`timeout: timed out after ${timeout} msec waiting until ${message}`)) + } else { + setTimeout(checker) + } + } + checker() + }) +} From c1ece9511d3e4bd892bdf09a1ad39d393ff8f7b0 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 20:48:32 -0800 Subject: [PATCH 30/47] I need dis --- package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package.json b/package.json index 54657ef9ab..4ba392ac47 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,8 @@ "multi-list-selection": "^0.1.1", "ncp": "^2.0.0", "nsfw": "1.0.12", + "react": "^15.4.1", + "react-dom": "^15.4.1", "recursive-readdir": "^2.0.0", "tree-kill": "^1.1.0", "what-the-diff": "^0.2.0" From 9f39cfc50b6a33c88bae3d5e307e93ba879a4602 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 21:40:12 -0800 Subject: [PATCH 31/47] Add test for Portal --- lib/views/portal.js | 4 ++-- test/views/portal.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 test/views/portal.test.js diff --git a/lib/views/portal.js b/lib/views/portal.js index 974d1b14b3..8d6f848e32 100644 --- a/lib/views/portal.js +++ b/lib/views/portal.js @@ -58,7 +58,7 @@ export default class Portal extends React.Component { } renderPortal (props) { - this.portal = ReactDom.unstable_renderSubtreeIntoContainer( + this.subtree = ReactDom.unstable_renderSubtreeIntoContainer( this, props.children, this.node ) } @@ -72,7 +72,7 @@ export default class Portal extends React.Component { } getRenderedSubtree () { - return this.portal + return this.subtree } getElement () { diff --git a/test/views/portal.test.js b/test/views/portal.test.js new file mode 100644 index 0000000000..30734f77a1 --- /dev/null +++ b/test/views/portal.test.js @@ -0,0 +1,34 @@ +/** @babel */ + +import React from 'react' +import ReactDom from 'react-dom' + +import Portal from '../../lib/views/portal' + +class Component extends React.Component { + render () { + return ( +
{this.props.text}
+ ) + } + + getText () { + return this.props.text + } +} + +describe('Portal', () => { + it('renders a subtree into a different dom node', () => { + const node = document.createElement('div') + let portal, oldPortal + ReactDom.render( portal = c}>, node) + assert.equal(portal.getElement().textContent, 'hello') + assert.equal(portal.getRenderedSubtree().getText(), 'hello') + oldPortal = portal + ReactDom.render( portal = c}>, node) + assert.equal(oldPortal, portal) // portal got updated, not recreated + assert.equal(oldPortal.getRenderedSubtree(), portal.getRenderedSubtree()) // subtree also got updated, not recreated + assert.equal(portal.getElement().textContent, 'world') + assert.equal(portal.getRenderedSubtree().getText(), 'world') + }) +}) From e665bb1940d9536af87f093682fb8b437ae54b47 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 29 Nov 2016 22:50:03 -0800 Subject: [PATCH 32/47] Add test for Panel --- lib/views/panel.js | 3 +- test/views/panel.test.js | 72 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 test/views/panel.test.js diff --git a/lib/views/panel.js b/lib/views/panel.js index e99efaada3..595e1ccc57 100644 --- a/lib/views/panel.js +++ b/lib/views/panel.js @@ -11,7 +11,7 @@ import Portal from './portal' * * You may pass a `getItem` function that takes an object with `portal` and * `subtree` properties. `getItem` should return an item to be added to the - * Panel. `portal` is an instance of th Portal component, and `subtree` is the + * Panel. `portal` is an instance of the Portal component, and `subtree` is the * rendered subtree component built from the `children` prop. The default * implementation simply returns the Portal instance, which contains a * `getElement` method (to be compatible with Atom's view system). @@ -47,6 +47,7 @@ export default class Panel extends React.Component { setupPanel (props) { if (this.panel) return + // "left" => "Left" const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) const methodName = `add${location}Panel` diff --git a/test/views/panel.test.js b/test/views/panel.test.js new file mode 100644 index 0000000000..f54d7ab382 --- /dev/null +++ b/test/views/panel.test.js @@ -0,0 +1,72 @@ +/** @babel */ + +import React from 'react' +import ReactDom from 'react-dom' +import sinon from 'sinon' + +import Panel from '../../lib/views/panel' + +class Component extends React.Component { + render () { + return ( +
{this.props.text}
+ ) + } + + getText () { + return this.props.text + } +} + +describe('Panel component', () => { + it('renders a React component into an Atom panel', () => { + const workspace = { + addLeftPanel: sinon.stub().returns({destroy: sinon.stub()}) + } + let panel, portal, subtree + const item = Symbol('item') + const node = document.createElement('div') + const app = ( + panel = c} + workspace={workspace} + location='left' + options={{some: 'option'}} + getItem={(obj) => { + portal = obj.portal + subtree = obj.subtree + return item + }} + > + + + ) + ReactDom.render(app, node) + assert.equal(workspace.addLeftPanel.callCount, 1) + assert.deepEqual(workspace.addLeftPanel.args[0], [{some: 'option', item: item}]) + assert.equal(portal.getElement().textContent, 'hello') + assert.equal(subtree.getText(), 'hello') + + const app2 = ( + panel = c} + workspace={workspace} + location='left' + options={{some: 'option'}} + getItem={(obj) => { + return item + }} + > + + + ) + ReactDom.render(app2, node) + assert.equal(workspace.addLeftPanel.callCount, 1) + assert.equal(portal.getElement().textContent, 'world') + assert.equal(subtree.getText(), 'world') + + const wrappedPanel = panel.getPanel() + ReactDom.unmountComponentAtNode(node) + assert.equal(wrappedPanel.destroy.callCount, 1) + }) +}) From 276156a135c5010a3160cfa93a0ceb2ff4aff1fc Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Wed, 30 Nov 2016 13:54:06 -0800 Subject: [PATCH 33/47] Add createRenderer React test helper and refactor tests --- test/helpers.js | 49 ++++++++++++++++++++++++++++++++++++++- test/views/panel.test.js | 21 ++++++++--------- test/views/portal.test.js | 23 +++++++++--------- 3 files changed, 70 insertions(+), 23 deletions(-) diff --git a/test/helpers.js b/test/helpers.js index 52d8ab5aaf..638cc23079 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -3,10 +3,13 @@ import fs from 'fs-extra' import path from 'path' import temp from 'temp' + import {Directory} from 'atom' -import GitShellOutStrategy from '../lib/git-shell-out-strategy' +import React from 'react' +import ReactDom from 'react-dom' import Repository from '../lib/models/repository' +import GitShellOutStrategy from '../lib/git-shell-out-strategy' assert.autocrlfEqual = (actual, expected, ...args) => { actual = actual.replace(/\r\n/g, '\n') @@ -149,3 +152,47 @@ export function until (...args) { checker() }) } + +let activeRenderers = [] +export function createRenderer () { + let instance + let lastInstance + let node = document.createElement('div') + // ref function should be reference equal over renders to avoid React + // calling the "old" one with `null` and the "new" one with the instance + const setTopLevelRef = (c) => { instance = c } + const renderer = { + render (app) { + lastInstance = instance + app = React.cloneElement(app, { ref: setTopLevelRef }) + ReactDom.render(app, node) + }, + + get instance () { + return instance + }, + + get lastInstance () { + return lastInstance + }, + + get node () { + return node + }, + + unmount () { + if (node) { + lastInstance = instance + ReactDom.unmountComponentAtNode(node) + node = null + } + } + } + activeRenderers.push(renderer) + return renderer +} + +afterEach(() => { + activeRenderers.forEach(r => r.unmount()) + activeRenderers = [] +}) diff --git a/test/views/panel.test.js b/test/views/panel.test.js index f54d7ab382..83b68b0e2c 100644 --- a/test/views/panel.test.js +++ b/test/views/panel.test.js @@ -6,6 +6,8 @@ import sinon from 'sinon' import Panel from '../../lib/views/panel' +import {createRenderer} from '../helpers' + class Component extends React.Component { render () { return ( @@ -20,15 +22,14 @@ class Component extends React.Component { describe('Panel component', () => { it('renders a React component into an Atom panel', () => { + const renderer = createRenderer() const workspace = { addLeftPanel: sinon.stub().returns({destroy: sinon.stub()}) } - let panel, portal, subtree + let portal, subtree const item = Symbol('item') - const node = document.createElement('div') - const app = ( + let app = ( panel = c} workspace={workspace} location='left' options={{some: 'option'}} @@ -41,15 +42,14 @@ describe('Panel component', () => { ) - ReactDom.render(app, node) + renderer.render(app) assert.equal(workspace.addLeftPanel.callCount, 1) assert.deepEqual(workspace.addLeftPanel.args[0], [{some: 'option', item: item}]) assert.equal(portal.getElement().textContent, 'hello') assert.equal(subtree.getText(), 'hello') - const app2 = ( + app = ( panel = c} workspace={workspace} location='left' options={{some: 'option'}} @@ -60,13 +60,12 @@ describe('Panel component', () => { ) - ReactDom.render(app2, node) + renderer.render(app) assert.equal(workspace.addLeftPanel.callCount, 1) assert.equal(portal.getElement().textContent, 'world') assert.equal(subtree.getText(), 'world') - const wrappedPanel = panel.getPanel() - ReactDom.unmountComponentAtNode(node) - assert.equal(wrappedPanel.destroy.callCount, 1) + renderer.unmount() + assert.equal(renderer.lastInstance.getPanel().destroy.callCount, 1) }) }) diff --git a/test/views/portal.test.js b/test/views/portal.test.js index 30734f77a1..8ee6f736c8 100644 --- a/test/views/portal.test.js +++ b/test/views/portal.test.js @@ -5,6 +5,8 @@ import ReactDom from 'react-dom' import Portal from '../../lib/views/portal' +import {createRenderer} from '../helpers' + class Component extends React.Component { render () { return ( @@ -19,16 +21,15 @@ class Component extends React.Component { describe('Portal', () => { it('renders a subtree into a different dom node', () => { - const node = document.createElement('div') - let portal, oldPortal - ReactDom.render( portal = c}>, node) - assert.equal(portal.getElement().textContent, 'hello') - assert.equal(portal.getRenderedSubtree().getText(), 'hello') - oldPortal = portal - ReactDom.render( portal = c}>, node) - assert.equal(oldPortal, portal) // portal got updated, not recreated - assert.equal(oldPortal.getRenderedSubtree(), portal.getRenderedSubtree()) // subtree also got updated, not recreated - assert.equal(portal.getElement().textContent, 'world') - assert.equal(portal.getRenderedSubtree().getText(), 'world') + const renderer = createRenderer() + renderer.render() + assert.equal(renderer.instance.getElement().textContent, 'hello') + assert.equal(renderer.instance.getRenderedSubtree().getText(), 'hello') + const oldSubtree = renderer.instance.getRenderedSubtree() + renderer.render() + assert.equal(renderer.lastInstance, renderer.instance) + assert.equal(oldSubtree, renderer.instance.getRenderedSubtree()) + assert.equal(renderer.instance.getElement().textContent, 'world') + assert.equal(renderer.instance.getRenderedSubtree().getText(), 'world') }) }) From 12ae9a4e13a7ba46b7969a164120ec833841ab6a Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 30 Nov 2016 17:52:57 -0800 Subject: [PATCH 34/47] Make Panel call prop when Atom panel is destroyed unexpectedly --- lib/views/panel.js | 18 ++++++++++++++++-- test/views/panel.test.js | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/lib/views/panel.js b/lib/views/panel.js index 595e1ccc57..ece616be98 100644 --- a/lib/views/panel.js +++ b/lib/views/panel.js @@ -19,8 +19,7 @@ import Portal from './portal' * You can get the underlying Atom panel via `getPanel()`, but you should * consider controlling the panel via React and the Panel component instead. */ -// TODO: all portals that wrap atom classes should watch for the underlying -// model to be mutated/destroyed and call callbacks appropriately +// TODO: add show/hide props export default class Panel extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, @@ -29,17 +28,27 @@ export default class Panel extends React.Component { ]).isRequired, getItem: React.PropTypes.func, options: React.PropTypes.object, + onDidClosePanel: React.PropTypes.func, } static defaultProps = { options: {}, getItem: ({portal, subtree}) => portal, + onDidClosePanel: (panel) => {}, } componentDidMount () { this.setupPanel(this.props) } + componentWillReceiveProps () { + if (this.didCloseItem) { + console.error( + `Unexpected update in \`Panel\`: the contained panel has been destroyed` + ) + } + } + render () { return this.portal = c}>{this.props.children} } @@ -54,9 +63,14 @@ export default class Panel extends React.Component { const item = props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) const options = Object.assign({}, this.props.options, { item }) this.panel = this.props.workspace[methodName](options) + this.subscriptions = this.panel.onDidDestroy(() => { + this.didCloseItem = true + this.props.onDidClosePanel(this.panel) + }) } componentWillUnmount () { + this.subscriptions && this.subscriptions.dispose() if (this.panel) { this.panel.destroy() } diff --git a/test/views/panel.test.js b/test/views/panel.test.js index 83b68b0e2c..db09cc7068 100644 --- a/test/views/panel.test.js +++ b/test/views/panel.test.js @@ -6,6 +6,8 @@ import sinon from 'sinon' import Panel from '../../lib/views/panel' +import {Emitter} from 'atom' + import {createRenderer} from '../helpers' class Component extends React.Component { @@ -23,8 +25,12 @@ class Component extends React.Component { describe('Panel component', () => { it('renders a React component into an Atom panel', () => { const renderer = createRenderer() + const emitter = new Emitter() const workspace = { - addLeftPanel: sinon.stub().returns({destroy: sinon.stub()}) + addLeftPanel: sinon.stub().returns({ + destroy: sinon.spy(() => emitter.emit('destroy')), + onDidDestroy: (cb) => emitter.on('destroy', cb), + }) } let portal, subtree const item = Symbol('item') @@ -56,6 +62,7 @@ describe('Panel component', () => { getItem={(obj) => { return item }} + onDidClosePanel={() => { throw new Error('Expected onDidClosePanel not to be called') }} > @@ -68,4 +75,29 @@ describe('Panel component', () => { renderer.unmount() assert.equal(renderer.lastInstance.getPanel().destroy.callCount, 1) }) + + it('calls props.onDidClosePanel when the panel is destroyed unexpectedly', () => { + const renderer = createRenderer() + const emitter = new Emitter() + const workspace = { + addLeftPanel: sinon.stub().returns({ + destroy: () => emitter.emit('destroy'), + onDidDestroy: (cb) => emitter.on('destroy', cb), + }) + } + const onDidClosePanel = sinon.stub() + let app = ( + + + + ) + renderer.render(app) + renderer.instance.getPanel().destroy() + assert.equal(onDidClosePanel.callCount, 1) + emitter.dispose() + }) }) From deff4b488cfc799ce31a680aa7f557ad5d007dee Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 30 Nov 2016 18:25:46 -0800 Subject: [PATCH 35/47] :art: extract code into before/afterEach --- test/views/panel.test.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/views/panel.test.js b/test/views/panel.test.js index db09cc7068..b6abc03725 100644 --- a/test/views/panel.test.js +++ b/test/views/panel.test.js @@ -23,15 +23,23 @@ class Component extends React.Component { } describe('Panel component', () => { - it('renders a React component into an Atom panel', () => { - const renderer = createRenderer() - const emitter = new Emitter() - const workspace = { + let renderer, emitter, workspace + beforeEach(() => { + renderer = createRenderer() + emitter = new Emitter() + workspace = { addLeftPanel: sinon.stub().returns({ destroy: sinon.spy(() => emitter.emit('destroy')), onDidDestroy: (cb) => emitter.on('destroy', cb), }) } + }) + + afterEach(() => { + emitter.dispose() + }) + + it('renders a React component into an Atom panel', () => { let portal, subtree const item = Symbol('item') let app = ( @@ -77,14 +85,7 @@ describe('Panel component', () => { }) it('calls props.onDidClosePanel when the panel is destroyed unexpectedly', () => { - const renderer = createRenderer() - const emitter = new Emitter() - const workspace = { - addLeftPanel: sinon.stub().returns({ - destroy: () => emitter.emit('destroy'), - onDidDestroy: (cb) => emitter.on('destroy', cb), - }) - } + const onDidClosePanel = sinon.stub() let app = ( { renderer.render(app) renderer.instance.getPanel().destroy() assert.equal(onDidClosePanel.callCount, 1) - emitter.dispose() }) }) From 9bfe99414d83a460ba62e647a59c2d9879e07ea7 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 30 Nov 2016 18:42:16 -0800 Subject: [PATCH 36/47] Add tests for PaneItem --- lib/views/pane-item.js | 8 ++- test/views/pane-item.test.js | 105 +++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 test/views/pane-item.test.js diff --git a/lib/views/pane-item.js b/lib/views/pane-item.js index b470629249..d89966d221 100644 --- a/lib/views/pane-item.js +++ b/lib/views/pane-item.js @@ -28,7 +28,7 @@ export default class PaneItem extends React.Component { static defaultProps = { getItem: ({portal, subtree}) => portal, - onDidCloseItem: () => {}, + onDidCloseItem: (paneItem) => {}, } componentDidMount () { @@ -55,11 +55,15 @@ export default class PaneItem extends React.Component { this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => { if (item === this.paneItem) { this.didCloseItem = true - this.props.onDidCloseItem() + this.props.onDidCloseItem(this.paneItem) } }) } + getPaneItem () { + return this.paneItem + } + componentWillUnmount () { this.subscriptions && this.subscriptions.dispose() if (this.paneItem && !this.didCloseItem) { diff --git a/test/views/pane-item.test.js b/test/views/pane-item.test.js new file mode 100644 index 0000000000..acef32864c --- /dev/null +++ b/test/views/pane-item.test.js @@ -0,0 +1,105 @@ +/** @babel */ + +import React from 'react' +import ReactDom from 'react-dom' +import sinon from 'sinon' + +import PaneItem from '../../lib/views/pane-item' + +import {Emitter} from 'atom' + +import {createRenderer} from '../helpers' + +class Component extends React.Component { + render () { + return ( +
{this.props.text}
+ ) + } + + getText () { + return this.props.text + } +} + +describe('PaneItem component', () => { + let renderer, emitter, workspace, activePane + beforeEach(() => { + renderer = createRenderer() + emitter = new Emitter() + const paneItem = { + destroy: sinon.spy(() => emitter.emit('destroy', {item: paneItem})) + } + activePane = { + addItem: sinon.spy((item) => { + return paneItem + }), + activateItem: sinon.stub() + } + workspace = { + getActivePane: sinon.stub().returns(activePane), + paneForItem: sinon.stub().returns(activePane), + onDidDestroyPaneItem: (cb) => emitter.on('destroy', cb), + } + }) + + afterEach(() => { + emitter.dispose() + }) + + it('renders a React component into an Atom pane item', () => { + let portal, subtree + const item = Symbol('item') + let app = ( + { + portal = obj.portal + subtree = obj.subtree + return item + }} + > + + + ) + renderer.render(app) + assert.equal(activePane.addItem.callCount, 1) + assert.deepEqual(activePane.addItem.args[0], [item]) + assert.equal(portal.getElement().textContent, 'hello') + assert.equal(subtree.getText(), 'hello') + + app = ( + { + return item + }} + onDidCloseItem={() => { throw new Error('Expected onDidCloseItem not to be called') }} + > + + + ) + renderer.render(app) + assert.equal(activePane.addItem.callCount, 1) + assert.equal(portal.getElement().textContent, 'world') + assert.equal(subtree.getText(), 'world') + + renderer.unmount() + assert.equal(renderer.lastInstance.getPaneItem().destroy.callCount, 1) + }) + + it('calls props.onDidCloseItem when the pane item is destroyed unexpectedly', () => { + const onDidCloseItem = sinon.stub() + let app = ( + + + + ) + renderer.render(app) + renderer.instance.getPaneItem().destroy() + assert.equal(onDidCloseItem.callCount, 1) + }) +}) From 287eed5b47d4e448a7faa17f423aa23479ea1fe4 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 30 Nov 2016 18:43:09 -0800 Subject: [PATCH 37/47] :arrow_up: sinon@2.0.0-pre.4 This fixes CSP errors with old versions of sinon.spy --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4ba392ac47..21ea3ee3e8 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "fs-extra": "^1.0.0", "mkdirp": "^0.5.1", "simulant": "^0.2.2", - "sinon": "^1.17.4", + "sinon": "2.0.0-pre.4", "temp": "^0.8.3" }, "consumedServices": { From c77d6908ab2e22d60cb42296ee32e9c9be5e8837 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 30 Nov 2016 19:06:18 -0800 Subject: [PATCH 38/47] Move subset of package state into GitController --- lib/controllers/git-controller.js | 39 +++++- lib/github-package.js | 28 +--- test/github-package.test.js | 208 ------------------------------ 3 files changed, 43 insertions(+), 232 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 716ec5560d..936761e42d 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -29,12 +29,24 @@ export default class GitController extends React.Component { notificationManager: React.PropTypes.object.isRequired, repository: React.PropTypes.object, statusBar: React.PropTypes.object, + savedState: React.PropTypes.object, + } + + static defaultProps = { + savedState: {}, + } + + serialize () { + return { + gitPanelActive: this.state.gitPanelActive + } } constructor (props, context) { super(props, context) this.state = { - ...nullFilePatchState + ...nullFilePatchState, + gitPanelActive: props.savedState.gitPanelActive, } this.showFilePatchForPath = this.showFilePatchForPath.bind(this) @@ -42,6 +54,10 @@ export default class GitController extends React.Component { this.didChangeAmending = this.didChangeAmending.bind(this) this.onRepoRefresh = this.onRepoRefresh.bind(this) this.subscriptions = new CompositeDisposable() + this.subscriptions.add( + props.commandRegistry.add('atom-workspace', {'git:toggle-git-panel': this.toggleGitPanel.bind(this)}), + props.commandRegistry.add('atom-workspace', {'git:focus-git-panel': this.focusGitPanel.bind(this)}), + ) } render () { @@ -56,7 +72,7 @@ export default class GitController extends React.Component { /> - {this.props.gitPanelActive ? this.renderGitPanel() : null} + {this.state.gitPanelActive ? this.renderGitPanel() : null} {(this.state.filePath && this.state.filePatch) ? this.renderFilePatchController() : null}
) @@ -68,6 +84,7 @@ export default class GitController extends React.Component { workspace={this.props.workspace} location="right" getItem={({subtree}) => subtree.getWrappedComponent()} + onDidClosePanel={() => this.setState({gitPanelActive: false})} > this.gitPanelController = c} reattachDomNode={false}> ({gitPanelActive: !state.gitPanelActive})) + } + + focusGitPanel () { + if (!this.state.gitPanelActive) { + this.setState({gitPanelActive: true}, () => { + // TODO: why doesn't a setTimeout of 0 or even 100 work here? + // Suspect it has to do with the command palette closing... + setTimeout(() => this.gitPanelController.getWrappedComponent().focus(), 200) + }) + } + } + focusFilePatchView() {} } diff --git a/lib/github-package.js b/lib/github-package.js index 1e3ff083ef..486a11df2a 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -23,11 +23,12 @@ export default class GithubPackage { : new FileSystemChangeObserver() this.activeRepository = null this.repositoriesByProjectDirectory = new Map() - this.gitPanelActive = false this.subscriptions = new CompositeDisposable() + this.savedState = {} } async activate (state = {}) { + this.savedState = state this.subscriptions = new CompositeDisposable() await this.updateActiveRepository() await this.changeObserver.start() @@ -36,10 +37,7 @@ export default class GithubPackage { this.project.onDidChangePaths(this.didChangeProjectPaths.bind(this)), this.workspace.onDidChangeActivePaneItem(this.didChangeActivePaneItem.bind(this)), this.changeObserver.onDidChange(this.refreshActiveRepository.bind(this)), - this.commandRegistry.add('atom-workspace', {'git:toggle-git-panel': this.toggleGitPanel.bind(this)}), - this.commandRegistry.add('atom-workspace', {'git:focus-git-panel': this.focusGitPanel.bind(this)}) ) - this.gitPanelActive = !!state.gitPanelActive if (state.activeRepositoryDir) { const dir = new Directory(state.activeRepositoryDir) const repo = await this.repositoryForProjectDirectory(dir) @@ -53,12 +51,12 @@ export default class GithubPackage { serialize () { const activeRepository = this.getActiveRepository() return { - gitPanelActive: this.gitPanelActive, activeRepositoryDir: activeRepository ? activeRepository.getWorkingDirectoryPath() : null, + gitController: this.controller.serialize(), } } - rerender (callback = null) { + rerender () { if (!this.element) { this.element = document.createElement('div') this.subscriptions.add(new Disposable(() => { @@ -75,8 +73,8 @@ export default class GithubPackage { notificationManager={this.notificationManager} repository={this.getActiveRepository()} statusBar={this.statusBar} - gitPanelActive={this.gitPanelActive} - />, this.element, typeof callback === 'function' ? callback : null + savedState={this.savedState.gitController} + />, this.element ) } @@ -110,20 +108,6 @@ export default class GithubPackage { this.filePatchController.refs.filePatchView.element.focus() } - toggleGitPanel (callback) { - this.gitPanelActive = !this.gitPanelActive - this.rerender(callback) - } - - focusGitPanel () { - if (!this.gitPanelActive) { - this.toggleGitPanel(() => { - // TODO: why doesn't a setTimeout of 0 or even 100 work here? - // Suspect it has to do with the command palette closing... - setTimeout(() => this.controller.gitPanelController.getWrappedComponent().focus(), 200) - }) - } - } getActiveRepository () { return this.activeRepository diff --git a/test/github-package.test.js b/test/github-package.test.js index 9e855cb05a..8ff270d985 100644 --- a/test/github-package.test.js +++ b/test/github-package.test.js @@ -160,212 +160,4 @@ describe('GithubPackage', () => { assert.isNull(githubPackage.getActiveRepository()) }) }) - - // TODO: move into GitController tests - // describe('showMergeConflictFileForPath(filePath)', () => { - // it('opens the file as a pane item if it exsits', async () => { - // const workdirPath = await cloneRepository('merge-conflict') - // const repository = await buildRepository(workdirPath) - // githubPackage.getActiveRepository = function () { return repository } - // await githubPackage.gitPanelController.props.didSelectMergeConflictFile('added-to-both.txt') - // assert.equal(workspace.getActivePaneItem().getPath(), path.join(workdirPath, 'added-to-both.txt')) - // }) - // - // describe('when the file doesn\'t exist', () => { - // it('shows an info notification and does not open the file', async () => { - // const workdirPath = await cloneRepository('merge-conflict') - // const repository = await buildRepository(workdirPath) - // githubPackage.getActiveRepository = function () { return repository } - // fs.unlinkSync(path.join(workdirPath, 'added-to-both.txt')) - // - // assert.equal(notificationManager.getNotifications().length, 0) - // await githubPackage.gitPanelController.props.didSelectMergeConflictFile('added-to-both.txt') - // assert.isUndefined(workspace.getActivePaneItem()) - // assert.equal(notificationManager.getNotifications().length, 1) - // }) - // }) - // }) - - // TODO: move into GitController tests - // describe('showFilePatchForPath(filePath, staged, {amending, activate})', () => { - // describe('when a file is selected in the staging panel', () => { - // it('shows a FilePatchView for the selected file as a pane item, always updating the existing pane item', async () => { - // const workdirPath = await cloneRepository('three-files') - // const repository = await buildRepository(workdirPath) - // - // fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') - // fs.writeFileSync(path.join(workdirPath, 'd.txt'), 'new-file', 'utf8') - // await repository.stageFiles(['d.txt']) - // - // githubPackage.getActiveRepository = function () { return repository } - // - // assert.isNull(githubPackage.filePatchController) - // - // await githubPackage.showFilePatchForPath('a.txt', 'unstaged', {activate: true}) - // assert(githubPackage.filePatchController) - // assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'a.txt') - // assert.equal(githubPackage.filePatchController.props.repository, repository) - // assert.equal(githubPackage.filePatchController.props.stagingStatus, 'unstaged') - // assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - // - // const existingFilePatchView = githubPackage.filePatchController - // const originalPane = workspace.getActivePane() - // originalPane.splitRight() // activate a different pane - // assert.isUndefined(workspace.getActivePaneItem()) - // - // await githubPackage.showFilePatchForPath('d.txt', 'staged') - // assert.equal(githubPackage.filePatchController, existingFilePatchView) - // assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'd.txt') - // assert.equal(githubPackage.filePatchController.props.repository, repository) - // assert.equal(githubPackage.filePatchController.props.stagingStatus, 'staged') - // assert.equal(originalPane.getActiveItem(), githubPackage.filePatchController) - // - // originalPane.getActiveItem().destroy() - // assert.isUndefined(workspace.getActivePaneItem()) - // assert.isNull(githubPackage.filePatchController) - // - // await githubPackage.showFilePatchForPath('d.txt', 'staged', {activate: true}) - // assert.notEqual(githubPackage.filePatchController, existingFilePatchView) - // assert.equal(githubPackage.filePatchController.props.filePatch.getPath(), 'd.txt') - // assert.equal(githubPackage.filePatchController.props.repository, repository) - // assert.equal(githubPackage.filePatchController.props.stagingStatus, 'staged') - // assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - // }) - // }) - - // TODO: Move into GitController tests - // describe('when there is a change to the repo', () => { - // it('updates the FilePatchController with the correct filePatch', async () => { - // const workdirPath = await cloneRepository('multiple-commits') - // const repository = await buildRepository(workdirPath) - // - // githubPackage.getActiveRepository = function () { return repository } - // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change', 'utf8') - // await githubPackage.showFilePatchForPath('file.txt', 'unstaged', {activate: true}) - // - // let filePatchController = githubPackage.filePatchController - // sinon.spy(filePatchController, 'update') - // - // // udpate unstaged file patch - // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nplus more changes', 'utf8') - // await repository.refresh() - // // repository refresh causes async actions so waiting for this is the best way to determine the update has finished - // await etch.getScheduler().getNextUpdatePromise() - // const unstagedFilePatch = await repository.getFilePatchForPath('file.txt') - // assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, unstagedFilePatch) - // - // // update staged file patch - // await repository.stageFiles(['file.txt']) - // await githubPackage.showFilePatchForPath('file.txt', 'staged', {activate: true}) - // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nplus more changes\nand more!', 'utf8') - // await repository.stageFiles(['file.txt']) - // await repository.refresh() - // // repository refresh causes async actions so waiting for this is the best way to determine the update has finished - // await etch.getScheduler().getNextUpdatePromise() - // const stagedFilePatch = await repository.getFilePatchForPath('file.txt', {staged: true}) - // assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, stagedFilePatch) - // - // // update amended file patch - // // didChangeAmending destroys current filePatchController - // githubPackage.gitPanelController.props.didChangeAmending() - // await githubPackage.showFilePatchForPath('file.txt', 'staged', {activate: true, amending: true}) - // filePatchController = githubPackage.filePatchController - // sinon.spy(filePatchController, 'update') - // fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change file being amended', 'utf8') - // await repository.stageFiles(['file.txt']) - // await repository.refresh() - // // repository refresh causes async actions so waiting for this is the best way to determine the update has finished - // await etch.getScheduler().getNextUpdatePromise() - // const amendedStagedFilePatch = await repository.getFilePatchForPath('file.txt', {staged: true, amending: true}) - // assert.deepEqual(filePatchController.update.lastCall.args[0].filePatch, amendedStagedFilePatch) - // }) - // }) - // }) - - // TODO: Move into GitController tests - // describe('when amend mode is toggled in the staging panel while viewing a staged change', () => { - // it('closes the file patch pane item', async () => { - // const workdirPath = await cloneRepository('three-files') - // const repository = await buildRepository(workdirPath) - // fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') - // await repository.stageFiles(['a.txt']) - // - // githubPackage.getActiveRepository = function () { return repository } - // - // await githubPackage.showFilePatchForPath('a.txt', 'staged', {activate: true}) - // assert.isOk(githubPackage.filePatchController) - // assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) - // - // githubPackage.gitPanelController.props.didChangeAmending() - // assert.isNull(githubPackage.filePatchController) - // assert.isUndefined(workspace.getActivePaneItem()) - // }) - // }) - // - // describe('when the changed files label in the status bar is clicked', () => { - // it('toggles the git panel', async () => { - // const workdirPath = await cloneRepository('three-files') - // project.setPaths([workdirPath]) - // await workspace.open(path.join(workdirPath, 'a.txt')) - // await githubPackage.updateActiveRepository() - // - // githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() - // assert.equal(workspace.getRightPanels().length, 1) - // - // githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() - // assert.equal(workspace.getRightPanels().length, 0) - // - // githubPackage.statusBarTileController.refs.changedFilesCountView.props.didClick() - // assert.equal(workspace.getRightPanels().length, 1) - // }) - // }) - - describe('toggleGitPanel()', () => { - it('shows-and-focuses or hides the git panel', async () => { - sinon.stub(githubPackage, 'rerender') - const workdirPath = await cloneRepository('three-files') - project.setPaths([workdirPath]) - await workspace.open(path.join(workdirPath, 'a.txt')) - await githubPackage.activate() - - githubPackage.rerender.reset() - assert.isFalse(githubPackage.gitPanelActive) - githubPackage.toggleGitPanel() - assert.isTrue(githubPackage.gitPanelActive) - assert.equal(githubPackage.rerender.callCount, 1) - // TODO: fix this - // assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - githubPackage.toggleGitPanel() - assert.isFalse(githubPackage.gitPanelActive) - assert.equal(githubPackage.rerender.callCount, 2) - }) - }) - - // TODO: fix - // describe('focusGitPanel()', () => { - // it('shows-and-focuses the git panel', async () => { - // const workspaceElement = viewRegistry.getView(workspace) - // document.body.appendChild(workspaceElement) - // const workdirPath = await cloneRepository('three-files') - // project.setPaths([workdirPath]) - // await workspace.open(path.join(workdirPath, 'a.txt')) - // await githubPackage.activate() - // - // assert.equal(workspace.getRightPanels().length, 0) - // githubPackage.focusGitPanel() - // assert.equal(workspace.getRightPanels().length, 1) - // assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) - // assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - // - // githubPackage.toggleGitPanel() - // assert.equal(workspace.getRightPanels().length, 0) - // - // githubPackage.focusGitPanel() - // assert.equal(workspace.getRightPanels().length, 1) - // assert.equal(workspace.getRightPanels()[0].item, githubPackage.gitPanelController) - // assert(githubPackage.gitPanelController.refs.gitPanel.refs.stagingView.isFocused()) - // - // workspaceElement.remove() - // }) - // }) }) From 49635c44fc41ef47a4e7db5d82864c77b9a936c0 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 16:53:11 -0800 Subject: [PATCH 39/47] Always use this.props in Atom wrapper components --- lib/views/pane-item.js | 6 +++--- lib/views/panel.js | 13 ++++++------- lib/views/status-bar.js | 12 ++++++------ 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/views/pane-item.js b/lib/views/pane-item.js index d89966d221..37d299f055 100644 --- a/lib/views/pane-item.js +++ b/lib/views/pane-item.js @@ -32,7 +32,7 @@ export default class PaneItem extends React.Component { } componentDidMount () { - this.setupPaneItem(this.props) + this.setupPaneItem() } componentWillReceiveProps () { @@ -47,10 +47,10 @@ export default class PaneItem extends React.Component { return this.portal = c}>{this.props.children} } - setupPaneItem (props) { + setupPaneItem () { if (this.paneItem) return - const item = props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) + const item = this.props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) this.paneItem = this.props.workspace.getActivePane().addItem(item) this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => { if (item === this.paneItem) { diff --git a/lib/views/panel.js b/lib/views/panel.js index ece616be98..c2e25556a7 100644 --- a/lib/views/panel.js +++ b/lib/views/panel.js @@ -19,7 +19,6 @@ import Portal from './portal' * You can get the underlying Atom panel via `getPanel()`, but you should * consider controlling the panel via React and the Panel component instead. */ -// TODO: add show/hide props export default class Panel extends React.Component { static propTypes = { workspace: React.PropTypes.object.isRequired, @@ -38,10 +37,10 @@ export default class Panel extends React.Component { } componentDidMount () { - this.setupPanel(this.props) + this.setupPanel() } - componentWillReceiveProps () { + componentWillReceiveProps (newProps) { if (this.didCloseItem) { console.error( `Unexpected update in \`Panel\`: the contained panel has been destroyed` @@ -53,15 +52,15 @@ export default class Panel extends React.Component { return this.portal = c}>{this.props.children} } - setupPanel (props) { + setupPanel () { if (this.panel) return // "left" => "Left" - const location = props.location.substr(0, 1).toUpperCase() + props.location.substr(1) + const location = this.props.location.substr(0, 1).toUpperCase() + this.props.location.substr(1) const methodName = `add${location}Panel` - const item = props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) - const options = Object.assign({}, this.props.options, { item }) + const item = this.props.getItem({portal: this.portal, subtree: this.portal.getRenderedSubtree()}) + const options = Object.assign({}, this.props.options, { visible: this.props.visible, item }) this.panel = this.props.workspace[methodName](options) this.subscriptions = this.panel.onDidDestroy(() => { this.didCloseItem = true diff --git a/lib/views/status-bar.js b/lib/views/status-bar.js index 727b2e06c1..893cb7f4eb 100644 --- a/lib/views/status-bar.js +++ b/lib/views/status-bar.js @@ -14,23 +14,23 @@ export default class StatusBar extends React.Component { } componentDidMount () { - this.consumeStatusBar(this.props) + this.consumeStatusBar() } componentDidUpdate () { - this.consumeStatusBar(this.props) + this.consumeStatusBar() } render () { return
this.container = c}>{this.props.children}
} - consumeStatusBar (props) { + consumeStatusBar () { if (this.tile) return - if (!props.statusBar) return + if (!this.props.statusBar) return const componentElement = this.container.children[0] - this.tile = props.statusBar.addRightTile({item: componentElement, priority: -50}) - this.props.onConsumeStatusBar(props.statusBar) + this.tile = this.props.statusBar.addRightTile({item: componentElement, priority: -50}) + this.props.onConsumeStatusBar(this.props.statusBar) } } From d7171f1c9eb1b00e31104ee96189cac44e75a0d0 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 16:54:27 -0800 Subject: [PATCH 40/47] Add visible prop to Panel --- lib/views/panel.js | 6 ++++++ test/views/panel.test.js | 45 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/views/panel.js b/lib/views/panel.js index c2e25556a7..4c357afcb8 100644 --- a/lib/views/panel.js +++ b/lib/views/panel.js @@ -28,12 +28,14 @@ export default class Panel extends React.Component { getItem: React.PropTypes.func, options: React.PropTypes.object, onDidClosePanel: React.PropTypes.func, + visible: React.PropTypes.bool, } static defaultProps = { options: {}, getItem: ({portal, subtree}) => portal, onDidClosePanel: (panel) => {}, + visible: true, } componentDidMount () { @@ -46,6 +48,10 @@ export default class Panel extends React.Component { `Unexpected update in \`Panel\`: the contained panel has been destroyed` ) } + + if (this.panel && this.props.visible !== newProps.visible) { + this.panel[newProps.visible ? 'show' : 'hide']() + } } render () { diff --git a/test/views/panel.test.js b/test/views/panel.test.js index b6abc03725..dfa8d8592a 100644 --- a/test/views/panel.test.js +++ b/test/views/panel.test.js @@ -31,6 +31,8 @@ describe('Panel component', () => { addLeftPanel: sinon.stub().returns({ destroy: sinon.spy(() => emitter.emit('destroy')), onDidDestroy: (cb) => emitter.on('destroy', cb), + show: sinon.stub(), + hide: sinon.stub(), }) } }) @@ -58,7 +60,7 @@ describe('Panel component', () => { ) renderer.render(app) assert.equal(workspace.addLeftPanel.callCount, 1) - assert.deepEqual(workspace.addLeftPanel.args[0], [{some: 'option', item: item}]) + assert.deepEqual(workspace.addLeftPanel.args[0], [{some: 'option', visible: true, item: item}]) assert.equal(portal.getElement().textContent, 'hello') assert.equal(subtree.getText(), 'hello') @@ -85,7 +87,6 @@ describe('Panel component', () => { }) it('calls props.onDidClosePanel when the panel is destroyed unexpectedly', () => { - const onDidClosePanel = sinon.stub() let app = ( { renderer.instance.getPanel().destroy() assert.equal(onDidClosePanel.callCount, 1) }) + + describe('when updating the visible prop', () => { + it('shows or hides the panel', () => { + let app = ( + + + + ) + renderer.render(app) + + const panel = renderer.instance.getPanel() + app = ( + + + + ) + renderer.render(app) + assert.equal(panel.hide.callCount, 1) + + app = ( + + + + ) + renderer.render(app) + assert.equal(panel.show.callCount, 1) + }) + }) }) From c86b83f235c6d83b32a3f7b5f78289b89cc2603c Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 16:54:43 -0800 Subject: [PATCH 41/47] :arrow_up: atom-mocha-test-runner@0.3.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 21ea3ee3e8..6d3354d98a 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "what-the-diff": "^0.2.0" }, "devDependencies": { - "atom-mocha-test-runner": "0.3.0", + "atom-mocha-test-runner": "0.3.1", "babel-eslint": "^6.0.4", "chai": "^3.5.0", "chai-as-promised": "^5.3.0", From 057273fa7b681db76573b67856a88a22f5988b60 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 16:57:25 -0800 Subject: [PATCH 42/47] Make until() error have sync stack trace --- test/helpers.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/helpers.js b/test/helpers.js index 638cc23079..0552824c8f 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -135,6 +135,7 @@ export function until (...args) { message = message || 'something happens' timeout = timeout || 5000 + const error = new Error(`timeout: timed out after ${timeout} msec waiting until ${message}`) return new Promise((resolve, reject) => { const checker = () => { @@ -144,7 +145,7 @@ export function until (...args) { const now = new Date().getTime() const delta = now - start if (delta > timeout) { - return reject(new Error(`timeout: timed out after ${timeout} msec waiting until ${message}`)) + return reject(error) } else { setTimeout(checker) } From b85ea2f15bba282639ce318f62bc658efc4fc318 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 16:59:34 -0800 Subject: [PATCH 43/47] Finish React entry point refactor --- lib/controllers/file-patch-controller.js | 13 +- lib/controllers/git-controller.js | 73 +++++-- lib/controllers/git-panel-controller.js | 4 +- lib/views/file-patch-view.js | 4 + lib/views/git-panel-view.js | 4 +- lib/views/staging-view.js | 1 - package.json | 2 + test/controllers/git-controller.test.js | 250 +++++++++++++++++++++++ test/views/staging-view.test.js | 1 - 9 files changed, 322 insertions(+), 30 deletions(-) create mode 100644 test/controllers/git-controller.test.js diff --git a/lib/controllers/file-patch-controller.js b/lib/controllers/file-patch-controller.js index f034bde207..dd81237310 100644 --- a/lib/controllers/file-patch-controller.js +++ b/lib/controllers/file-patch-controller.js @@ -5,7 +5,6 @@ import {Emitter, CompositeDisposable} from 'atom' import etch from 'etch' import FilePatchView from '../views/file-patch-view' -import ModelObserver from '../models/model-observer' export default class FilePatchController { constructor (props) { @@ -15,23 +14,17 @@ export default class FilePatchController { this.unstageHunk = this.unstageHunk.bind(this) this.stageLines = this.stageLines.bind(this) this.unstageLines = this.unstageLines.bind(this) - this.repositoryObserver = new ModelObserver({ - didUpdate: () => this.props.onRepoRefresh && this.props.onRepoRefresh() - }) - this.repositoryObserver.setActiveModel(props.repository) etch.initialize(this) } async update (props) { this.props = Object.assign({}, this.props, props) this.emitter.emit('did-change-title', this.getTitle()) - await this.repositoryObserver.setActiveModel(props.repository) return etch.update(this) } destroy () { this.emitter.emit('did-destroy') - this.repositoryObserver.destroy() return etch.destroy(this) } @@ -105,4 +98,10 @@ export default class FilePatchController { didDestroyFilePatch () { this.destroy() } + + focus () { + if (this.refs.filePatchView) { + this.refs.filePatchView.focus() + } + } } diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 936761e42d..87046f26a3 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -14,6 +14,7 @@ import PaneItem from '../views/pane-item' import FilePatchController from './file-patch-controller' import GitPanelController from './git-panel-controller' import StatusBarTileController from './status-bar-tile-controller' +import ModelObserver from '../models/model-observer' const nullFilePatchState = { filePath: null, @@ -46,38 +47,59 @@ export default class GitController extends React.Component { super(props, context) this.state = { ...nullFilePatchState, - gitPanelActive: props.savedState.gitPanelActive, + gitPanelActive: !!props.savedState.gitPanelActive, } this.showFilePatchForPath = this.showFilePatchForPath.bind(this) this.showMergeConflictFileForPath = this.showMergeConflictFileForPath.bind(this) this.didChangeAmending = this.didChangeAmending.bind(this) this.onRepoRefresh = this.onRepoRefresh.bind(this) + this.toggleGitPanel = this.toggleGitPanel.bind(this) + this.openAndFocusGitPanel = this.openAndFocusGitPanel.bind(this) + this.focusFilePatchView = this.focusFilePatchView.bind(this) + this.focusGitPanel = this.focusGitPanel.bind(this) this.subscriptions = new CompositeDisposable() this.subscriptions.add( - props.commandRegistry.add('atom-workspace', {'git:toggle-git-panel': this.toggleGitPanel.bind(this)}), - props.commandRegistry.add('atom-workspace', {'git:focus-git-panel': this.focusGitPanel.bind(this)}), + props.commandRegistry.add('atom-workspace', { + 'git:toggle-git-panel': this.toggleGitPanel.bind(this), + 'git:focus-git-panel': this.openAndFocusGitPanel.bind(this), + }) ) + + this.repositoryObserver = new ModelObserver({ + didUpdate: () => this.onRepoRefresh() + }) + this.repositoryObserver.setActiveModel(props.repository) + } + + componentWillReceiveProps(newProps) { + this.repositoryObserver.setActiveModel(newProps.repository) } render () { return (
- this.onConsumeStatusBar(sb)}> - - - - - {this.state.gitPanelActive ? this.renderGitPanel() : null} + {this.renderStatusBarTile()} + {this.renderGitPanel()} {(this.state.filePath && this.state.filePatch) ? this.renderFilePatchController() : null}
) } + renderStatusBarTile () { + return ( + this.onConsumeStatusBar(sb)}> + + + + + ) + } + renderGitPanel () { return ( subtree.getWrappedComponent()} onDidClosePanel={() => this.setState({gitPanelActive: false})} + visible={!!this.state.gitPanelActive} > this.gitPanelController = c} reattachDomNode={false}> { // TODO: can be better done w/ a prop? - if (activate) { + if (activate && this.filePatchControllerPane) { this.filePatchControllerPane.activate() } }) @@ -151,7 +174,7 @@ export default class GitController extends React.Component { } onRepoRefresh () { - this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: this.state.amending}) + return this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: this.state.amending}) } async showMergeConflictFileForPath (relativeFilePath, {focus} = {}) { @@ -164,22 +187,34 @@ export default class GitController extends React.Component { } didChangeAmending (isAmending) { - this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: isAmending}) + return this.showFilePatchForPath(this.state.filePath, this.state.stagingStatus, {amending: isAmending}) } toggleGitPanel () { this.setState(state => ({gitPanelActive: !state.gitPanelActive})) } - focusGitPanel () { + openAndFocusGitPanel () { if (!this.state.gitPanelActive) { this.setState({gitPanelActive: true}, () => { // TODO: why doesn't a setTimeout of 0 or even 100 work here? // Suspect it has to do with the command palette closing... - setTimeout(() => this.gitPanelController.getWrappedComponent().focus(), 200) + setTimeout(() => this.focusGitPanel(), 200) }) + } else { + this.focusGitPanel() + } + } + + focusGitPanel () { + if (this.gitPanelController) { + this.gitPanelController.getWrappedComponent().focus() } } - focusFilePatchView() {} + focusFilePatchView () { + if (this.filePatchController) { + this.filePatchController.getWrappedComponent().focus() + } + } } diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index e347dc6186..4131dd21c6 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -174,6 +174,8 @@ export default class GitPanelController { } focus () { - if (this.refs.gitPanel) this.refs.gitPanel.focus() + if (this.refs.gitPanel) { + this.refs.gitPanel.focus() + } } } diff --git a/lib/views/file-patch-view.js b/lib/views/file-patch-view.js index a163169402..4ab3fc726c 100644 --- a/lib/views/file-patch-view.js +++ b/lib/views/file-patch-view.js @@ -205,4 +205,8 @@ export default class FilePatchView { throw new Error(`Unknown stagingStatus: ${this.props.stagingStatus}`) } } + + focus () { + this.element.focus() + } } diff --git a/lib/views/git-panel-view.js b/lib/views/git-panel-view.js index 27e595a004..a9d97d92f2 100644 --- a/lib/views/git-panel-view.js +++ b/lib/views/git-panel-view.js @@ -77,6 +77,8 @@ export default class GitPanelView { } focus () { - if (this.refs.stagingView) this.refs.stagingView.focus() + if (this.refs.stagingView) { + this.refs.stagingView.focus() + } } } diff --git a/lib/views/staging-view.js b/lib/views/staging-view.js index 1e378bf2e9..f04167d763 100644 --- a/lib/views/staging-view.js +++ b/lib/views/staging-view.js @@ -271,7 +271,6 @@ export default class StagingView { focus () { this.element.focus() - this.onDidChangeSelectedItem() } isFocused () { diff --git a/package.json b/package.json index 6d3354d98a..118457ef36 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "chai": "^3.5.0", "chai-as-promised": "^5.3.0", "dedent-js": "^1.0.1", + "enzyme": "^2.6.0", "eslint": "2.10.2", "eslint-config-standard": "^5.2.0", "eslint-plugin-babel": "^3.2.0", @@ -44,6 +45,7 @@ "eslint-plugin-standard": "^1.3.2", "fs-extra": "^1.0.0", "mkdirp": "^0.5.1", + "react-addons-test-utils": "^15.4.1", "simulant": "^0.2.2", "sinon": "2.0.0-pre.4", "temp": "^0.8.3" diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js new file mode 100644 index 0000000000..7347f21e60 --- /dev/null +++ b/test/controllers/git-controller.test.js @@ -0,0 +1,250 @@ +/** @babel */ + +import path from 'path' +import fs from 'fs' + +import React from 'react' +import ReactDom from 'react-dom' +import ReactTestUtils from 'react-addons-test-utils' +import sinon from 'sinon' +import {shallow} from 'enzyme' + +import {cloneRepository, buildRepository, until} from '../helpers' + +import GitController from '../../lib/controllers/git-controller' + +import {expect} from 'chai' + +describe('GitController', () => { + let atomEnv, workspace, commandRegistry, notificationManager, app + + beforeEach(async () => { + atomEnv = global.buildAtomEnvironment() + workspace = atomEnv.workspace + commandRegistry = atomEnv.commands + notificationManager = atomEnv.notifications + app = + }) + + afterEach(() => { + atomEnv.destroy() + }) + + describe('showMergeConflictFileForPath(filePath)', () => { + it('opens the file as a pending pane item if it exsits', async () => { + const workdirPath = await cloneRepository('merge-conflict') + const repository = await buildRepository(workdirPath) + sinon.spy(workspace, 'open') + app = React.cloneElement(app, {repository}) + const wrapper = shallow(app) + await wrapper.instance().showMergeConflictFileForPath('added-to-both.txt') + + assert.equal(workspace.open.callCount, 1) + assert.deepEqual(workspace.open.args[0], [path.join(workdirPath, 'added-to-both.txt'), {activatePane: false, pending: true}]) + }) + + describe('when the file doesn\'t exist', () => { + it('shows an info notification and does not open the file', async () => { + const workdirPath = await cloneRepository('merge-conflict') + const repository = await buildRepository(workdirPath) + fs.unlinkSync(path.join(workdirPath, 'added-to-both.txt')) + + sinon.spy(notificationManager, 'addInfo') + sinon.spy(workspace, 'open') + app = React.cloneElement(app, {repository}) + const wrapper = shallow(app) + + assert.equal(notificationManager.getNotifications().length, 0) + await wrapper.instance().showMergeConflictFileForPath('added-to-both.txt') + assert.equal(workspace.open.callCount, 0) + assert.equal(notificationManager.addInfo.callCount, 1) + assert.deepEqual(notificationManager.addInfo.args[0], ['File has been deleted.']) + }) + }) + }) + + describe('rendering a FilePatch', () => { + it('renders the FilePatchController based on state', async () => { + const workdirPath = await cloneRepository('three-files') + const repository = await buildRepository(workdirPath) + app = React.cloneElement(app, {repository}) + const wrapper = shallow(app) + + wrapper.setState({ + filePath: null, + filePatch: null, + stagingStatus: null, + }) + assert.equal(wrapper.find('FilePatchController').length, 0) + + const state = { + filePath: 'path', + filePatch: Symbol('filePatch'), + stagingStatus: 'stagingStatus', + } + wrapper.setState(state) + assert.equal(wrapper.find('FilePatchController').length, 1) + assert.equal(wrapper.find('PaneItem').length, 1) + assert.equal(wrapper.find('PaneItem FilePatchController').length, 1) + assert.equal(wrapper.find('FilePatchController').prop('filePatch'), state.filePatch) + assert.equal(wrapper.find('FilePatchController').prop('stagingStatus'), state.stagingStatus) + assert.equal(wrapper.find('FilePatchController').prop('repository'), app.props.repository) + }) + }) + + describe('showFilePatchForPath(filePath, staged, {amending, activate})', () => { + describe('when a file is selected in the staging panel', () => { + it('sets appropriate state', async () => { + const workdirPath = await cloneRepository('three-files') + const repository = await buildRepository(workdirPath) + + fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'change', 'utf8') + fs.writeFileSync(path.join(workdirPath, 'd.txt'), 'new-file', 'utf8') + await repository.stageFiles(['d.txt']) + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + await wrapper.instance().showFilePatchForPath('a.txt', 'unstaged') + + assert.equal(wrapper.state('filePath'), 'a.txt') + assert.equal(wrapper.state('filePatch').getPath(), 'a.txt') + assert.equal(wrapper.state('stagingStatus'), 'unstaged') + + await wrapper.instance().showFilePatchForPath('d.txt', 'staged') + + assert.equal(wrapper.state('filePath'), 'd.txt') + assert.equal(wrapper.state('filePatch').getPath(), 'd.txt') + assert.equal(wrapper.state('stagingStatus'), 'staged') + + wrapper.find('PaneItem').prop('onDidCloseItem')() + assert.isNull(wrapper.state('filePath')) + assert.isNull(wrapper.state('filePatch')) + assert.isNull(wrapper.state('stagingStatus')) + + const activate = sinon.stub() + wrapper.instance().filePatchControllerPane = {activate} + await wrapper.instance().showFilePatchForPath('d.txt', 'staged', {activate: true}) + assert.equal(activate.callCount, 1) + }) + }) + + describe('when there is a change to the repo', () => { + it('calls onRepoRefresh', async () => { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + + fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change', 'utf8') + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + sinon.spy(wrapper.instance(), 'onRepoRefresh') + await repository.refresh() + assert(wrapper.instance().onRepoRefresh.called) + }) + }) + + describe('#onRepoRefresh', () => { + it('sets the correct FilePatch as state', async () => { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + + fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change', 'utf8') + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + await wrapper.instance().showFilePatchForPath('file.txt', 'unstaged', {activate: true}) + + const originalFilePatch = wrapper.state('filePatch') + assert.equal(wrapper.state('filePath'), 'file.txt') + assert.equal(wrapper.state('filePatch').getPath(), 'file.txt') + assert.equal(wrapper.state('stagingStatus'), 'unstaged') + + fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change\nand again!', 'utf8') + await repository.refresh() + await wrapper.instance().onRepoRefresh() + + assert.equal(wrapper.state('filePath'), 'file.txt') + assert.equal(wrapper.state('filePatch').getPath(), 'file.txt') + assert.equal(wrapper.state('stagingStatus'), 'unstaged') + assert.notEqual(originalFilePatch, wrapper.state('filePatch')) + }) + }) + }) + + describe('when amend mode is toggled in the staging panel while viewing a staged change', () => { + it('refetches the FilePatch with the amending flag toggled', async () => { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + fs.writeFileSync(path.join(workdirPath, 'file.txt'), 'change', 'utf8') + await wrapper.instance().showFilePatchForPath('file.txt', 'unstaged', {amending: false}) + const originalFilePatch = wrapper.state('filePatch') + assert.isOk(originalFilePatch) + + sinon.spy(wrapper.instance(), 'showFilePatchForPath') + await wrapper.instance().didChangeAmending(true) + assert.isTrue(wrapper.instance().showFilePatchForPath.args[0][2].amending) + }) + }) + + describe('when the StatusBarTileController calls toggleGitPanel', () => { + it('toggles the git panel', async () => { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + assert.isFalse(wrapper.find('Panel').prop('visible')) + wrapper.find('StatusBarTileController').prop('toggleGitPanel')() + assert.isTrue(wrapper.find('Panel').prop('visible')) + wrapper.find('StatusBarTileController').prop('toggleGitPanel')() + assert.isFalse(wrapper.find('Panel').prop('visible')) + }) + }) + + describe('toggleGitPanel()', () => { + it('toggles the visibility of the Git panel', async () => { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + assert.isFalse(wrapper.find('Panel').prop('visible')) + wrapper.instance().toggleGitPanel() + assert.isTrue(wrapper.find('Panel').prop('visible')) + wrapper.instance().toggleGitPanel() + assert.isFalse(wrapper.find('Panel').prop('visible')) + }) + }) + + describe('openAndFocusGitPanel()', () => { + it('shows-and-focuses the git panel', async () => { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + + app = React.cloneElement(app, {repository}) + let wrapper = shallow(app) + + sinon.spy(wrapper.instance(), 'focusGitPanel') + assert.isFalse(wrapper.find('Panel').prop('visible')) + wrapper.instance().openAndFocusGitPanel() + assert.isTrue(wrapper.find('Panel').prop('visible')) + // TODO: remove this once we figure out the odd behavior that requires + // a setTimeout in openAndFocusGitPanel's setState callbasck + await new Promise(res => setTimeout(res, 250)) + assert.equal(wrapper.instance().focusGitPanel.callCount, 1) + }) + }) +}) diff --git a/test/views/staging-view.test.js b/test/views/staging-view.test.js index 66c0bc6544..53de5bfd91 100644 --- a/test/views/staging-view.test.js +++ b/test/views/staging-view.test.js @@ -93,7 +93,6 @@ describe('StagingView', () => { assert.equal(didSelectFilePath.callCount, 0) view.focus() - assert.isTrue(didSelectFilePath.calledWith(filePatches[0].filePath)) await view.selectNext() assert.isTrue(didSelectFilePath.calledWith(filePatches[1].filePath)) await view.selectNext() From 7435dacd4931c4b7b759baa2da8c71fb4d59c644 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 17:19:35 -0800 Subject: [PATCH 44/47] Destroy ModelObserver when GitController unmounts --- lib/controllers/git-controller.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 87046f26a3..061b255f38 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -2,7 +2,7 @@ import path from 'path' -import {CompositeDisposable, File} from 'atom' +import {CompositeDisposable, Disposable, File} from 'atom' import React from 'react' import ReactDom from 'react-dom' @@ -70,6 +70,9 @@ export default class GitController extends React.Component { didUpdate: () => this.onRepoRefresh() }) this.repositoryObserver.setActiveModel(props.repository) + this.subscriptions.add( + new Disposable(() => this.repositoryObserver.destroy()) + ) } componentWillReceiveProps(newProps) { From f6d43dcdd39485a5caa8f74cdaf000852c1b587a Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 17:31:44 -0800 Subject: [PATCH 45/47] :fire: unused focusFilePatchView --- lib/github-package.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/github-package.js b/lib/github-package.js index 486a11df2a..16b7805da3 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -103,12 +103,6 @@ export default class GithubPackage { } } - focusFilePatchView () { - // TODO: migrate to ref - this.filePatchController.refs.filePatchView.element.focus() - } - - getActiveRepository () { return this.activeRepository } From 1e8d32b7db64c29a01f946f84a3cb47685aaa25a Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 5 Dec 2016 18:07:57 -0800 Subject: [PATCH 46/47] Add EtchWrapper test --- test/views/etch-wrapper.test.js | 58 +++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 test/views/etch-wrapper.test.js diff --git a/test/views/etch-wrapper.test.js b/test/views/etch-wrapper.test.js new file mode 100644 index 0000000000..3fba23409f --- /dev/null +++ b/test/views/etch-wrapper.test.js @@ -0,0 +1,58 @@ +/** @babel */ + +import React from 'react' +import ReactDom from 'react-dom' +import sinon from 'sinon' +import {mount} from 'enzyme' +import etch from 'etch' + +import EtchWrapper from '../../lib/views/etch-wrapper' + + +class EtchComponent { + constructor (props) { + this.updated = 0 + this.destroyed = 0 + this.props = props + etch.initialize(this) + } + + update (props) { + this.updated++ + this.props = props + return etch.update(this) + } + + destroy () { + this.destroyed++ + etch.destroy(this) + } + + render () { + return etch.dom('div', null, this.props.text) + } +} + +describe('EtchWrapper', () => { + it('constructs a wrapped etch component with the given props', async () => { + let app = ( + + + + ) + + const wrapper = mount(app) + const instance = wrapper.instance().getWrappedComponent() + assert.equal(wrapper.text(), 'hello') + assert.equal(instance.props.other, 'world') + + wrapper.setProps({children: }) + await etch.getScheduler().getNextUpdatePromise() + assert.equal(wrapper.text(), 'world') + assert.equal(instance.props.other, 'more') + assert.equal(instance.updated, 1) + + wrapper.unmount() + assert.equal(instance.destroyed, 1) + }) +}) From 496638a818fb34e69e3183416662b7023430b9c7 Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Tue, 6 Dec 2016 11:42:46 -0800 Subject: [PATCH 47/47] =?UTF-8?q?=C2=AF\(=C2=BA=5Fo)/=C2=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/controllers/git-controller.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 061b255f38..d920f5eeb8 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -199,11 +199,7 @@ export default class GitController extends React.Component { openAndFocusGitPanel () { if (!this.state.gitPanelActive) { - this.setState({gitPanelActive: true}, () => { - // TODO: why doesn't a setTimeout of 0 or even 100 work here? - // Suspect it has to do with the command palette closing... - setTimeout(() => this.focusGitPanel(), 200) - }) + this.setState({gitPanelActive: true}, () => this.focusGitPanel()) } else { this.focusGitPanel() }