Shake on matching kanji default behavior

hummusw commented 5 months ago

NOTE: This post is a bit longer than usual because it deals with some complex behavior regarding what the "default" behavior is for advanced settings. I've tried to summarize it as briefly as I could in the TL;DR section at the bottom

The issue

I saw that there was an earlier pull request to change the default behavior to set shake_on_matching_kanji to true in #54, where we wanted to match the website behavior closer (judging from the conversation in #34).

This puts the app in an interesting spot. Based on a brief testing session, it looks like all preference values are set to their default values when the user first installs the app. This means all users who installed the app before v1.0.9 have shake_on_matching_kanji set to false by default. Only new users who installed the app (or reset their data by clearing app storage) since v1.0.9 will have shake_on_matching_kanji set to true by default.

But there's more, because we don't only have one default value, we have two! In GlobalSettings.java, there's a check if the user has advanced settings enabled. If they are disabled, then we use the non-advanced behavior.

See in GlobalSettings.java:

public static boolean getShakeOnMatchingKanji() {
    if (!getAdvancedEnabled()) {
        return false;
    }
    return prefs().getBoolean("shake_on_matching_kanji", true);
}

This second "non-advanced" default value changes depending on the currently installed version of the app, not when the app was initially installed.

So this is where things get interesting...

The possible solutions

(all tables below assume the user is using the latest version of the app)

Option 0: no action

If we leave things as is, this is how different users will see the shaking behavior if they have not touched the setting:

Shakes by default/Saved preference value	Advanced settings off	Advanced settings on
Installed before v1.0.9	no/`false`	no/`false`
Installed since v1.0.9	no/`true` (?)	yes/`true`

New users with advanced settings off are in a strange boat where the preference is actually set to true, but the app behaves as if it were set to false.

Option 1: revert change

If we change the setting back to false, then new users moving forward will see the old behavior again.

Shakes by default/Saved preference value	Advanced settings off	Advanced settings on
Installed before v1.0.9	no/`false`	no/`false`
Installed between v1.0.9 and v1.1.1 (inclusive)	no/`true` (?)	yes/`true`
Installed after v1.1.1	no/`false`	no/`false`

This is a step away from mirroring the website behavior. However, we also minimize the number of users who have two different sets of default shaking behaviors.

Option 2: change non-advanced default behavior

If we change the default value for non-advanced users to true, then all non-advanced users will see the new behavior.

Shakes by default/Saved preference value	Advanced settings off	Advanced settings on
Installed before v1.0.9	yes/`false` (?)	no/`false`
Installed since v1.0.9	yes/`true`	yes/`true`

This means that more users see the new behavior. Advanced users who have had the app installed for a while are not affected.

Option 3: make it a non-advanced setting

If we change the category this setting is in from "Other advanced settings" to "Other settings", then we no longer have conflicting default values.

Shakes by default/Saved preference value	All users
Installed before v1.0.9	no/`false`
Installed since v1.0.9	yes/`true`

This means we are "downgrading" this option from advanced to non-advanced. Users will continue to see the default behavior that they are used to seeing.

TL;DR

For any advanced setting (such as shake_on_matching_kanji), there are two default values:

The value saved in the shared preferences file (set upon first installation of app)
The value used if advanced settings are turned off (changes based on currently installed app)

All options (except for 3) result in a group of users who will see a change in default behavior just by turning on or off the advanced settings switch. We need to decide what group that will be, and if necessary, how to communicate this change to them.

hummusw commented 5 months ago

Personally, I'm leaning towards either option 2 as a "we've decided to change the default behavior" solution. This keeps in line with the spirit of mirroring the website by default.

If we assume users with advanced settings turned off are "normal users", then more "normal users" will be able to get the new behavior that mirrors the website closer regardless of when they first installed the app.
If we assume users with advanced settings turned on are "power users" who are more likely to check all the settings and fine-tune it to their preferences:
- Those who have already had advanced settings on will see no change to the behavior when they first installed the app (keep it as they like it).
- Long-time users who turn on advanced settings for the first time will see a change. However, they may also see this setting while checking out the advanced options and make a decision on whether to turn it on or off.

jerryhcooke commented 1 month ago

Sorry for taking so long to reply to thing, I've been absolutely snowed under lately.

My instinct would be towards option 2 as well, as the intention was the make the default state one that mirrors website defaults. I'll pop this change into the next build 👍

jerryhcooke / smouldering_durtles