Skip to content
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

Implement wifi no-download alert #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jafayer
Copy link
Contributor

@jafayer jafayer commented May 24, 2022

Known issues: clicking "download all" will initialize n notifications and probably crash the app.

@jafayer jafayer requested a review from SyntaxBlitz May 24, 2022 01:01
@@ -64,7 +65,19 @@ const AllLessons = ({route}: {route: any}) => {
},
{
text: 'OK',
onPress: () => {
onPress: async() => {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

del


const [wifiOnly, network] = await Promise.all([
genPreferenceDownloadOnlyOnWifi(),
NetInfo.fetch()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

,


if (wifiOnly && network.type !== 'wifi') {
Alert.alert(`You\'re not currently connected to wi-fi, and your preferences don\'t allow downloads over data.
\nPlease go to your Language Transfer settings and disable "Download Only on wi-fi" to continue.`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowercase the o

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's put '\n' on the prev line

Comment on lines 103 to 112
const [wifiOnly, network] = await Promise.all([
genPreferenceDownloadOnlyOnWifi(),
NetInfo.fetch()
]);

if (wifiOnly && network.type !== 'wifi') {
Alert.alert(`You\'re not currently connected to wi-fi, and your preferences don\'t allow downloads over data.
\nPlease go to your Language Transfer settings and disable "Download Only on wi-fi" to continue.`);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dedup the entire block, put it somewhere vaguely in js/

if (wifiOnly && network.type !== 'wifi') {
Alert.alert(`You\'re not currently connected to wi-fi, and your preferences don\'t allow downloads over data.
\nPlease go to your Language Transfer settings and disable "Download Only on wi-fi" to continue.`);
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not return. and workshop the copy a bit

]);

if (wifiOnly && network.type !== 'wifi') {
Alert.alert(`You\'re not currently connected to wi-fi, and your preferences don\'t allow downloads over data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think you need backslashes here

@jafayer jafayer force-pushed the warn-when-mobile-data branch from 01e4b8c to 76b1538 Compare June 14, 2022 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants