-
Notifications
You must be signed in to change notification settings - Fork 984
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
[#9986] Allow to disable fetching of tx history on mobile network #11007
Conversation
Jenkins BuildsClick to see older builds (36)
|
7c6364d
to
9bcbb4a
Compare
@rasom This is a good solution. Didn't learn too much about bandwidth use in target markets unfortunately, but anyway we can reduce and give control makes sense. Does @errorists what are your thoughts on Enable/Disable message syncing and tx syncing over mobile data? Should this be a single setting or should both be controlled separately? As this impacts Settings and the Sync over Mobile bottom sheet |
@hesterbruikman it should be a single setting and unified with the chat dialogues |
@hesterbruikman @errorists |
🙌 great! Thanks @rasom I think this needs minor copy changes to accurately represent the mobile data switch. What do you think of below @errorists @rasom This is the fastest route I can think of to include in this PR. You might want to better organize the sync settings though. If so I'll create a separate issue Note that white line in the bottom sheet is weird 🤔 |
@hesterbruikman
We could have two options there:
As far as I understand currently wallet requires much more traffic than messages (@cammellos @oskarth do you confirm?). There are many users who use the app as a messenger in the first place, not as a wallet. For them it might make sense to keep syncing only messages history. And we can also add a button in wallet which will refresh history on demand without listening to the network (thus without using mobile data constantly). |
If you're feeling adventurous, this is a long outstanding issue to reduce frequency of fetching and include fetching on demand: #10242 (pull to refresh). Having this manual fetching, I think we could keep the mobile data setting a catch all Mobile data
Potential addition to the original design of pull to refresh could be: |
@hesterbruikman I would merge this PR at first and then take a look at #10242 later |
Sounds good to me. Also without #10242 being able to switch off sync over mobile data for messages and tx is a win if you want to spare your data plan. Let's consider |
95% of end-end tests have passed
Failed tests (5)Click to expand
Passed tests (92)Click to expand |
ISSUE / QUESTION 1 IOS vs Android - different behaviorSteps:
On IOS(LTE):
on Android(4G+):
Logs from PR build, Android: Status-debug-logs.zip ISSUE 2 On IOS when creating account on mobile network after enabling wifi transaction history is not loaded until app restart (also in nightly)As it is an issue in nightly as well, can be reported separately, so it is in case you can take a look in this PR into this.
Expected result: transaction history is loaded |
is issue 2 iOS specific? |
It is not reproducible on Android, only on IOS. Tried on IOS 13.5.1, however, can try on another IOS versions if it makes sense |
@churik both issues should be fixed now |
75% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (3)Click to expand
|
100% of end-end tests have passed
Passed tests (1) |
Android: Messaging in 1-1 chats is still working while on 3G network. GREAT WORK! |
The esiest way to test it would be to compare data usage on release version and this one. You would need to switch to mobile network, select "stop syncing", and then keep app in the foreground for 5mins.
fixes #9986
status: ready