Mobile: Add quick actions #2247
Conversation
ReactNativeClient/android/app/src/main/java/net/cozic/joplin/MainApplication.java
Outdated
Show resolved
Hide resolved
|
This is ready for review and Android testing :) |
|
I will be happy to test it if there is an apk for it. |
devonzuegel
force-pushed
the
mobile-quick-actions
branch
from
52fe4ac
to
2024fb1
Compare
|
|
||
| import {DeviceEventEmitter} from 'react-native'; | ||
| import QuickActions from 'react-native-quick-actions'; | ||
| const { _ } = require('lib/locale.js'); | ||
|
|
||
| export default (dispatch, folderId) => { | ||
| QuickActions.setShortcutItems([ | ||
| {type: 'New note', title: _('New note'), icon: 'Compose'}, | ||
| {type: 'New to-do', title: _('New to-do'), icon: 'Add'}, | ||
| ]); | ||
|
|
||
| DeviceEventEmitter.addListener('quickActionShortcut', data => { | ||
| // This dispatch is to momentarily go back to reset state, similar to what | ||
| // happens in onJoplinLinkClick_(). Easier to just go back, then go to the | ||
| // note since the Note screen doesn't handle reloading a different note. | ||
| // | ||
| // This hack is necessary because otherwise you get this problem: | ||
| // The first time you create a note from the quick-action menu, it works | ||
| // perfectly. But if you do it again immediately later, it re-opens the | ||
| // page to that first note you made rather than creating an entirely new | ||
| // note. If you navigate around enough (which I think changes the redux | ||
| // state sufficiently or something), then it'll work again. | ||
| dispatch({type: 'NAV_BACK'}); | ||
|
|
There was a problem hiding this comment.
Please could you move this file outside of "lib"?
Also would you be ok with converting it to TypeScript? I think it should be relatively easy if you follow these steps:
- Rename the file to QuickAction.ts
- Add the generated "QuickAction.js" to .gitignore and .eslintignore
- Run
npm run tscfrom the root, which might display a few errors related to types. I don't know if you are familiar with TypeScript, but most likely you'll need to set the type of the function parameters. Sodispatch:Function,folderId:string,data:any. Then try againnpm run tsc.
Other than that, I think the file should pass the TypeScript compilation, if not please let me know.
There was a problem hiding this comment.
Please could you move this file outside of "lib"?
Sure thing! I just moved it up one level, under ReactNativeClient/.
Also would you be ok with converting it to TypeScript?
Yep, just pushed that change and after some fiddling it seems to compile just fine. Excited to see the project transitioning towards Typescript!
| {type: 'New note', title: _('New note'), icon: 'Compose'}, | ||
| {type: 'New to-do', title: _('New to-do'), icon: 'Add'}, |
There was a problem hiding this comment.
Here it should be:
{type: 'New note', title: _('New note'), icon: 'Compose', userInfo: { url: "" }},
{type: 'New to-do', title: _('New to-do'), icon: 'Add', userInfo: { url: "" }},
Due to this bug: jordanbyron/react-native-quick-actions#90
Also please add a command and link to that issue so that if we add more actions we don't forget to add the empty userinfo key.
There was a problem hiding this comment.
Sure thing, I added a comment with a link to the issue. I wasn't quite sure what you meant by "command" (maybe typo of "comment"?), so lmk if I missed something.
|
I've tested on Android and it works great 👍 The only thing is that you need to add the
|
|
Sorry I've been really busy at work so I haven't had time to look at the feedback yet! I'll do this as soon as I have a moment free. |
…/joplin into mobile-quick-actions
|
All righty, thanks for your patience! I believe I've resolved all outstanding feedback. Cheers. |
|
Thanks for the update. There are 2 @laurent22 we also have to add a tsc target to the package.json file and add it to the CI pipeline. Currently a |
Thanks, fixed
Anything I can do here, or does the Android version need to be buildable on my machine for me to be helpful? |
|
Thanks for the update.
I don't think so. I believe @laurent22 has to add something to the build process. But there are a few conflicts, which have to be resolved, before we can merge. |
|
Sorry I missed the comments from 2 weeks ago. I think it's good to merge once the conflicts are fixed. For pluginAssets/index you can get the latest from master by the way. |
|
Hmm no in fact you can delete pluginAssets/index as it's no longer in the tree. |
|
@devonzuegel ok, so the only thing that is missing is fixing the conflicts. then we can merge. |
|
Had some trouble trying to resolve merge conflicts today. The ios app now won't build, so I must've done something wrong in resolving the conflicts. Sorry this PR has been dragging on so long, I'll try to get it over the line soon. |
|
@devonzuegel yep, don't worry about it, things like that can happen. I think that one thing stands out. We removed the share extension, since we upgraded to reaact-native 0.61. But due to the conflict resolving it's there again. Maybe this is the reason for the compile errors. |
|
The build doesn't work because RN has been upgraded recently and the iOS app in particular is built differently (it now auto-links native packages, and uses Podfiles, which is much better, but also a bit of troubles during the transition). It's going to be hard to fix so the simplest is to merge the pull request, then I'm going to re-link the packages afterwards. |
|
Okay cool! As long as it's building for you then I'm happy. I'm just afraid I may have broken more things by resolving the merge conflicts... Lmk if I can help at all, as I'd hate to create more work for you if I did screw up. |
|
No problem, I've merged it the other day and fixed the build afterwards, so it will be in the next Android and iOS releases. |
This adds the ability to quick-add notes and to-dos with force touch directly from your homescreen. Demo:
To build this, you may need to follow the directions in here to link the native QuickActions library:
https://facebook.github.io/react-native/docs/linking-libraries-ios.html#content
Next steps to complete this PR:
Null notebook bug:
Notes get added without being associated properly to a notebook. For illustration, here you can see that they show up inAll notesbut not in the single notebook (i.e. the only option):👉 UPDATE: This only happens if you have no notebooks (which almost only happens in a test environment). I'm going to consider that fine, since they still show up in
All notes. @laurent22 lmk if you disagree though! We could consider (a) not showing the quick-actions when there's no notebook to add a note/to-do to, (b) forcing the user to create a notebook before completing the quick-action (I don't love this one), or (c) something else entirely.New note screen bug:
The first time you create a note from the quick-action menu, it works perfectly. But if you do it again immediately later, it re-opens the page to that first note you made rather than creating an entirely new note. If you navigate around enough (which I think changes the redux state sufficiently or something), then it'll work again.Interestingly, there's no conflict between new notes and new to-dos. In other words, if you create a new note via quick-action and then a new to-do immediately after, you don't see this problem. But you do see it if you do two successive notes or two successive to-dos.👉 UPDATE: I believe I fixed this bug by adding this line, but I'm not quite sure why the fix worked a and it's hacky, so I'd love someone's eyes on it to (a) make sure it's actually a fix and (b) potentially suggest something less hacky.
Android testing: I still haven't been able to successfully set up Android on my machine, so I'd love for someone to test this on theirs and report back before we merge!