Skip to content

Save Sketches on mobile #1544

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
merged 31 commits into from
Aug 28, 2020
Merged

Save Sketches on mobile #1544

merged 31 commits into from
Aug 28, 2020

Conversation

ghalestrilo
Copy link
Collaborator

@ghalestrilo ghalestrilo commented Aug 12, 2020 •
edited
Loading

Requires #1528

Part of the Mobile UI project. This branch implements the ability to save sketches on the Mobile IDE, and to persist project state across mobile views. Roadmap:

  • Connect the <Editor /> component and refactor IDE Views
  • Persist sketch state
  • Add save and autosave functionality

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named and links to an issue number, i.e. Fixes #123

@ghalestrilo ghalestrilo self-assigned this Aug 12, 2020
@ghalestrilo
Merge branch 'feature/mobile-examples' of https://github.com/ghalestr…
dabd5e0 
…ilo/p5.js-web-editor into feature/mobile-save-sketch
@ghalestrilo
💄 start with terinal collapsed
5da8b24
@ghalestrilo
✨ create useEffectWithComparison hook
955d8c2
@ghalestrilo
✨ create autosave method / thunk
99594a3
@ghalestrilo
✨ create useEventListener hook
53a5198
@ghalestrilo
🚧 copy handleGlobalKeydown to MobileIDEView
fac3f96
@ghalestrilo
🚧 enable keyboard shortcuts on mobile
a0b302f
@ghalestrilo
🐛 fix hook not detecting user authenticated
074334e
@ghalestrilo
🔀 merge from mobile-examples
1503b1b
@ghalestrilo
🚧 update saveProject action redirection
6941e06
@ghalestrilo
🚧 pass project and user to autosave
e996d49
@ghalestrilo
✨ add save button to bottom bar
100a6a0
@ghalestrilo
Merge branch 'feature/mobile-save-sketch' of https://github.com/ghale…
c0b9ae4 
…strilo/p5.js-web-editor into feature/mobile-save-sketch
@ghalestrilo
🔀 pull from feature/mobile-files-tab
54ae17f
@ghalestrilo
Merge branch 'develop' of https://github.com/processing/p5.js-web-editor
379cb94 
 into feature/mobile-save-sketch
@ghalestrilo
💄 save icon
09c8f77
@ghalestrilo
🐛 fix missing files on sidebar
33848e2
@ghalestrilo
Merge branch 'develop' of https://github.com/processing/p5.js-web-editor
5f24a71 
 into feature/mobile-save-sketch
@ghalestrilo ghalestrilo marked this pull request as ready for review August 18, 2020 20:29
@ghalestrilo ghalestrilo requested a review from catarak August 18, 2020 20:29
@ghalestrilo
Copy link
Collaborator Author

This is done! I just updated it with develop, and it's working fine. Note that it does not have yet a way to rename the sketches from the MobileIDEView header.

catarak
catarak reviewed Aug 24, 2020
View reviewed changes
Copy link
Member

@catarak catarak left a comment

Choose a reason for hiding this comment

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

Overall this looks good! I just have one small suggestion, and then I think it makes sense to open another PR with some feedback that the save is happening with the Toast component.

client/images/save.svg
@@ -0,0 +1,3 @@
<svg xmlns="http://www.w3.org/2000/svg" height="24" viewBox="-4 -4 32 32" width="24">
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to add this icon, there's already an icon for this called unsaved-changes-dot.svg.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the little diskette icon that the user presses to save the project. I couldn't find an icon for this. is there one available?

Copy link
Member

Choose a reason for hiding this comment

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

Oh my bad! I got confused and thought this was the unsaved dot icon. All good then :)

catarak
catarak requested changes Aug 28, 2020
View reviewed changes
@ghalestrilo ghalestrilo requested a review from catarak August 28, 2020 18:16
catarak
catarak approved these changes Aug 28, 2020
View reviewed changes
Copy link
Member

@catarak catarak left a comment

Choose a reason for hiding this comment

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

Working great for me!

@ghalestrilo ghalestrilo merged commit 6cb9968 into processing:develop Aug 28, 2020
@catarak catarak mentioned this pull request Sep 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@catarak catarak catarak approved these changes

Assignees

@ghalestrilo ghalestrilo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ghalestrilo @catarak