From 5ecb15979d2097d85c261d1f192469c9ab5011ee Mon Sep 17 00:00:00 2001 From: Zekan Qian Date: Mon, 17 Oct 2022 15:03:34 +0800 Subject: [PATCH] Add log on preference widgets. Define enums of User events, and add log on all preferences. Allow toggle on the Switch component in two targets preferences. Bug: 253979024 Test: manual - build Gallery Change-Id: Ie5543fb5b42722d8c0fd8fd03532d8af62e514c4 --- .../spa/framework/BrowseActivity.kt | 18 +++---- .../spa/framework/common/SettingsEntry.kt | 8 +-- .../spa/framework/common/SettingsPage.kt | 18 +++++++ .../spa/framework/common/SpaLogger.kt | 25 +++++++-- .../spa/framework/util/WidgetLogger.kt | 52 +++++++++++++++++++ .../spa/widget/preference/Preference.kt | 13 +++-- .../spa/widget/preference/SwitchPreference.kt | 14 +++-- .../settingslib/spa/widget/ui/Switch.kt | 3 +- 8 files changed, 125 insertions(+), 26 deletions(-) create mode 100644 packages/SettingsLib/Spa/spa/src/com/android/settingslib/spa/framework/util/WidgetLogger.kt diff --git 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 index 476dd30b499b0..89daeb1ee0e6a 100644 --- 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() } } diff --git 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 index ca75b77dabc23..a3aeda6f086cd 100644 --- 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 { error("LocalEntryDataProvider: No Default Value!") } + compositionLocalOf { object : EntryData{} } /** * Defines data of a Settings entry. diff --git 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 index e7d89066b357d..8f63c47b1a9bc 100644 --- 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 { diff --git 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 index 5efedecae8d73..00a0362abd917 100644 --- 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") } } \ No newline at end of file diff --git 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 new file mode 100644 index 0000000000000..6c7432eee16ab --- /dev/null +++ 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) + } +} diff --git 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 index b900b6413d0ab..d1021e2783b8d 100644 --- 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, diff --git 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 index b6d6936442e76..992ce9e6c8724 100644 --- 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, + ) } } diff --git 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 index 45d5f6baa9cb2..82ab0be550021 100644 --- 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 {