Merge "Add log on preference widgets."

2022-10-18 06:11:16 +00:00
parent b2a3d85f63 5ecb15979d
commit 7bc0dbddcd
8 changed files with 125 additions and 26 deletions
--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/BrowseActivity.kt
@@ -24,6 +24,7 @@ import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.ui.platform.LocalLifecycleOwner
 import androidx.lifecycle.Lifecycle
@@ -89,23 +90,16 @@ open class BrowseActivity : ComponentActivity() {
                        arguments = spp.parameter,
                    ) { navBackStackEntry ->
                        val lifecycleOwner = LocalLifecycleOwner.current
-                        val spaLogger = spaEnvironment.logger
-                        val sp = spp.createSettingsPage(arguments = navBackStackEntry.arguments)
+                        val sp = remember(navBackStackEntry.arguments) {
+                            spp.createSettingsPage(arguments = navBackStackEntry.arguments)
+                        }

                        DisposableEffect(lifecycleOwner) {
                            val observer = LifecycleEventObserver { _, event ->
                                if (event == Lifecycle.Event.ON_START) {
-                                    spaLogger.event(
-                                        sp.id,
-                                        "enter page ${sp.formatDisplayTitle()}",
-                                        category = LogCategory.FRAMEWORK
-                                    )
+                                    sp.enterPage()
                                } else if (event == Lifecycle.Event.ON_STOP) {
-                                    spaLogger.event(
-                                        sp.id,
-                                        "leave page ${sp.formatDisplayTitle()}",
-                                        category = LogCategory.FRAMEWORK
-                                    )
+                                    sp.leavePage()
                                }
                            }

--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsEntry.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsEntry.kt
@@ -32,14 +32,16 @@ const val INJECT_ENTRY_NAME = "INJECT"
 const val ROOT_ENTRY_NAME = "ROOT"

 interface EntryData {
-    val pageId: String
-    val entryId: String
+    val pageId: String?
+        get() = null
+    val entryId: String?
+        get() = null
    val isHighlighted: Boolean
        get() = false
 }

 val LocalEntryDataProvider =
-    compositionLocalOf<EntryData> { error("LocalEntryDataProvider: No Default Value!") }
+    compositionLocalOf<EntryData> { object : EntryData{} }

 /**
 * Defines data of a Settings entry.
--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPage.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SettingsPage.kt
@@ -93,6 +93,24 @@ data class SettingsPage(
        }
        return false
    }
+
+    fun enterPage() {
+        SpaEnvironmentFactory.instance.logger.event(
+            id,
+            LogEvent.PAGE_ENTER,
+            category = LogCategory.FRAMEWORK,
+            details = formatDisplayTitle()
+        )
+    }
+
+    fun leavePage() {
+        SpaEnvironmentFactory.instance.logger.event(
+            id,
+            LogEvent.PAGE_LEAVE,
+            category = LogCategory.FRAMEWORK,
+            details = formatDisplayTitle()
+        )
+    }
 }

 fun String.toHashId(): String {
--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SpaLogger.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/common/SpaLogger.kt
@@ -30,6 +30,18 @@ enum class LogCategory {
    VIEW,
 }

+// Defines the log events in Spa.
+enum class LogEvent {
+    // Page related events.
+    PAGE_ENTER,
+    PAGE_LEAVE,
+
+    // Entry related events.
+    ENTRY_CLICK,
+    ENTRY_SWITCH_ON,
+    ENTRY_SWITCH_OFF,
+}
+
 /**
 * The interface of logger in Spa
 */
@@ -38,7 +50,13 @@ interface SpaLogger {
    fun message(tag: String, msg: String, category: LogCategory = LogCategory.DEFAULT) {}

    // log a user event.
-    fun event(id: String, event: String, category: LogCategory = LogCategory.DEFAULT) {}
+    fun event(
+        id: String,
+        event: LogEvent,
+        category: LogCategory = LogCategory.DEFAULT,
+        details: String? = null
+    ) {
+    }
 }

 class LocalLogger : SpaLogger {
@@ -46,7 +64,8 @@ class LocalLogger : SpaLogger {
        Log.d("SpaMsg-$category", "[$tag] $msg")
    }

-    override fun event(id: String, event: String, category: LogCategory) {
-        Log.d("SpaEvent-$category", "[$id] $event")
+    override fun event(id: String, event: LogEvent, category: LogCategory, details: String?) {
+        val extraMsg = if (details == null) "" else " ($details)"
+        Log.d("SpaEvent-$category", "[$id] $event$extraMsg")
    }
 }
--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/WidgetLogger.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/WidgetLogger.kt
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settingslib.spa.framework.util
+
+import androidx.compose.runtime.Composable
+import com.android.settingslib.spa.framework.common.LocalEntryDataProvider
+import com.android.settingslib.spa.framework.common.LogCategory
+import com.android.settingslib.spa.framework.common.LogEvent
+import com.android.settingslib.spa.framework.common.SpaEnvironmentFactory
+
+@Composable
+fun LogEntryEvent(): (event: LogEvent) -> Unit {
+    val entryId = LocalEntryDataProvider.current.entryId ?: return {}
+    return {
+        SpaEnvironmentFactory.instance.logger.event(entryId, it, category = LogCategory.VIEW)
+    }
+}
+
+@Composable
+fun WrapOnClickWithLog(onClick: (() -> Unit)?): (() -> Unit)? {
+    if (onClick == null) return null
+    val logEvent = LogEntryEvent()
+    return {
+        logEvent(LogEvent.ENTRY_CLICK)
+        onClick()
+    }
+}
+
+@Composable
+fun WrapOnSwitchWithLog(onSwitch: ((checked: Boolean) -> Unit)?): ((checked: Boolean) -> Unit)? {
+    if (onSwitch == null) return null
+    val logEvent = LogEntryEvent()
+    return {
+        val event = if (it) LogEvent.ENTRY_SWITCH_ON else LogEvent.ENTRY_SWITCH_OFF
+        logEvent(event)
+        onSwitch(it)
+    }
+}
--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/widget/preference/Preference.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/widget/preference/Preference.kt
@@ -26,6 +26,7 @@ import com.android.settingslib.spa.framework.common.EntryMacro
 import com.android.settingslib.spa.framework.common.EntrySearchData
 import com.android.settingslib.spa.framework.compose.navigator
 import com.android.settingslib.spa.framework.compose.stateOf
+import com.android.settingslib.spa.framework.util.WrapOnClickWithLog
 import com.android.settingslib.spa.widget.ui.createSettingsIcon

 data class SimplePreferenceMacro(
@@ -105,10 +106,14 @@ fun Preference(
    model: PreferenceModel,
    singleLineSummary: Boolean = false,
 ) {
-    val modifier = remember(model.enabled.value, model.onClick) {
-        model.onClick?.let { onClick ->
-            Modifier.clickable(enabled = model.enabled.value, onClick = onClick)
-        } ?: Modifier
+    val onClickWithLog = WrapOnClickWithLog(model.onClick)
+    val modifier = remember(model.enabled.value) {
+        if (onClickWithLog != null) {
+            Modifier.clickable(
+                enabled = model.enabled.value,
+                onClick = onClickWithLog
+            )
+        } else Modifier
    }
    BasePreference(
        title = model.title,
--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/widget/preference/SwitchPreference.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/widget/preference/SwitchPreference.kt
@@ -31,6 +31,7 @@ import com.android.settingslib.spa.framework.compose.stateOf
 import com.android.settingslib.spa.framework.compose.toState
 import com.android.settingslib.spa.framework.theme.SettingsDimension
 import com.android.settingslib.spa.framework.theme.SettingsTheme
+import com.android.settingslib.spa.framework.util.WrapOnSwitchWithLog
 import com.android.settingslib.spa.widget.ui.SettingsSwitch

 /**
@@ -100,15 +101,16 @@ internal fun InternalSwitchPreference(
 ) {
    val checkedValue = checked.value
    val indication = LocalIndication.current
+    val onChangeWithLog = WrapOnSwitchWithLog(onCheckedChange)
    val modifier = remember(checkedValue, changeable.value) {
-        if (checkedValue != null && onCheckedChange != null) {
+        if (checkedValue != null && onChangeWithLog != null) {
            Modifier.toggleable(
                value = checkedValue,
                interactionSource = MutableInteractionSource(),
                indication = indication,
                enabled = changeable.value,
                role = Role.Switch,
-                onValueChange = onCheckedChange,
+                onValueChange = onChangeWithLog,
            )
        } else Modifier
    }
@@ -121,7 +123,13 @@ internal fun InternalSwitchPreference(
        paddingEnd = paddingEnd,
        paddingVertical = paddingVertical,
    ) {
-        SettingsSwitch(checked = checked, changeable = changeable)
+        SettingsSwitch(
+            checked = checked,
+            changeable = changeable,
+            // The onCheckedChange is handled on the whole SwitchPreference.
+            // DO NOT set it on SettingsSwitch.
+            onCheckedChange = null,
+        )
    }
 }

--- a/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/widget/ui/Switch.kt
+++ b/packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/widget/ui/Switch.kt
@@ -20,6 +20,7 @@ import androidx.compose.material3.Checkbox
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.State
+import com.android.settingslib.spa.framework.util.WrapOnSwitchWithLog

@OptIn(ExperimentalMaterial3Api::class)
@Composable
@@ -34,7 +35,7 @@ fun SettingsSwitch(
    if (checkedValue != null) {
        Checkbox(
            checked = checkedValue,
-            onCheckedChange = onCheckedChange,
+            onCheckedChange = WrapOnSwitchWithLog(onCheckedChange),
            enabled = changeable.value,
        )
    } else {