you-apps / RecordYou

Privacy focused recorder app built with MD3
https://you-apps.net
GNU General Public License v3.0
739 stars 30 forks source link

Added a screen annotation tool (experimental) #154

Closed SuhasDissa closed 1 year ago

SuhasDissa commented 1 year ago

I just added a way to annotate screen while screen recording

DEMO:

https://github.com/Bnyro/RecordYou/assets/64766434/6d8ff0b1-d9ae-4e11-8600-628c9665f022

SuhasDissa commented 1 year ago

Hi there!

I'm excited to announce that I've finally made the annotation tool production ready. I've tested it thoroughly, but there may still be some bugs that I haven't found.

Would you mind taking a look at it and letting me know what you think? Any suggestions are welcome.

Thanks!

Bnyro commented 1 year ago

This looks really cool, thanks for your work so far! I'll take a look as soon as possible!

Bnyro commented 1 year ago

This works really well! Most of my comments are only some minor formatting changes.

Please also make all the strings translatable by adding them to res/values/strings.xml and using the stringResource(stringId) function instead of hardcoding them in English.

Looks great otherwise!

SuhasDissa commented 1 year ago

Please also make all the strings translatable by adding them to res/values/strings.xml and using the stringResource(stringId) function instead of hardcoding them in English.

I didn't want to mess with your strings file. I just added that strings for context. I suggest you to change those strings to more formal descriptions as you like.

Bnyro commented 1 year ago

Okay, makes sense! Two other things we should do for user convenience before merging:

  1. Add a preference to disable the annotation tool, nevertheless the overlay should be enabled by default
  2. Prompt users to allow RecordYou to display above other apps when they start a screen recording (I think we can open the Android settings for that with an intent).

It doesn't matter to me who of us is going to quickly add these two things, as you prefer :)

Bnyro commented 1 year ago

Please use the Preferences object for handling with preferences instead of a Context#prefereces extension function, I'd like to keep that consistent across the whole app. That'll be useful if we migrated to Jetpack Datastore for example.

SuhasDissa commented 1 year ago

Just a side note here, I suggest you to put these lines in your Preferences.kt file


inline fun <reified T : Enum<T>> SharedPreferences.getEnum(
    key: String,
    defaultValue: T
): T =
    getString(key, null)?.let {
        try {
            enumValueOf<T>(it)
        } catch (e: IllegalArgumentException) {
            null
        }
    } ?: defaultValue

inline fun <reified T : Enum<T>> SharedPreferences.Editor.putEnum(
    key: String,
    value: T
): SharedPreferences.Editor =
    putString(key, value.name)

val Context.preferences: SharedPreferences
    get() = getSharedPreferences("preferences", Context.MODE_PRIVATE)

@Composable
fun rememberPreference(key: String, defaultValue: Boolean): MutableState<Boolean> {
    val context = LocalContext.current
    return remember {
        mutableStatePreferenceOf(context.preferences.getBoolean(key, defaultValue)) {
            context.preferences.edit { putBoolean(key, it) }
        }
    }
}

@Composable
fun rememberPreference(key: String, defaultValue: Int): MutableState<Int> {
    val context = LocalContext.current
    return remember {
        mutableStatePreferenceOf(context.preferences.getInt(key, defaultValue)) {
            context.preferences.edit { putInt(key, it) }
        }
    }
}

@Composable
fun rememberPreference(key: String, defaultValue: String): MutableState<String> {
    val context = LocalContext.current
    return remember {
        mutableStatePreferenceOf(context.preferences.getString(key, null) ?: defaultValue) {
            context.preferences.edit { putString(key, it) }
        }
    }
}

@Composable
inline fun <reified T : Enum<T>> rememberPreference(key: String, defaultValue: T): MutableState<T> {
    val context = LocalContext.current
    return remember {
        mutableStatePreferenceOf(context.preferences.getEnum(key, defaultValue)) {
            context.preferences.edit { putEnum(key, it) }
        }
    }
}

inline fun <T> mutableStatePreferenceOf(
    value: T,
    crossinline onStructuralInequality: (newValue: T) -> Unit
) =
    mutableStateOf(
        value = value,
        policy = object : SnapshotMutationPolicy<T> {
            override fun equivalent(a: T, b: T): Boolean {
                val areEquals = a == b
                if (!areEquals) onStructuralInequality(b)
                return areEquals
            }
        })

This makes the life much easier when it comes to managing preferences

SuhasDissa commented 1 year ago

Please use the Preferences object for handling with preferences instead of a Context#prefereces extension function, I'd like to keep that consistent across the whole app. That'll be useful if we migrated to Jetpack Datastore for example.

Sorry for that. I thought you'd like that

Bnyro commented 1 year ago

I've seen something like that on other apps before already, and I do really like the approach. However, in my opinion it should be used across the whole app then, and not only in a single file, which would make it very inconsistent and more difficult to maintain in general.

Bnyro commented 1 year ago

And our current approach should have a slightly better performance than using the extension functions because we don't recreated a SharedPreferences object for each preference, but that's mostly irrelevant anyways.

SuhasDissa commented 1 year ago

I've seen something like that on other apps before already, and I do really like the approach. However, in my opinion it should be used across the whole app then, and not only in a single file, which would make it very inconsistent and more difficult to maintain in general.

the rememberPreference function lets you edit the variable directly and thus save the preference. so there's no need to save the preference each time the value change

for example

var checked by rememberPreference(prefKey,false)

Checkbox(
            checked = checked,
            onCheckedChange = {
                checked = it
            }
        )
Bnyro commented 1 year ago

the rememberPreference function lets you edit the variable directly and thus save the preference. so there's no need to save the preference each time the value change

Yes, that's pretty cool! I wouldn't mind merging a PR for that if you'd like to work on it, but then all preferences should use it the same way.

Bnyro commented 1 year ago

And thanks for your contribution! <3