-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
probe-android: QA for release 3.8.8-beta.764 #2756
Comments
Merging this PR will fix this ooni/probe-android#603 - we will test again and then merge
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 ooni/probe-android#603
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
@hellais and @aanorbel need to discuss options and decide what to do. [arturo] we can add a click here to see the latest results
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. ooni/run#186
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.
Suggestion: add a horizontal rule, maybe copy that says 'My OONI Run Links'. [arturo] this was added by norbel.
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. |
We discussed this stuff with @aanorbel and I added comments marked with |
Does it crash on Android 7.0? |
Fixes ooni/probe#2756 ## Proposed Changes - Change share icon. - WebConnectivity row for run v2 tests - Change checkbox size - Add dividers to `RunTestActivity` - Add `eta` to progress fragment. - Limit the number of revisions displayed. |.|.|.|.|.| |-|-|-|-|-| | ![Screenshot_20240705_192047](https://github.com/ooni/probe-android/assets/17911892/d3b02572-533e-484b-a1aa-e81c7e0e4d3f) | ![Screenshot_20240705_192104](https://github.com/ooni/probe-android/assets/17911892/6cbffd29-6c1d-4684-bf6c-8bc5afec4c3c) | ![Screenshot_20240705_192121](https://github.com/ooni/probe-android/assets/17911892/2305fd82-a1f5-41b0-918f-b60e5f00a135) | ![Screenshot_20240705_192157](https://github.com/ooni/probe-android/assets/17911892/641e5561-3bfa-48f7-9155-d4723725d663) | ![Screenshot_20240705_194853](https://github.com/ooni/probe-android/assets/17911892/0ba4086f-2f58-49cb-bcb4-e1183a930321) |
Issue has been filed in backend for now: ooni/backend#859 |
I am testing build General new UI feedback
This is partially done - the app no longer immediately asks for notification permission but currently unclear about when it should ask
The tap targets look better for me, Arturo will need to confirm if this is better for him
This looks good, again it would be best for Arturo to confirm
The arrow points back now
Both are now selected to run automatically, I assume that is what is desired. the tap function seems to work better
I will re-test this once notifications stuff is sorted out
There is now an ETA on the dashboard
Works okay for me, but Arturo should confirm he is happy with it
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
We now display a message saying that the tests results are ready to view
This has been addressed OONI Run v2 specific
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
This looks okay, Arturo should confirm he is happy with it
This also looks okay, I don't have to tap twice to select run automatically
This also looks okay, Arturo should confirm that this works better for him
We now have a separator
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 |
To summarize, the outstanding issues are as follows:
|
Here are a few clarifications.
The current notification preferences for the app are to manage
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 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.
We are currently limiting the number of revision on mobile to 5. You can always check the web for complete list of revisions. |
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. @aanorbel will add the show more link |
I filed a separate issue for the uploading issue: #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. |
Fixes ooni/probe#2756 (comment) ## Proposed Changes - Add see more button when revisions are more than 5 |.|.|.| |-|-|-| | ![Screenshot_20240712_140730](https://github.com/user-attachments/assets/225d7a72-0a6c-4d57-b076-cbddffedfb4b) | ![Screenshot_20240712_140840](https://github.com/user-attachments/assets/1db3d447-c9f2-4cf4-b179-50a09cac9fd5) | ![Screenshot_20240712_140851](https://github.com/user-attachments/assets/3a29df1f-508d-4354-90bb-9434c9682510) |
Updated summary of where things stand now. These are the outstanding items we should discuss together:
|
I will close this issue for now. At this point, we have addressed actionable feedback and made issues for stuff we might follow up on in the future |
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
[ ] 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 copyNew issue: Copy editing: decide the wording to use for Language settings run#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: probe-android: improve copy about having a VPN enabled #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: probe-android: improve timeout situation when there is a VPN enabled #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.probe-android: improve warnings when a VPN in enabled #2762OONI Run v2 specific
Click to see attached screenshots.
[ ] 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: Run v2 test overview screen: improve layout run#186The text was updated successfully, but these errors were encountered: