Launching app with #1032 pops up permission screen in summary #998

shankari · 2023-10-03T06:45:57Z

Steps to reproduce:

native build with 👋 New Onboarding Flow + Switch to React Navigation e-mission-phone#1032 included
launch app (on android)
sign in with a staging token
the summary page (looking much uglier than before since it is all stuck to the top of the screen) shows up
almost immediately, the permission popup is displayed and nothing else can be done

Please see screenshot below showing the permission popup over the summary screen

Popup screen blocking summary	optimization is with the poup now!	After resolving, uglier summary screen with text all scrunched up

@JGreenlee @Abby-Wheelis

shankari · 2023-10-03T06:54:34Z

Also, I am not sure that backwards compat works properly. If I launch an existing installed app that hasn't gone through the onboarding process, I get the following. On uninstall + reinstall on iOS, I see the same behavior as well.

Re-launch over an existing app	Permission popup over settings

I don't think I can push even a staging release with this issue. @JGreenlee @Abby-Wheelis can one of you fix this tomorrow morning? It seems related to the true|false|undefined state showstopper from the previous release (the permission check is not true, so I must popup) but not sure why that fix didn't carry through here.

JGreenlee · 2023-10-03T13:10:53Z

I will attempt to reproduce and fix ASAP

JGreenlee · 2023-10-03T13:57:39Z

The first issue was a regression from the summary page being added. The onboarding flow wasn't originally designed with the summary being its own page, and I forgot to add it to the list of checks for whether the AppStatusModal should be allowed to show or not.
The popup should show after the consent page. To ensure this I was just checking that the route was not welcome or consent. Obviously, this is problematic because the new page was inserted in between these.

While fixing this, I'm going to refactor and have the onboarding state be an ordered enum (a TypeScript feature - nonexistent in plain JS). This way, we can say something like onboardingRoute > Routes.CONSENT to check whether we are past the consent page or not, instead of having to specifically list routes .

e-mission/e-mission-docs#998 Adding the 'summary' page caused a regression in which the permissions popup could appear before the consent page, while the onboarding route is 'summary'. This is because when it was added, I forgot to add it to the conditional statement in App.tsx, line 79. Semantically we want the routes to be named, but also ordered. So a cleaner way to handle these routes is to declare them as enum values instead of strings. Behind the scense, TypeScript enums automatically get assigned an integer value (WELCOME=0, SUMMARY=1, etc). This way, to see if a route is past consent or not, we can use `<` and `>` comparisons instead of specifically listing out and checking against different string values. So, this commit switches over to using `enum` for the onboarding state route and uses `>` in the conditional to ensure that the AppStatusModal can only show if we are past the consent page. Also tidied up all the code comments around the onboarding states and routes.

JGreenlee · 2023-10-03T14:24:10Z

For the ugly-looking summary page, I'm not sure what is desired. In my opinion, there is just not enough content there to warrant a full page - which was part of the reason I suggested combining it with the privacy policy.
Ideally, we'd have some graphics content to show around the text and take up the empty space. It currently looks decent on smaller screens, but large screens have too much space for this little bit of text.

To take up space, we could increase the margins and padding and we could make the text bigger. But we can't make phones smaller.

shankari · 2023-10-03T14:54:36Z

@JGreenlee not a showstopper for staging, but a reminder to test out the "install over existing app" (left screenshot) from
#998 (comment)

wrt ugly-looking summary page, that is not a showstopper for staging

I think that what we had last time was indeed a much bigger font and a lot more padding. We could also add a logo (not sure if we did or not), and potentially even have the logo be one that the partner provides. However, that is not a showstopper to get it onto staging - you can brainstorm with the team at the meeting tomorrow and make changes.

The goal is to have a simple, visually appealing screen that introduces participants to the app and the program before throwing a bunch of legal jargon at them. The equivalent of a "Welcome" or "Hola" screen while installing a phone, or even our current "welcome page", for example, which is very nice looking but doesn't say anything about what the app does or why (and can't really say the "why" because people haven't joined the study/program yet).

