Allow choosing a ringtone for FindMyPhone
ClosedPublic

Authored by nicolasfella on Feb 5 2018, 9:23 PM.

Details

Summary

BUG: 379023

Let users choose a ring tone in the settings.

Test Plan

Open settings, choose a ringtone, search for device.

Diff Detail

Repository
R225 KDE Connect - Android application
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nicolasfella requested review of this revision.Feb 5 2018, 9:23 PM
nicolasfella created this revision.
apol added a subscriber: apol.Feb 6 2018, 1:14 AM
apol added inline comments.
src/org/kde/kdeconnect/Plugins/FindMyPhonePlugin/FindMyPhoneActivity.java
89

It's not defaulting to RingtoneManager.getDefaultUri(RingtoneManager.TYPE_RINGTONE), right?

nicolasfella added inline comments.Feb 6 2018, 1:21 AM
src/org/kde/kdeconnect/Plugins/FindMyPhonePlugin/FindMyPhoneActivity.java
89

It should use the same default like before

I don't see the need for this setting. Which is the use case for this? I mean, why the default ringtone is not good to find your phone so you might want to change it?

In general, I think settings should only be added when there is a use case for which people might need to change them. Otherwise, I prefer less complexity (both in the UI and in the code).

A use case is described in the Bug report

Why not include looped sound, which could then be identified as specifically KDE Connect. Such as with cordless house phones.

albertvaka accepted this revision.Feb 17 2018, 10:56 AM

True, I still find it a bit weird but let's make GizmoChicken happy :P

This revision is now accepted and ready to land.Feb 17 2018, 10:56 AM

About the KDE Connect-specific ringtone: I'm fine with it, it could be shipped as the default as long as it doesn't make the app size grow by much.

Shipping our own ringtone AND having the setting seems an overkill to me. Since we got an awesome ringtone picker "for free" the complexity seems bearable to me. The actual Activity isn't much bigger in the end

The code looks good to me (didn't test yet though).

The change is small and it actually helps some users, so I'd say this is ok. Shipping our own sound would work too, but I think that actually matters a lot.

This revision was automatically updated to reflect the committed changes.