mueller-ma / PrepaidBalance

App to check your prepaid balance
Apache License 2.0
35 stars 14 forks source link

Settings: use `displayName` if `carrierName` is not set. #297

Closed juleskers closed 1 month ago

juleskers commented 1 month ago

I've scratched together enough free time to fix the issue I researched in #296 . As usual, figuring out what the problem was took 10x the time as actually solving it :sweat_smile:

As documented in #296, the UI-setting element will mis-detect an empty sim-card name string as "not set", even though the setting is active (underlying setting uses the slot-index, so all other machinery works).

I'm adding a fallback to displayName (user-settable) under those circumstances. Showing what the user calls the sim-card is definitely better than nothing.

Open to discussion: should user-chosen displayName always be preferred over system-defined carrierName?
The carriername is what is shown on the status-bar, so is what most people probably recognise the fastest.

closes #296

juleskers commented 1 month ago

P.S. I feel bad that I'm not including a unit test for this. I'm completely new to Android UI, so I haven't the foggiest on where to start writing a test of UI-elements.

Any tips would be appreciated.

mueller-ma commented 1 month ago

Open to discussion: should user-chosen displayName always be preferred over system-defined carrierName? The carriername is what is shown on the status-bar, so is what most people probably recognise the fastest.

Maybe you can check if both strings are equal (e.g. on my phone that's the case) and otherwise use "displayName (carrierName)" as entry.

It's fine when you don't add a test here.

juleskers commented 1 month ago

Thanks for the feedback!

I won't feel guilty about not having a test then :slightly_smiling_face:

As for displayname/carriername: I think having both would quickly become redundant, and there isn't that much space in that menu.

At least for me, the display-name is "just" the original carrier, I manually set it, since I'm always roaming (living abroad). For example, for me it would become: "KPN (KPN - O2 DE)". So zero added info.

Of course other people may have different naming conventions, but I doubt it's a very big niche, and thus I'd prefer "if it ain't broken, don't fix it".


I am also cautious, since I have another plan to improve the recognisability of the sim-entries: include the sim-specific accent colour in the drop-down.

SubscriptionInfo provides a [getIconTint()](https://developer.android.com/reference/android/telephony/SubscriptionInfo#getIconTint()) and a createIconBitmap(..), which I think would be far more recognisable.

I hope to experiment with the custom dropdown from this StackOverflow: https://stackoverflow.com/questions/32226245/how-to-set-icon-in-listpreferences-item-in-android, however that's going to be a separate MR (if ever, I do have young children :sweat_smile: ).

mueller-ma commented 1 month ago

Thanks for the bug fix :)

I think the colored dropdown would be nice to have, but it adds some complexity for only a little bit of improvement for the user and I'd rather keep this app easy to maintain.

juleskers commented 1 month ago

You're welcome! Thanks for the wonderful app!
It really helps me keep track of costs, where my carrier is intentionally(?) vague about them. (they're probably hoping for bigger spending if you don't notice the amounts flowing 🫤)

Rw: coloured icons: I get what you are saying about maintainability. I wish to learn some Android UI anyway, so would it be alright if I take a stab anyway? If the solution remains simple enough, you can accept it. If not, a rejection won't hurt me now that I am forewarned. The learning will be value enough for me, even if you don't accept ❤️