From 8b6c3165d91ae176b92e7ba09eeefa0d4fe573b4 Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Thu, 28 Sep 2023 12:26:08 -0400 Subject: [PATCH 1/4] fix: handle undetermined state of appstatus checks In AppStatusModal, we listen for the value of overallStatus and if it is false, we trigger the permissions popup. But there is an issue where the AppStatusModal sometimes shows up even if all the checks are green. This is because it can take some time for the checks to initialize and return back their value. During this time, we want to have a sort of 'undetermined' state for overallStatus, so that we don't show the popup prematurely. We'll do this by allowing overallStatus to be either true/false OR undefined. If there are no checks yet, overallStatus will be undefined and we won't show the popup yet. Once the checks are initialized, we'll be able to determine the status and it will return true or false. For the individual checks themselves, they also need to be able to report an 'undetermined' state. If their statusStates are false by default, and are false before they have actually been determined, then overallStatus will also be false while the check is still ongoing. So we should remove the initial 'false' condition from the few checks that had it, allowing the initial to be 'undefined', and when we iterate through the checks, we will explicitly check if `statusState === false` instead of just checking if `!statusState`. Result of this is that AppStatusModal only shows if any permissions are broken. Tested by loading the app, with all of the correct permissions, 20 times both before and after the change. Before the change, 2 of the attempts resulted in the AppStatusModal incorrectly appearing. After the change, the AppStatusModal did not appear on any of the 20 attempts. After this, I disallowed the Location permission and the modal did still appear as expected. --- www/js/control/AppStatusModal.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/www/js/control/AppStatusModal.tsx b/www/js/control/AppStatusModal.tsx index ff5d18ca1..5cb2d3e00 100644 --- a/www/js/control/AppStatusModal.tsx +++ b/www/js/control/AppStatusModal.tsx @@ -31,10 +31,12 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { let iconMap = (statusState) => statusState ? "check-circle-outline" : "alpha-x-circle-outline"; let colorMap = (statusState) => statusState ? colors.success : colors.danger; - const overallStatus = useMemo(() => { + const overallStatus = useMemo(() => { let status = true; + if (!checkList?.length) return undefined; // if checks not loaded yet, status is undetermined checkList.forEach((lc) => { - if(!lc.statusState){ + console.debug('check in permission status for ' + lc.name + ':', lc.statusState); + if (lc.statusState === false) { status = false; } }) @@ -114,14 +116,12 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { let locSettingsCheck = { name: t("intro.appstatus.locsettings.name"), desc: t(androidSettingsDescTag), - statusState: false, fix: fixSettings, refresh: checkSettings } let locPermissionsCheck = { name: t("intro.appstatus.locperms.name"), desc: t(androidPermDescTag), - statusState: false, fix: fixPerms, refresh: checkPerms } @@ -161,14 +161,12 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { const locSettingsCheck = { name: t("intro.appstatus.locsettings.name"), desc: t(iOSSettingsDescTag), - statusState: false, fix: fixSettings, refresh: checkSettings }; const locPermissionsCheck = { name: t("intro.appstatus.locperms.name"), desc: t(iOSPermDescTag), - statusState: false, fix: fixPerms, refresh: checkPerms }; @@ -443,4 +441,4 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { ) } -export default AppStatusModal; \ No newline at end of file +export default AppStatusModal; From 8e061a0ca954dfe6b2c9180fe2c789d6d0b6a11c Mon Sep 17 00:00:00 2001 From: Abby Wheelis Date: Thu, 28 Sep 2023 12:17:24 -0600 Subject: [PATCH 2/4] fix blank checkList problem on some occasions, the checkList was failing to be initialized. Upon investigation, this turned out to be because platform and osver were "" and 0, respectively. The underlying issue is that they were held in useState, which is updated asyncronously. useRef is more appropriate here, as the platform and osver only need to be set once, and are not anticipated to change. The access changed, as useRef variables are read and set from "var.current" Before these changes, roughly 1/3 of the time, the platform and osver were not set yet, and so the checks were not set up as the platform was interpreted as not valid. After the change, the platform and osver are always set properly in time to call createChecklist. I also updated the useEffect that calls the createChecklist to do so if checkList does not exist or has a length of 0, rather than the else that I had before. --- www/js/control/AppStatusModal.tsx | 39 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/www/js/control/AppStatusModal.tsx b/www/js/control/AppStatusModal.tsx index 5cb2d3e00..34f5820bf 100644 --- a/www/js/control/AppStatusModal.tsx +++ b/www/js/control/AppStatusModal.tsx @@ -1,5 +1,5 @@ //component to view and manage permission settings -import React, { useState, useEffect, useMemo } from "react"; +import React, { useState, useEffect, useMemo, useRef } from "react"; import { Modal, useWindowDimensions, ScrollView } from "react-native"; import { Dialog, Button, Text, useTheme } from 'react-native-paper'; import { useTranslation } from "react-i18next"; @@ -16,8 +16,8 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { const { appConfig, loading } = useAppConfig(); const { height: windowHeight } = useWindowDimensions(); - const [osver, setOsver] = useState(0); - const [platform, setPlatform] = useState(""); + const osver = useRef(0); + const platform = useRef(""); const [error, setError] = useState(""); const [errorVis, setErrorVis] = useState(false); @@ -98,17 +98,17 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { return checkOrFix(locPermissionsCheck, window['cordova'].plugins.BEMDataCollection.isValidLocationPermissions, false); }; var androidSettingsDescTag = "intro.appstatus.locsettings.description.android-gte-9"; - if (osver < 9) { + if (osver.current < 9) { androidSettingsDescTag = "intro.appstatus.locsettings.description.android-lt-9"; } var androidPermDescTag = "intro.appstatus.locperms.description.android-gte-12"; - if(osver < 6) { + if(osver.current < 6) { androidPermDescTag = 'intro.appstatus.locperms.description.android-lt-6'; - } else if (osver < 10) { + } else if (osver.current < 10) { androidPermDescTag = "intro.appstatus.locperms.description.android-6-9"; - } else if (osver < 11) { + } else if (osver.current < 11) { androidPermDescTag= "intro.appstatus.locperms.description.android-10"; - } else if (osver < 12) { + } else if (osver.current < 12) { androidPermDescTag= "intro.appstatus.locperms.description.android-11"; } console.log("description tags are "+androidSettingsDescTag+" "+androidPermDescTag); @@ -153,7 +153,7 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { }; var iOSSettingsDescTag = "intro.appstatus.locsettings.description.ios"; var iOSPermDescTag = "intro.appstatus.locperms.description.ios-gte-13"; - if(osver < 13) { + if(osver.current < 13) { iOSPermDescTag = 'intro.appstatus.locperms.description.ios-lt-13'; } console.log("description tags are "+iOSSettingsDescTag+" "+iOSPermDescTag); @@ -176,7 +176,7 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { } function setupAndroidFitnessChecks() { - if(osver >= 10){ + if(osver.current >= 10){ let fixPerms = function() { console.log("fix and refresh fitness permissions"); return checkOrFix(fitnessPermissionsCheck, window['cordova'].plugins.BEMDataCollection.fixFitnessPermissions, @@ -267,10 +267,10 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { false); }; var androidUnusedDescTag = "intro.appstatus.unusedapprestrict.description.android-disable-gte-13"; - if (osver == 12) { + if (osver.current == 12) { androidUnusedDescTag= "intro.appstatus.unusedapprestrict.description.android-disable-12"; } - else if (osver < 12) { + else if (osver.current < 12) { androidUnusedDescTag= "intro.appstatus.unusedapprestrict.description.android-disable-lt-12"; } let unusedAppsUnrestrictedCheck = { @@ -295,9 +295,9 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { let overallFitnessName = t('intro.appstatus.overall-fitness-name-android'); let locExplanation = t('intro.appstatus.overall-loc-description'); - if(platform == "ios") { + if(platform.current == "ios") { overallFitnessName = t('intro.appstatus.overall-fitness-name-ios'); - if(osver < 13) { + if(osver.current < 13) { locExplanation = (t("intro.permissions.locationPermExplanation-ios-lt-13")); } else { locExplanation = (t("intro.permissions.locationPermExplanation-ios-gte-13")); @@ -316,12 +316,13 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { } function createChecklist(){ - if(platform == "android") { + console.debug("setting up checks, platform is " + platform.current + "and osver is " + osver.current); + if(platform.current == "android") { setupAndroidLocChecks(); setupAndroidFitnessChecks(); setupAndroidNotificationChecks(); setupAndroidBackgroundRestrictionChecks(); - } else if (platform == "ios") { + } else if (platform.current == "ios") { setupIOSLocChecks(); setupIOSFitnessChecks(); setupAndroidNotificationChecks(); @@ -369,8 +370,8 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { //load when ready useEffect(() => { if (appConfig && window['device']?.platform) { - setPlatform(window['device'].platform.toLowerCase()); - setOsver(window['device'].version.split(".")[0]); + platform.current = window['device'].platform.toLowerCase(); + osver.current = window['device'].version.split(".")[0]; if(!haveSetText) { @@ -378,7 +379,7 @@ const AppStatusModal = ({permitVis, setPermitVis}) => { setupPermissionText(); setHaveSetText(true); } - else{ + if(!checkList || checkList.length == 0) { console.log("setting up permissions"); createChecklist(); } From faaeda60bf93190e0998fecab6c529da9e3e412e Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Thu, 28 Sep 2023 14:54:13 -0400 Subject: [PATCH 3/4] export modeColors so it can be used in tests Something I noticed while adding new tests - modeColors is used in diaryHelper.test.ts, but it isn't exported from diaryHelper.ts. I think this was due to a bad merge conflict. Simply exporting it fixes the tests, so I'm just going to bundle in that fix here. --- www/js/diary/diaryHelper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/www/js/diary/diaryHelper.ts b/www/js/diary/diaryHelper.ts index c836d9db2..0b834a485 100644 --- a/www/js/diary/diaryHelper.ts +++ b/www/js/diary/diaryHelper.ts @@ -5,7 +5,7 @@ import moment from "moment"; import { DateTime } from "luxon"; import { LabelOptions, readableLabelToKey } from "../survey/multilabel/confirmHelper"; -const modeColors = { +export const modeColors = { pink: '#c32e85', // oklch(56% 0.2 350) // e-car red: '#c21725', // oklch(52% 0.2 25) // car orange: '#bf5900', // oklch(58% 0.16 50) // air, hsr From e5b2834df09bf01cea814525e3f09a872d0e1fc5 Mon Sep 17 00:00:00 2001 From: Abby Wheelis Date: Thu, 28 Sep 2023 13:28:46 -0600 Subject: [PATCH 4/4] fix toggle issue - remove cch I forgot to remove cch from one place in this file when I translated it from it's former angular version, which was causing the problem with android devices and the medium accuracy toggle While here, resolve the syntax highlighting related to window.cordova by replacing it with window['cordova'], not fixing this syntax in the first place likely contributed to me overlooking this error. --- www/js/control/ControlCollectionHelper.tsx | 31 +++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/www/js/control/ControlCollectionHelper.tsx b/www/js/control/ControlCollectionHelper.tsx index b0855a733..99318b1ac 100644 --- a/www/js/control/ControlCollectionHelper.tsx +++ b/www/js/control/ControlCollectionHelper.tsx @@ -46,9 +46,10 @@ export async function isMediumAccuracy() { return undefined; // config not loaded when loading ui, set default as false } else { var v = await accuracy2String(config); - if (window.cordova.platformId == 'ios') { + console.log("window platform is", window['cordova'].platformId); + if (window['cordova'].platformId == 'ios') { return v != "kCLLocationAccuracyBestForNavigation" && v != "kCLLocationAccuracyBest" && v != "kCLLocationAccuracyTenMeters"; - } else if (window.cordova.platformId == 'android') { + } else if (window['cordova'].platformId == 'android') { return v != "PRIORITY_HIGH_ACCURACY"; } else { window.alert("Emission does not support this platform"); @@ -62,15 +63,15 @@ export async function helperToggleLowAccuracy() { let accuracyOptions = await getAccuracyOptions(); let medium = await isMediumAccuracy(); if (medium) { - if (window.cordova.platformId == 'ios') { + if (window['cordova'].platformId == 'ios') { tempConfig.accuracy = accuracyOptions["kCLLocationAccuracyBest"]; - } else if (window.cordova.platformId == 'android') { - tempConfig.accuracy = cch.accuracyOptions["PRIORITY_HIGH_ACCURACY"]; + } else if (window['cordova'].platformId == 'android') { + tempConfig.accuracy = accuracyOptions["PRIORITY_HIGH_ACCURACY"]; } } else { - if (window.cordova.platformId == 'ios') { + if (window['cordova'].platformId == 'ios') { tempConfig.accuracy = accuracyOptions["kCLLocationAccuracyHundredMeters"]; - } else if (window.cordova.platformId == 'android') { + } else if (window['cordova'].platformId == 'android') { tempConfig.accuracy = accuracyOptions["PRIORITY_BALANCED_POWER_ACCURACY"]; } } @@ -87,7 +88,7 @@ export async function helperToggleLowAccuracy() { */ export const getState = function() { - return window.cordova.plugins.BEMDataCollection.getState(); + return window['cordova'].plugins.BEMDataCollection.getState(); }; export async function getHelperCollectionSettings() { @@ -101,18 +102,18 @@ export async function getHelperCollectionSettings() { } const setConfig = function(config) { - return window.cordova.plugins.BEMDataCollection.setConfig(config); + return window['cordova'].plugins.BEMDataCollection.setConfig(config); }; const getConfig = function() { - return window.cordova.plugins.BEMDataCollection.getConfig(); + return window['cordova'].plugins.BEMDataCollection.getConfig(); }; const getAccuracyOptions = function() { - return window.cordova.plugins.BEMDataCollection.getAccuracyOptions(); + return window['cordova'].plugins.BEMDataCollection.getAccuracyOptions(); }; export const forceTransitionWrapper = function(transition) { - return window.cordova.plugins.BEMDataCollection.forceTransition(transition); + return window['cordova'].plugins.BEMDataCollection.forceTransition(transition); }; const formatConfigForDisplay = function(config, accuracyOptions) { @@ -197,7 +198,7 @@ const ControlSyncHelper = ({ editVis, setEditVis }) => { /*ios vs android*/ let filterComponent; - if(window.cordova.platformId == 'ios') { + if(window['cordova'].platformId == 'ios') { filterComponent = Filter Distance onChangeText(text, "filter_distance")}/> @@ -209,7 +210,7 @@ const ControlSyncHelper = ({ editVis, setEditVis }) => { } let iosToggles; - if(window.cordova.platformId == 'ios') { + if(window['cordova'].platformId == 'ios') { iosToggles = <> {/* use visit notifications toggle NO ANDROID */} @@ -224,7 +225,7 @@ const ControlSyncHelper = ({ editVis, setEditVis }) => { } let geofenceComponent; - if(window.cordova.platformId == 'android') { + if(window['cordova'].platformId == 'android') { geofenceComponent = Geofence Responsiveness onChangeText(text, "android_geofence_responsiveness")}/>