Skip to content

Conversation

@shinytang6
Copy link
Member

@shinytang6 shinytang6 commented Feb 21, 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! Closes #560 , Fixes #567

@shinytang6 shinytang6 mentioned this pull request Feb 21, 2018
3 tasks
@catarak
Copy link
Member

catarak commented Feb 21, 2018

not sure if this is the right way to handle this—just added a thought on #560!

@catarak
Copy link
Member

catarak commented Feb 22, 2018

i found a weird bug, and here are the steps to reproduce:

  1. Open the editor, no logged in (I recommend an incognito window)
  2. Open an existing sketch (one of the examples is easiest)
  3. Try to save it using command + s. It will tell you to login or signup
  4. Login.
  5. Try to save it again using command + s. It will properly fork it but then it will also show a modal asking you to login or signup.

This does work if you are already logged in and navigate to a sketch that you do not own!

@shinytang6
Copy link
Member Author

l think it should be a real bug and doesn't matter with my changes because it can be reproduced in current version:)
Just fixed it in this PR and l will report that issue 😶

@catarak
Copy link
Member

catarak commented Feb 23, 2018

got it, thanks for fixing it!

@catarak catarak merged commit 2876634 into processing:master Feb 23, 2018
@shinytang6
Copy link
Member Author

shinytang6 commented Feb 27, 2018

@catarak It seems there are still some bugs related to that PR.

  1. l shouldn't delete the closeOverlay={this.props.hideErrorModal}
  2. when you log in normally and save the changes through command/ctrl s, it still pops up the ErrorModal. The problem is that the 'keydown' event is triggered twice in that case:)

shinytang6 added a commit to shinytang6/p5.js-web-editor that referenced this pull request Feb 27, 2018
shinytang6 added a commit to shinytang6/p5.js-web-editor that referenced this pull request Feb 27, 2018
@shinytang6 shinytang6 deleted the enhance-keyboard-save branch February 27, 2018 14:53
@catarak catarak mentioned this pull request Feb 27, 2018
3 tasks
catarak pushed a commit that referenced this pull request Mar 2, 2018
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