Alert user to new version in app store #369
Conversation
|
@clmntnbrn @stevecottle would be good to get some UXD input into how we might surface this to users without annoying them :) |
|
@markholland When you next have the chance it would be great to get your opinion on this. Want to make sure that it fits with how we are managing version numbers. |
|
This is fine as it uses the public version numbers. We'd only have to worry if it was using the Android versionCode or iOS Build Number. |
This reverts commit 453c36e.
|
I've added "Ask me later" functionality and productionised the code. In it's current state this could go out to production if we really want it and there's always the option to make it prettier later. If a user presses "Ask me later" the app will wait until the start of the next day to notify the user when they next open the app.
|
robbiemccorkell
changed the title
|
Could definitely do with a review of the copy in the dialog too |
| } | ||
|
|
||
| handleAppStoreAlert = () => | ||
| Alert.alert( |
There was a problem hiding this comment.
Will this work if the app was send to the backround while the update check was running?
There was a problem hiding this comment.
I did a quick test by throwing a delay into the code. It looks like when the app is in the background, nothing happens on the system level, but when you open the app again the dialog is there.
| import * as storage from "../integrations/storage"; | ||
| import * as date from "../lib/date"; | ||
|
|
||
| const nextTick = () => new Promise(resolve => setTimeout(resolve, 0)); |
There was a problem hiding this comment.
Do you think we can use jest.runAllTicks() instead?
There was a problem hiding this comment.
I didn't know about this, but I tried it out and it gave me a weird warning and didn't work.
I've simplified it a little to use process.nextTick though, since I only just found out about this too.
src/actions/reporting.js
Outdated
| export const notifyReporter = (error: Error) => ( | ||
| _: Dispatch<>, | ||
| __: () => State, | ||
| errorReporter: Client |
There was a problem hiding this comment.
I wonder if it would be better to create a top level action, like our INIT action that is just called ERROR with an error property to hold the error data. Then to notify bugsnag, we create a bugsnag middleware that listens for this action. This would mean that we don't need to instantiate the thunk middleware with extra arguments and complicate our action creators.
There was a problem hiding this comment.
Sure thing, makes sense. Sorry it took me so long to get back to this, I'll fix this up on Saturday
|
Is it possible to only show this for minor updates rather than patches? Because of our current release process, if there's a bug on one platform we still have to release a patch update for both. |
|
@markholland Yea I think you can specify a depth to the library. That's a good point, it might be a bit too chatty on on patch versions. I'll have a go at limiting to major and minor only. |
|
This looks fine, I have a couple of questions about the details: How often will this be triggered? Could the copy read: Ref: I can't find an iOS version of this doc^ for guidance of button order. Has anyone copme across anything similar? |
|
@stevecottle We should be able to change the CTA text- and I agree it should be clearer than just "OK" - "Update now" sounds good to me. That CTA would take you to the app store or google play, where you can update the app. "Ask me tomorrow" could be a weird one, as from what I understand, you can customise when you want to get reminders on Android. Ask me later is the generic and most common way most apps go about it. Rather than having to build something different for Android- if that's the case - we should keep both the same and keep it consistent with other apps out there. So.. @robbiemccorkell sounds like this one's good to go, with the change from OK to Update now, if possible :) |
|
I can have a look at changing the text today and limit to major and minor versions only. |
|
@stevecottle: It will be triggered on every app start, but not when the app is already running and just brought to the foreground. @serdemli Android won't be an issue. Android restricts usage of exact timers and users can choose to disable/deley notifications. But we use neither of those in this PR. |
|
We discussed in stand-up, that this might not be needed anymore since won't get the notification for the parade update anyway. So I'll let @robbiemccorkell finish this 🙂 |
This PR uses the package
react-native-version-checkto check if there is a newer version of the app in a respective app store and alerts the user.The package seems to compare the latest app store version to that found in
Info.plistin IOS, and theversionNamedefined in Android. I'm not very familiar with how this works but I assume we set these numbers via fastlane at build time.To manually test this on each platform we had to:
IOS:
Info.plistto something lower than that listed in the app store.Android:
run-androidcommand inpackage.jsontoorg.prideinlondon.festivalinstead oforg.prideinlondon.festival.devto match that app store app Id.android/app/build.gradlechangeproductFlavors.dev.applicationIdtoorg.prideinlondon.festival.I'm sure there's a better way of doing this, but those are the steps we took.
If everyone thinks this is feasible we can run it past UX to see what the best way of alerting the user is, and to consider a "Remind me later" feature.
Pre-flight check-list
Before raising a pull request
Pre-merge check-list
Tester check-list
Tested on multiple devices / OS versions (Test on the physical device(s) you have access to, otherwise use BrowserStack):
Optional:
Accessibility Testing (If applicable)
Weak connection testing (If applicable)