probe-android: QA for release 3.8.8-beta.764

hellais commented 3 months ago

First of all I am really happy with how the app is looking. I think this is a major leap forward in terms of aestetics and usability. It is however still quite rough around the edges and here is the feedback I have from some initial round of testing.

General new UI feedback

[ ] p0: When I install the app, without any intermediate screen it immediately asks for notification permissions without having the interim modal. We should instead only ask for push notification permission after the user has run the app a few times and after some amount of time. I believe this was documented somewhere in some requirements.
[ ] p0: The tap target on the selectors for the run tests is too small. I have a hard time selecting and deselecting tests even on a modern phone that has a decent and responsive screen. We should make the tap targets larger.
[ ] p0: related to this the tap targets sometime confuse selecting a test and collapsing a group. We need to do some more testing of the UX of this part to get it work smoothly. Probably increase the tap target of the checkboxes (and make them bigger) and make the tap target of the collapse much smaller.
[ ] p1: The fact that there is a test running looks good on the home screen, however the button to return to the home should now no longer be an arrow pointing down, but should rather be an arrow pointing back (you are not minimizing a toast, but are returning to previous screen)
[ ] p1: The circumvention tests are not part of auto-run by default. Is that desired?
[ ] p1: Same with experimental and actually to enable the auto-run of them I have to tap on the Run tests automatically button a few times before it becomes enabled
[ ] p1: Even though I gave the notification permission, in the app settings for notifications, they are still disable
[ ] p1: The ETA for a running test is no longer visible in the dashboard. We should provide some visible estimate (as we used to) or at the very least provide a percentage
[ ] p1: The tap targets on the details view are too small
[ ] p1: Sometimes results are not uploaded by default and I have to re-upload them again manually (IIRC this is an old bug)
~~[ ] p1: in the settings->advanced the copy should read "Language Settings" (with the "s") since it's plural. However I defer this to @agrabeli who is more native english than me and does copy~~ New issue: https://github.com/ooni/run/issues/185
~~[ ] p1: this is an old issue, but the prompt relative to the VPN being has very confusing options. We should work on improving the copy for it.~~ New issue: https://github.com/ooni/probe/issues/2760
[ ] p2: when I am running my custom VPN (InternetOK) all probe services fail and the timeout seems to high. I should spend more time debugging this, but seems like an important issue. edit: my custom VPN is broken for some reason, still the timeout maybe is a bit too high. New issue: https://github.com/ooni/probe/issues/2761
[ ] p2: even though I have a VPN active when I re-open the app the warning of a VPN being active goes away. edit: this is still valid issue. I don't know if I'm doing something silly in how I declare the VPN being such. We should work on figuring that out, but I am reducing the priority of these two issues. https://github.com/ooni/probe/issues/2762
[ ] p1: maybe when the test finishes it can say something under the run button like, hey we got your results, tap here to go to the result list? Since we don't redirect ATM it might be good to prod users in that direction.
[ ] p1: the share icon has a black outline that is different from the re-run test icon. They should probably be aligned.

OONI Run v2 specific

[ ] p0: when I open an OONI Run link and either I close it or install it there still remains a window open saying opening link. This might be behaviour specific to slack. The links were tapped on from slack.

Click to see attached screenshots.

[ ] p0: On the install link page, the logic for the run tests automatically is broken. Try clicking on it a couple of times and you will see it alternating between different states in a way that's not consistent.
[ ] p0: on the test details view, you have to tap twice on a section related to run tests automatically for it to actually be toggled.
~~[ ] p1: I am not really convinced by the layout of this section. It looks quite messy, but I can't think of a good solution now off the top of my head.~~ Tracking in a new issue: https://github.com/ooni/run/issues/186
[x] p2: this is more of a question, should we display a previous revision if it's the current one? Maybe not, since it's redundant. Perhaps we should only display revisions starting from N-1? What do people think about this?
[ ] p0: similar to what I already reported in the general section, the tap targets to select a custom OONI Run link are to small making them basically unclickable
[ ] p1: there should be some sort of separator between the sections of "official" OONI tests and OONI Run links in the run all button
[ ] p1: the icons for pure web_connectivity tests should maybe be consistent with the rest of the views "x blocked" "y accessible". It should not say generally "3 measured"
[ ] p1: why do we not have the category for some URLs? Is something missing in the backend? Should we document what the backend needs to do in order to fix this?

jbonisteel commented 3 months ago

p0: When I install the app, without any intermediate screen it immediately asks for notification permissions without having the interim modal. We should instead only ask for push notification permission after the user has run the app a few times and after some amount of time. I believe this was documented somewhere in some requirements.

Merging this PR will fix this https://github.com/ooni/probe-android/pull/603 - we will test again and then merge

p1: Even though I gave the notification permission, in the app settings for notifications, they are still disable

Further discussion between @hellais and @aanorbel needed because Android settings are complicated and it could depend on default settings on the individual's device.

