MES-3574 reload app config when app resumes #757
Conversation
| ), | ||
| ), | ||
| )), | ||
| filter(([action, dateConfigLoaded]) => dateConfigLoaded !== this.dateTimeProvider.now().format('YYYY-MM-DD')), |
| switchMap(([action, dateConfigLoaded]) => { | ||
| console.log('App resumed after being suspended. Config was not loaded today... app will refresh'); | ||
| this.navigationProvider.getNav().setRoot(LOGIN_PAGE); | ||
| return of(new appInfoActions.RestartApp()); |
There was a problem hiding this comment.
Is this action used for anything or is it just for info
There was a problem hiding this comment.
Yeah, it's just for info. Need to return an observable, so figured I'd return a descriptive action. Could easily just return empty of() but the benefit of this is I'll be able to write a test.
There's also an AppSuspended action. It get's dispatched, but nothing acts on it (yet).
buzzub
force-pushed
the
mes-3574-refresh-app-daily
branch
from
6336b86
to
3b87e72
Compare
There was a problem hiding this comment.
Just looked at this to try and broaden my understanding of the app - looks nice!
One thing I would like to see is a consistent approach to naming actions. Looks like the established pattern is VERB_NOUN using present tense for the verb. Some of the new actions introduced go with NOUN_VERB using past tense for the verb.
Good observation. If this refers to
|
Andy tested and found no problems. |
| } | ||
|
|
||
| configureAccessibility = () => { | ||
| window.MobileAccessibility.updateTextZoom(); |
There was a problem hiding this comment.
wondering about the removal of the window && window.MobileAccessibility line (51).
Is this because window is guaranteed to exists and this was an unnecessary check?
There was a problem hiding this comment.
Exactly. And we can assume MobileAccessibility will exist too unless the plugin fails.
There was a problem hiding this comment.
Also, we shouldn't really need to use window.MobileAccessibility as this should work:
constructor(mobileAccessibility: MobileAccessibility) {
this.platform.ready()
.then(() => mobileAccessibility.getTextZoom())
.then(e => this.zoomLevel = e);
}
see more at danielsogl/awesome-cordova-plugins#2138
...but it doesn't. The signature is wrong - it doesn't return a promise, it returns a callback ... but you can't pass it a function. Went down a rabbit hole trying to reference the plugin correctly. In the end, decided to leave the code as is because it works.
There was a problem hiding this comment.
How does this react in the browser , we know the plugin won't be there and that line will error
| loadConfigSuccessEffect$ = this.actions$.pipe( | ||
| ofType(appInfoActions.LOAD_CONFIG_SUCCESS), | ||
| switchMap(() => { | ||
| console.log('Config loaded successfully'); |
There was a problem hiding this comment.
do we need console.logs?
There was a problem hiding this comment.
I think Ammar wanted this. I can't remember exactly why... but it definitely made sense at the time.
Description
When the app starts up and loads the configuration successfully, store the current date in the
appInfopart of the store.Then using the platform 'resume` event, check if the date the app config was loaded is different to today's date, and if so, reload the app by setting the app root as the login page.
Checklist
Screenshots (optional)