Skip to content

Conversation

@shinytang6
Copy link
Member

@shinytang6 shinytang6 commented Feb 27, 2018

Before your pull request is reviewed and merged, make sure you

  • there are no linting errors -- npm run lint
  • code is in uniquely-named feature branch, and has been rebased on top of latest master. If you're asked to make more changes make sure you rebase onto master then too!
  • pull request is descriptively named and links to an issue number, i.e. Fixes #123

Thank you! Fixes #587

@catarak
Copy link
Member

catarak commented Feb 27, 2018

is this related to #561?

@shinytang6
Copy link
Member Author

maybe l should open a new issue? Acually if #561 is not submitted, handleGlobalKeydown is still triggered twice in the following case, but will not cause problem because first trigger will do nothing(with this.isUserOwner() = this.props.user.authenticated = false).

  1. log in
  2. saving a default sketch through ctrl/command s
  3. an ErrorModal pops up
  4. if you refresh the page and try saving again, it will go back to normal

@catarak
Copy link
Member

catarak commented Feb 28, 2018

go ahead and open a new issue! do you think it makes sense to merge this in since it fixes part of the issue, or roll it together with whatever fixes the issue you've outlined above?

@shinytang6
Copy link
Member Author

not very sure of what you mean, do you think its not clean enough or?l think l have listed the problem and pr related to that one O_o

@catarak
Copy link
Member

catarak commented Mar 2, 2018

i'm confused! which issue/PR is that? hard to keep track of all of this stuff 🎉

@catarak
Copy link
Member

catarak commented Mar 2, 2018

but i can merge this in since it amends #561.

@catarak catarak merged commit 43f482a into processing:master Mar 2, 2018
@shinytang6 shinytang6 mentioned this pull request Mar 3, 2018
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants