-
Notifications
You must be signed in to change notification settings - Fork 408
Refactor dialog handling #2209
Refactor dialog handling #2209
Changes from all commits
104d3c4
14c7308
d40987a
e768682
e3ac43a
569a9bc
0a83364
f93070d
7ff2157
a4f48b8
5b993de
43929f6
d353db8
90ceabd
c3c18da
8b5572d
38a5433
2fd134c
0083537
81142cf
1b9e363
4dc6d37
020f3e4
b4b5fcb
8733937
b1b8f7c
98e6b5d
466d4cb
f86aa3b
8050d36
b07cfdc
2b98cb9
b183168
45be3ed
baa3c5e
8296d3f
eddc484
61acc05
4ceb9b2
b4056d3
9446510
3a668f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,9 @@ export default class AtomTextEditor extends React.Component { | |
| didDestroySelection: PropTypes.func, | ||
|
|
||
| hideEmptiness: PropTypes.bool, | ||
| preselect: PropTypes.bool, | ||
| className: PropTypes.string, | ||
| tabIndex: PropTypes.number, | ||
|
|
||
| refModel: RefHolderPropType, | ||
|
|
||
|
|
@@ -49,6 +52,8 @@ export default class AtomTextEditor extends React.Component { | |
| didDestroySelection: () => {}, | ||
|
|
||
| hideEmptiness: false, | ||
| preselect: false, | ||
| tabIndex: 0, | ||
| } | ||
|
|
||
| constructor(props) { | ||
|
|
@@ -77,6 +82,13 @@ export default class AtomTextEditor extends React.Component { | |
|
|
||
| this.refParent.map(element => { | ||
| const editor = new TextEditor(modelProps); | ||
| editor.getElement().tabIndex = this.props.tabIndex; | ||
| if (this.props.className) { | ||
| editor.getElement().classList.add(this.props.className); | ||
| } | ||
| if (this.props.preselect) { | ||
| editor.selectAll(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went back and forth between supporting "all text preselected" and "cursor positioned at the end of the provided buffer." I think I landed on "all text preselected" because it's slightly more flexible (one keypress to decline a suggested starting value!) and because it's slightly easier to code (I don't have to pull anything from the buffer here.) |
||
| } | ||
| element.appendChild(editor.getElement()); | ||
| this.getRefModel().setter(editor); | ||
| this.refElement.setter(editor.getElement()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,14 +22,12 @@ export default class Panel extends React.Component { | |
| children: PropTypes.element.isRequired, | ||
| options: PropTypes.object, | ||
| onDidClosePanel: PropTypes.func, | ||
| visible: PropTypes.bool, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 Unused props from waaaay back in the day. |
||
| itemHolder: RefHolderPropType, | ||
| } | ||
|
|
||
| static defaultProps = { | ||
| options: {}, | ||
| onDidClosePanel: panel => {}, | ||
| visible: true, | ||
| } | ||
|
|
||
| constructor(props) { | ||
|
|
@@ -46,21 +44,6 @@ export default class Panel extends React.Component { | |
| this.setupPanel(); | ||
| } | ||
|
|
||
| shouldComponentUpdate(newProps) { | ||
| return this.props.visible !== newProps.visible; | ||
| } | ||
|
|
||
| componentDidUpdate() { | ||
| if (this.didCloseItem) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('Unexpected update in `Panel`: the contained panel has been destroyed'); | ||
| } | ||
|
|
||
| if (this.panel) { | ||
| this.panel[this.props.visible ? 'show' : 'hide'](); | ||
| } | ||
| } | ||
|
|
||
| render() { | ||
| return ReactDOM.createPortal( | ||
| this.props.children, | ||
|
|
@@ -76,7 +59,7 @@ export default class Panel extends React.Component { | |
| const methodName = `add${location}Panel`; | ||
|
|
||
| const item = createItem(this.domNode, this.props.itemHolder); | ||
| const options = {...this.props.options, visible: this.props.visible, item}; | ||
| const options = {...this.props.options, item}; | ||
| this.panel = this.props.workspace[methodName](options); | ||
| this.subscriptions.add( | ||
| this.panel.onDidDestroy(() => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| /** | ||
| * When triggered, automatically focus the first element ref passed to this object. | ||
| * | ||
| * To unconditionally focus a single element: | ||
| * | ||
| * ``` | ||
| * class SomeComponent extends React.Component { | ||
| * constructor(props) { | ||
| * super(props); | ||
| * this.autofocus = new Autofocus(); | ||
| * } | ||
| * | ||
| * render() { | ||
| * return ( | ||
| * <div className="github-Form"> | ||
| * <input ref={this.autofocus.target} type="text" /> | ||
| * <input type="text" /> | ||
| * </div> | ||
| * ); | ||
| * } | ||
| * | ||
| * componentDidMount() { | ||
| * this.autofocus.trigger(); | ||
| * } | ||
| * } | ||
| * ``` | ||
| * | ||
| * If multiple form elements are present, use `firstTarget` to create the ref instead. The rendered ref you assign the | ||
| * lowest numeric index will be focused on trigger: | ||
| * | ||
| * ``` | ||
| * class SomeComponent extends React.Component { | ||
| * constructor(props) { | ||
| * super(props); | ||
| * this.autofocus = new Autofocus(); | ||
| * } | ||
| * | ||
| * render() { | ||
| * return ( | ||
| * <div className="github-Form"> | ||
| * {this.props.someProp && <input ref={this.autofocus.firstTarget(0)} />} | ||
| * <input ref={this.autofocus.firstTarget(1)} type="text" /> | ||
| * <input type="text" /> | ||
| * </div> | ||
| * ); | ||
| * } | ||
| * | ||
| * componentDidMount() { | ||
| * this.autofocus.trigger(); | ||
| * } | ||
| * } | ||
| * ``` | ||
| * | ||
| */ | ||
| export default class AutoFocus { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if I could generalize this somehow to deal with #1403. It'd likely need to wait on a hooks refactor, though, and be passed as a context... 🤔 |
||
| constructor() { | ||
| this.index = Infinity; | ||
| this.captured = null; | ||
| } | ||
|
|
||
| target = element => this.firstTarget(0)(element); | ||
|
|
||
| firstTarget = index => element => { | ||
| if (index < this.index) { | ||
| this.index = index; | ||
| this.captured = element; | ||
| } | ||
| }; | ||
|
|
||
| trigger() { | ||
| if (this.captured !== null) { | ||
| setTimeout(() => this.captured.focus(), 0); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| import InitDialog from '../views/init-dialog'; | ||
| import CloneDialog from '../views/clone-dialog'; | ||
| import CredentialDialog from '../views/credential-dialog'; | ||
| import OpenIssueishDialog from '../views/open-issueish-dialog'; | ||
| import OpenCommitDialog from '../views/open-commit-dialog'; | ||
|
|
||
| const DIALOG_COMPONENTS = { | ||
| null: NullDialog, | ||
| init: InitDialog, | ||
| clone: CloneDialog, | ||
| credential: CredentialDialog, | ||
| issueish: OpenIssueishDialog, | ||
| commit: OpenCommitDialog, | ||
| }; | ||
|
|
||
| export default class DialogsController extends React.Component { | ||
| static propTypes = { | ||
| // Model | ||
| request: PropTypes.shape({ | ||
| identifier: PropTypes.string.isRequired, | ||
| isProgressing: PropTypes.bool.isRequired, | ||
| }).isRequired, | ||
|
|
||
| // Atom environment | ||
| workspace: PropTypes.object.isRequired, | ||
| commands: PropTypes.object.isRequired, | ||
| config: PropTypes.object.isRequired, | ||
| }; | ||
|
|
||
| state = { | ||
| requestInProgress: null, | ||
| requestError: [null, null], | ||
| } | ||
|
|
||
| render() { | ||
| const DialogComponent = DIALOG_COMPONENTS[this.props.request.identifier]; | ||
| return <DialogComponent {...this.getCommonProps()} />; | ||
| } | ||
|
|
||
| getCommonProps() { | ||
| const {request} = this.props; | ||
| const accept = request.isProgressing | ||
| ? async (...args) => { | ||
| this.setState({requestError: [null, null], requestInProgress: request}); | ||
| try { | ||
| const result = await request.accept(...args); | ||
| this.setState({requestInProgress: null}); | ||
| return result; | ||
| } catch (error) { | ||
| this.setState({requestError: [request, error], requestInProgress: null}); | ||
| return undefined; | ||
| } | ||
| } : (...args) => { | ||
| this.setState({requestError: [null, null]}); | ||
| try { | ||
| return request.accept(...args); | ||
| } catch (error) { | ||
| this.setState({requestError: [request, error]}); | ||
| return undefined; | ||
| } | ||
| }; | ||
| const wrapped = wrapDialogRequest(request, {accept}); | ||
|
|
||
| return { | ||
| config: this.props.config, | ||
| commands: this.props.commands, | ||
| workspace: this.props.workspace, | ||
| inProgress: this.state.requestInProgress === request, | ||
| error: this.state.requestError[0] === request ? this.state.requestError[1] : null, | ||
| request: wrapped, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| function NullDialog() { | ||
| return null; | ||
| } | ||
|
|
||
| class DialogRequest { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class is in here so that I can restrict the scope of "things that need to know the List Of All Possible Dialogs" to only this file. Hopefully that'll make it less error-prone to add new ones in the future. |
||
| constructor(identifier, params = {}) { | ||
| this.identifier = identifier; | ||
| this.params = params; | ||
| this.isProgressing = false; | ||
| this.accept = () => {}; | ||
| this.cancel = () => {}; | ||
| } | ||
|
|
||
| onAccept(cb) { | ||
| this.accept = cb; | ||
| } | ||
|
|
||
| onProgressingAccept(cb) { | ||
| this.isProgressing = true; | ||
| this.onAccept(cb); | ||
| } | ||
|
|
||
| onCancel(cb) { | ||
| this.cancel = cb; | ||
| } | ||
|
|
||
| getParams() { | ||
| return this.params; | ||
| } | ||
| } | ||
|
|
||
| function wrapDialogRequest(original, {accept}) { | ||
| const dup = new DialogRequest(original.identifier, original.params); | ||
| dup.isProgressing = original.isProgressing; | ||
| dup.onAccept(accept); | ||
| dup.onCancel(original.cancel); | ||
| return dup; | ||
| } | ||
|
|
||
| export const dialogRequests = { | ||
| null: { | ||
| identifier: 'null', | ||
| isProgressing: false, | ||
| params: {}, | ||
| accept: () => {}, | ||
| cancel: () => {}, | ||
| }, | ||
|
|
||
| init({dirPath}) { | ||
| return new DialogRequest('init', {dirPath}); | ||
| }, | ||
|
|
||
| clone(opts) { | ||
| return new DialogRequest('clone', { | ||
| sourceURL: '', | ||
| destPath: '', | ||
| ...opts, | ||
| }); | ||
| }, | ||
|
|
||
| credential(opts) { | ||
| return new DialogRequest('credential', { | ||
| includeUsername: false, | ||
| includeRemember: false, | ||
| prompt: 'Please authenticate', | ||
| ...opts, | ||
| }); | ||
| }, | ||
|
|
||
| issueish() { | ||
| return new DialogRequest('issueish'); | ||
| }, | ||
|
|
||
| commit() { | ||
| return new DialogRequest('commit'); | ||
| }, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a renaming I slipped in just because it was annoying me 👀