For the record, I used to not have the summary screen and added it based on a request from an early adopter (my friend).

I can merge the permission screen fix now if it is ready and you can address the others later today.

JGreenlee · 2023-10-03T15:05:51Z

The permission popup fix is ready and I'll look at the others later today.

JGreenlee · 2023-10-03T23:53:29Z

Does this look better? (on one big screen and one small screen)

shankari · 2023-10-05T03:17:55Z

Sure, that is not ugly any more. I still don't think it looks as beautiful as the current welcome screen.

Note that this is also a welcome screen of sorts - not a welcome to the app, but a welcome to the study/program. But I'm fine with merging this and seeing if you want to improve it later.

shankari · 2023-10-05T03:18:36Z

@JGreenlee #998 (comment) is still pending, would you like me to file a new issue?

JGreenlee · 2023-10-05T03:38:48Z

@JGreenlee #998 (comment) is still pending, would you like me to file a new issue?

I am not sure whether the backwards-compat issue will have already been resolved by the other fixes. I am unsure what specific steps I would take to reproduce it.

Can you clarify what state the old install was in when the new version was installed over it? Was it part-way through onboarding? Or not even past the welcome page?

shankari · 2023-10-05T03:55:42Z

Can you clarify what state the old install was in when the new version was installed over it? Was it part-way through onboarding? Or not even past the welcome page?

I believe it was not past the welcome page. Especially on iOS, I had just launched the iOS13 emulator to make sure that there was not WSOD, to ensure that we weren't missing issues in older versions as reported by Anat.

I would suggest the following checks. If all of them work, I am fine to close this issue.

Use the devapp with the current master branch
Switch to the onboarding... branch
Verify that the page is rendered correctly

Repeat steps 1-3 above after entering the opcode

Install the current production version of the app from the release binaries in the NREL internal repo
Install the new staging version over it by using the release binaries from the same location
Verify that the welcome page is updated

Repeat steps 1-3 above after entering the opcode

JGreenlee · 2023-10-05T15:58:10Z

Further changes will be needed. It appears that "logout" on the old version of the app cleared the config, but it did not clear the "INTRO_DONE" or "DATA_COLLECTION_CONSENTED_PROTOCOL" markers from local storage.

If the new version of the app encounters a blank config, but "INTRO_DONE" or "DATA_COLLECTION_CONSENTED_PROTOCOL" are still true, it will load in a broken state. The new version of the app will need a backwards-compat check that clears the the localstorage if there is no config.

JGreenlee · 2023-10-05T17:24:07Z

Use the devapp with the current master branch

Switch to the onboarding... branch

Verify that the page is rendered correctly

Repeat steps 1-3 above after entering the opcode

I did this part and was able to switch back-and-forth between master and my newest branch without issues. I tested this for both i) before starting onboarding and ii) in the middle of onboarding.

Install the current production version of the app from the release binaries in the NREL internal repo

Install the new staging version over it by using the release binaries from the same location

Verify that the welcome page is updated

The current staging release doesn't have the fixes. To see that it works, wouldn't I need to build the app with the fixes included?
Would it be easier to put out a new staging release that includes the fixes, and test this on real phones?

shankari · 2023-10-05T19:31:29Z

Fair point. Given that it is not reproducible in the devapp, I will consider this "tentatively fixed" and push out an RC today (by tonight at the latest) for testing on real phones.

JGreenlee · 2023-10-05T20:55:50Z

To clarify, I have a fix for:

Further changes will be needed. It appears that "logout" on the old version of the app cleared the config, but it did not clear the "INTRO_DONE" or "DATA_COLLECTION_CONSENTED_PROTOCOL" markers from local storage.

