Skip to content

tidy up initial "external editor" experience #2611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
joshaber merged 12 commits into from
Sep 6, 2017
Merged

tidy up initial "external editor" experience #2611

joshaber merged 12 commits into from
Sep 6, 2017

Conversation

@shiftkey
Copy link
Member

@shiftkey shiftkey commented Sep 5, 2017 •
edited
Loading

Fixes #2551 in a few ways:

  • checks what you have installed when initializing the application state
  • assign first found editor to local storage if found
  • if no matching editors, display "Open in External Editor" for menu items
  • when trying to launch with no editors, user should see existing dialog about recommending Atom
  • fix bug about item switching being required to actually persist

shiftkey
shiftkey commented Sep 5, 2017
View reviewed changes
app/src/ui/preferences/advanced.tsx

const editors = availableEditors.map(editor => editor.editor)
let selectedExternalEditor = this.props.selectedExternalEditor
public async componentWillReceiveProps(nextProps: IAdvancedPreferencesProps) {

This comment was marked as spam.

Sign in to view

@shiftkey shiftkey added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 5, 2017
@joshaber joshaber self-assigned this Sep 6, 2017
joshaber
joshaber approved these changes Sep 6, 2017
View reviewed changes
Copy link
Contributor

@joshaber joshaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable 👍

@joshaber joshaber merged commit a51bd03 into master Sep 6, 2017
@joshaber joshaber deleted the tidy-up-initial-editor-experience branch September 6, 2017 21:20
@shiftkey shiftkey mentioned this pull request Sep 20, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@joshaber joshaber joshaber approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@joshaber joshaber

Labels

ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shiftkey @joshaber