search

NightscoutFoundation / xDrip

Nightscout version of xDrip+
https://jamorham.github.io/#xdrip-plus
GNU General Public License v3.0
1.41k stars 1.15k forks source link

Battery optimization prompt clarification #3540

Closed Navid200 closed 4 months ago

Navid200 commented 5 months ago

When a switch has a visual representation, it is best to have a unique summary specifying what the switch does when enabled.
As an example, please have a look at the following screenshots of the speak reading switch.
Screenshot_20240615-074535 Screenshot_20240615-074553
The summary says "it will speak each new reading" regardless of if the checkbox is enabled or not.
This tells the user what the checkbox does if/when enabled. This is a reasonable expectation from the user to consider the summary to explain what the switch does when enabled.

If the switch did not have a visual representation, it would make sense to use the summary to tell the user what tapping on the switch will do.
But, when the switch already has a visual representation, making the summary change depending on the switch condition makes it a confusing setup.

The following shows what the battery optimization prompt switch does now before this PR.
Screenshot_20240615-074836 Screenshot_20240615-074903
Look at the summary for when the checkbox is disabled. It says "recommended". What if the user confuses that with us telling them that we recommend that they enable the switch?



This PR

This PR changes the summary to one statement regardless of if the checkbox is enabled or not as shown below.
Screenshot_20240615-075026 Screenshot_20240615-075043

Now, we are using the summary to tell the user what the checkbox does when enabled. And the checkbox representation (checkmark present or not) tells the user if the checkbox has been set or not. Exactly the same as the speak reading checkbox and almost every other checkbox in xDrip.

asavageiv commented 5 months ago

Shouldn't it be called "No Battery Optimization Prompt"?

asavageiv commented 5 months ago

Good change regardless.

Navid200 commented 5 months ago

@asavageiv In my opinion negative switches should not be used anywhere. If we were creating xDrip from scratch, I would suggest to change this switch to one that would enable the prompt and set it to be enabled by default.

But, making that change now is problematic. If we change it now, we will also have to deal with what happens if someone decides to go back to a release prior to this pr and that will make the pr more complicated.

I am open to suggestions. I don't want anyone to think we have made a change that we really haven't. So, I didn't want to make too many changes.

I agree with what you are saying. But, I am not sure if it will help someone who sees this for the first time or not. Because the question remains even if we set the title to "No optimization prompt". A user may wonder if I want no optimization, do I enable the switch or do I disable it.

We need a convention that we consistently follow. For example, no negative switches ever.

asavageiv commented 5 months ago

That makes sense. I agree with using positive settings and it would really be separate from this change. Can we keep it in the model layer as it is now but invert it in the UI only?

Navid200 commented 5 months ago

Can we keep it in the model layer as it is now but invert it in the UI only?

You will need an even number of inversions or else, you will be flipping the logic.

asavageiv commented 4 months ago

Without analytics we can't really know how much it's used, but I'd be fine with it being removed.

Navid200 commented 4 months ago

I was about to open a PR to remove this setting and I saw this.

Screenshot 2024-06-27 102512



I think we need to keep it.