If the new version of the app encounters a blank config, but "INTRO_DONE" or "DATA_COLLECTION_CONSENTED_PROTOCOL" are still true, it will load in a broken state. The new version of the app will need a backwards-compat check that clears the the localstorage if there is no config.

at e-mission/e-mission-phone#1056 and it is ready to also be included in the RC, but it is uncertain whether it fixes the "undefined undefined undefined" issue.

shankari · 2023-10-27T18:52:35Z

@JGreenlee and @Abby-Wheelis were not able to reproduce this after all the fixes

shankari added this to OpenPATH Tasks Overview Oct 3, 2023

shankari moved this to Showstoppers for the current release in OpenPATH Tasks Overview Oct 3, 2023

shankari changed the title ~~Launching android app with #1032 pops up permission screen in summary~~ Launching ~android~ app with #1032 pops up permission screen in summary Oct 3, 2023

shankari changed the title ~~Launching ~android~ app with #1032 pops up permission screen in summary~~ Launching app with #1032 pops up permission screen in summary Oct 3, 2023

JGreenlee mentioned this issue Oct 3, 2023

fix permissions popup on 'summary' page; refactor e-mission/e-mission-phone#1048

Merged

JGreenlee mentioned this issue Oct 4, 2023

🐛 Onboarding fixes 10/3 e-mission/e-mission-phone#1050

Merged

Abby-Wheelis mentioned this issue Oct 5, 2023

❗ Login on fresh install resulting in dashboard error and "not logged in" #1002

Closed

shankari closed this as completed Oct 27, 2023

github-project-automation bot moved this from Showstoppers for the current release to Tasks completed in OpenPATH Tasks Overview Oct 27, 2023

shankari removed this from OpenPATH Tasks Overview Feb 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Launching app with #1032 pops up permission screen in summary #998

Launching app with #1032 pops up permission screen in summary #998

shankari commented Oct 3, 2023 •

edited

Loading

shankari commented Oct 3, 2023 •

edited

Loading

JGreenlee commented Oct 3, 2023

JGreenlee commented Oct 3, 2023 •

edited

Loading

JGreenlee commented Oct 3, 2023

shankari commented Oct 3, 2023 •

edited

Loading

JGreenlee commented Oct 3, 2023

JGreenlee commented Oct 3, 2023

shankari commented Oct 5, 2023

shankari commented Oct 5, 2023

JGreenlee commented Oct 5, 2023

shankari commented Oct 5, 2023

JGreenlee commented Oct 5, 2023

JGreenlee commented Oct 5, 2023

shankari commented Oct 5, 2023 •

edited

Loading

JGreenlee commented Oct 5, 2023

shankari commented Oct 27, 2023 •

edited

Loading

Launching app with #1032 pops up permission screen in summary #998

Launching app with #1032 pops up permission screen in summary #998

Comments

shankari commented Oct 3, 2023 • edited Loading

shankari commented Oct 3, 2023 • edited Loading

JGreenlee commented Oct 3, 2023

JGreenlee commented Oct 3, 2023 • edited Loading

JGreenlee commented Oct 3, 2023

shankari commented Oct 3, 2023 • edited Loading

JGreenlee commented Oct 3, 2023

JGreenlee commented Oct 3, 2023

shankari commented Oct 5, 2023

shankari commented Oct 5, 2023

JGreenlee commented Oct 5, 2023

shankari commented Oct 5, 2023

JGreenlee commented Oct 5, 2023

JGreenlee commented Oct 5, 2023

shankari commented Oct 5, 2023 • edited Loading

JGreenlee commented Oct 5, 2023

shankari commented Oct 27, 2023 • edited Loading

shankari commented Oct 3, 2023 •

edited

Loading

shankari commented Oct 3, 2023 •

edited

Loading

JGreenlee commented Oct 3, 2023 •

edited

Loading

shankari commented Oct 3, 2023 •

edited

Loading

shankari commented Oct 5, 2023 •

edited

Loading

shankari commented Oct 27, 2023 •

edited

Loading