[arturo] We discussed this and I am told this will be fixed by https://github.com/ooni/probe-android/pull/603

p1: The ETA for a running test is no longer visible in the dashboard. We should provide some visible estimate (as we used to) or at the very least provide a percentage

Further discussion between @hellais and @aanorbel needed here. The new app works the same as the current - the ETA is only visible on the test running screen. We need to discuss if we want to change that.

[arturo] we add a simple string that says ETA under the progress bar and is the same as what you get in the full view of the displayed toast

p1: maybe when the test finishes it can say something under the run button like, hey we got your results, tap here to go to the result list? Since we don't redirect ATM it might be good to prod users in that direction.

@hellais and @aanorbel need to discuss options and decide what to do.

[arturo] we can add a click here to see the latest results

p1: I am not really convinced by the layout of this section. It looks quite messy, but I can't think of a good solution now off the top of my head.

Will require further discussion to decide what to do here. @jbonisteel will make a separate issue to track

[arturo] this has been moved to another issue, we think of it in the future. https://github.com/ooni/run/issues/186

p2: this is more of a question, should we display a previous revision if it's the current one? Maybe not, since it's redundant. Perhaps we should only display revisions starting from N-1? What do people think about this?

It works this way currently.

[arturo] I was confused. This actually works as intendend.

We should however limit the number of revisions in the app to the last 5 and then have link to show more which points to the OONI Run v2 web interface.

p1: there should be some sort of separator between the sections of "official" OONI tests and OONI Run links in the run all button

Suggestion: add a horizontal rule, maybe copy that says 'My OONI Run Links'.

[arturo] this was added by norbel.

p1: why do we not have the category for some URLs? Is something missing in the backend? Should we document what the backend needs to do in order to fix this?

If we bring in a new URL that has never been run through checkin before, we don't know the category so it shows up as misc.

[arturo] We should file an issue in engine and backend.

hellais commented 2 months ago

We discussed this stuff with @aanorbel and I added comments marked with [arturo]

Lanius-collaris commented 2 months ago

Does it crash on Android 7.0? ooni/probe#2750

jbonisteel commented 2 months ago

p1: why do we not have the category for some URLs? Is something missing in the backend? Should we document what the backend needs to do in order to fix this?

If we bring in a new URL that has never been run through checkin before, we don't know the category so it shows up as misc.

[arturo] We should file an issue in engine and backend.

Issue has been filed in backend for now: https://github.com/ooni/backend/issues/859

jbonisteel commented 2 months ago

I am testing build

General new UI feedback

[ ] p0: When I install the app, without any intermediate screen it immediately asks for notification permissions without having the interim modal. We should instead only ask for push notification permission after the user has run the app a few times and after some amount of time. I believe this was documented somewhere in some requirements.

This is partially done - the app no longer immediately asks for notification permission but currently unclear about when it should ask

[x] p0: The tap target on the selectors for the run tests is too small. I have a hard time selecting and deselecting tests even on a modern phone that has a decent and responsive screen. We should make the tap targets larger.

The tap targets look better for me, Arturo will need to confirm if this is better for him

[x] p0: related to this the tap targets sometime confuse selecting a test and collapsing a group. We need to do some more testing of the UX of this part to get it work smoothly. Probably increase the tap target of the checkboxes (and make them bigger) and make the tap target of the collapse much smaller.

This looks good, again it would be best for Arturo to confirm

[x] p1: The fact that there is a test running looks good on the home screen, however the button to return to the home should now no longer be an arrow pointing down, but should rather be an arrow pointing back (you are not minimizing a toast, but are returning to previous screen)

The arrow points back now

[x] p1: The circumvention tests are not part of auto-run by default. Is that desired?
[x] p1: Same with experimental and actually to enable the auto-run of them I have to tap on the Run tests automatically button a few times before it becomes enabled

Both are now selected to run automatically, I assume that is what is desired. the tap function seems to work better

[ ] p1: Even though I gave the notification permission, in the app settings for notifications, they are still disable

I will re-test this once notifications stuff is sorted out

[x] p1: The ETA for a running test is no longer visible in the dashboard. We should provide some visible estimate (as we used to) or at the very least provide a percentage

There is now an ETA on the dashboard

[x] p1: The tap targets on the details view are too small

Works okay for me, but Arturo should confirm he is happy with it

[ ] p1: Sometimes results are not uploaded by default and I have to re-upload them again manually (IIRC this is an old bug)

My results were all automatically uploaded by default so I couldn't reproduce this. If this is a known bug we likely shouldn't block on it

[x] p1: maybe when the test finishes it can say something under the run button like, hey we got your results, tap here to go to the result list? Since we don't redirect ATM it might be good to prod users in that direction.

We now display a message saying that the tests results are ready to view

[x] p1: the share icon has a black outline that is different from the re-run test icon. They should probably be aligned.

This has been addressed

OONI Run v2 specific

[ ] p0: when I open an OONI Run link and either I close it or install it there still remains a window open saying opening link. This might be behaviour specific to slack. The links were tapped on from slack.

