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

Made connectivity card message optional #310

Merged
merged 6 commits into from
May 30, 2023
Merged

Conversation

ilievski-david
Copy link
Contributor

Made connectivity card message optional

The connectivity card shows every time you are on a mobile network. This would be useful for first time users but using the app often just becomes an annoyance. You already know how it works, why should you be stuck with an immutable notification. This PR solves the problem by adding an option in the settings menu to disable the connectivity card from showing.

Copy link
Owner

@fm-sys fm-sys left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! I'm always surprised how much such a (rather small and helpful?) hint can bother users 😅😂

I've added some comments, especially suggested some strings written more from user perspective

@@ -54,6 +54,8 @@
<string name="keep_on_title">Keep screen on while transferring</string>
<string name="floating_text_selection_title">Floating text selection</string>
<string name="floating_text_selection_summary">When selecting text in other apps, an option \"Send with Snapdrop\" will be shown</string>
<string name="show_connectivity_card_title">Display connectivity card</string>
<string name="show_connectivity_card_summary"> Card to notify you that you need to pair or connect to the same wifi</string>
Copy link
Owner

@fm-sys fm-sys May 30, 2023

Choose a reason for hiding this comment

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

Maybe something like:

Show a warning when you are not connected with a WiFi network

@@ -54,6 +54,8 @@
<string name="keep_on_title">Keep screen on while transferring</string>
<string name="floating_text_selection_title">Floating text selection</string>
<string name="floating_text_selection_summary">When selecting text in other apps, an option \"Send with Snapdrop\" will be shown</string>
<string name="show_connectivity_card_title">Display connectivity card</string>
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe just "Connectivity banner"?

Users might not know what a "card" UI element is, and that enabled = show is pretty self-explanatory IMHO

Copy link
Owner

Choose a reason for hiding this comment

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

Just pushed a small change: "Display connectivity warning" to "Connectivity warning" which is a bit shorter...

Comment on lines 149 to 160
final Preference showConnectivityCardPref = findPreference(getString(R.string.pref_show_connectivity_card));

showConnectivityCardPref.setVisible(true);
showConnectivityCardPref.setOnPreferenceChangeListener((pref, newValue) -> {

PreferenceManager.getDefaultSharedPreferences(getContext()).edit().putBoolean(showConnectivityCardPref.getKey(), (boolean) newValue).apply();
return true;
});




Copy link
Owner

Choose a reason for hiding this comment

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

This whole code part is not necessary, the android Preferences framework will automatically store preference changes for you ;-)

@ilievski-david
Copy link
Contributor Author

ilievski-david commented May 30, 2023

I've added some comments, especially suggested some strings written more from user perspective

I pushed a new commit fixing the things you mentioned.

Thanks for your PR! I'm always surprised how much such a (rather small and helpful?) hint can bother users 😅😂

Well yes, everything that's not necessary is annoying one way or another. Unless it's a gif of a cat running back and forward with a usb stick.

@fm-sys
Copy link
Owner

fm-sys commented May 30, 2023

Oh no, now I want to have this cat!

DALL·E2023-05-3016 44 59-Cutecatwithanusbstickrunningaroundwithwhitebackgroundclipart_copy_256x256_edit_168411624590968

@ilievski-david
Copy link
Contributor Author

It will cost you 2 bites per second.

@fm-sys
Copy link
Owner

fm-sys commented May 30, 2023

Thanks again for your contribution!

@fm-sys fm-sys merged commit da96c8b into fm-sys:master May 30, 2023
2 checks passed
@ilievski-david
Copy link
Contributor Author

Thanks again for your contribution!

No problem! Awesome App.

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.

None yet

2 participants