I don't think this has been fixed, and I am not sure if it can be fixed. I will attached a screenshot of what I see

[x] p0: On the install link page, the logic for the run tests automatically is broken. Try clicking on it a couple of times and you will see it alternating between different states in a way that's not consistent.

This looks okay, Arturo should confirm he is happy with it

[x] p0: on the test details view, you have to tap twice on a section related to run tests automatically for it to actually be toggled.

This also looks okay, I don't have to tap twice to select run automatically

[x] p2: this is more of a question, should we display a previous revision if it's the current one? Maybe not, since it's redundant. Perhaps we should only display revisions starting from N-1? What do people think about this?
[x] p0: similar to what I already reported in the general section, the tap targets to select a custom OONI Run link are to small making them basically unclickable

This also looks okay, Arturo should confirm that this works better for him

[x] p1: there should be some sort of separator between the sections of "official" OONI tests and OONI Run links in the run all button

We now have a separator

[x] p1: the icons for pure web_connectivity tests should maybe be consistent with the rest of the views "x blocked" "y accessible". It should not say generally "3 measured"

It now reads 'x blocked y tested' - but it seems inconsistent across our different tests. For example, we use the words available, accessible and tested. I double checked against the current production app, and this seems consistent now with what we do currently

jbonisteel commented 2 months ago

To summarize, the outstanding issues are as follows:

Sorting out exactly when/where the notification permission happens, and ensuring appropriate settings are toggled in the settings section
Uploading sometimes failing - I couldn't reproduce in this round of testing, but if it is an old bug we likely shouldn't block on it. @aanorbel do you know if there is already an issue filed for this somewhere?
This comment: when I open an OONI Run link and either I close it or install it there still remains a window open saying opening link. This might be behaviour specific to slack. The links were tapped on from slack. I think I can reproduce this still, but I am not really sure if we can change this behavior? It seems that when I tap on a link, install it in the app, I can still switch between apps and I see the previous 'install' view in Slack. (see screenshot). But not sure if this is default behaviour and if we can change this? At any rate, it doesn't effect or change the core v2 flow.

Click to see attached screenshots.

This comment: We should however limit the number of revisions in the app to the last 5 and then have link to show more which points to the OONI Run v2 web interface. We do limit now to the latest 5, it seems, but there is no link to the web interface.

aanorbel commented 2 months ago

Here are a few clarifications.

Sorting out exactly when/where the notification permission happens, and ensuring appropriate settings are toggled in the settings section

The current notification preferences for the app are to manage censorship alert notifications. Additionally, we need to request permission to show notifications from the android operating system which is the system modal shown on first install. We have to revisit this in a completely separate issue to address usability and other related chalenges.

Uploading sometimes failing - I couldn't reproduce in this round of testing, but if it is an old bug we likely shouldn't block on it. @aanorbel do you know if there is already an issue filed for this somewhere?

We don't have an issue for this yet. I think we should create one that would also allow us to made the measurement upload more resilient in cases where the backend is not available/overloaded at the time of the upload.

This comment: when I open an OONI Run link and either I close it or install it there still remains a window open saying opening link. This might be behaviour specific to slack. The links were tapped on from slack. ........

This issue has been fixed. Its was more a problem with the loading screen still being displayed when the user returns to slack and not what is show in the recent activities.

This comment: We should however limit the number of revisions in the app to the last 5 and then have link to show more which points to the OONI Run v2 web interface. We do limit now to the latest 5, it seems, but there is no link to the web interface.

We are currently limiting the number of revision on mobile to 5. You can always check the web for complete list of revisions.

jbonisteel commented 2 months ago

Adding convo from Slack:

For this comment: We are currently limiting the number of revision on mobile to 5. You can always check the web for complete list of revisions. In Arturo's original feedback he said "We should however limit the number of revisions in the app to the last 5 and then have link to show more which points to the OONI Run v2 web interface.' - there is no 'show more' link. Yes, there are links to the individual revisions, but if you want to see all revisions, you have to click 'current link' on this page: https://run.test.ooni.org/revisions/10436?revision=7 - so my interpretation of Arturo's feedback was that he wanted a link that says 'show more' that could take you to the current link which displays all revisions.

@aanorbel will add the show more link

jbonisteel commented 2 months ago

I filed a separate issue for the uploading issue: https://github.com/ooni/probe/issues/2775 we should decide if we want to block shipping Run v2 on this. My opinion would be no, not if it is a long-standing issue.

jbonisteel commented 2 months ago

Updated summary of where things stand now. These are the outstanding items we should discuss together:

Notifications - in a discussion with Arturo he mentioned maybe we should just disable asking for notifications altogether. So we should decide what to do.
The issue with results sometimes not uploading: I have filed a separate issue, we need to decide if this would block Run v2
We previously discussed that while we are not totally happy with the test overview screen layout, we should proceed anyway. Just want to confirm that is still the plan.

ooni / probe