feat: system widgets foundation + price widget#895
Conversation
jvsena42
changed the title
… MainActivity on finish
jvsena42
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review
Found several issues — see inline comments for details.
Additionally: CHANGELOG.md is missing a required entry. Per CLAUDE.md: ALWAYS add exactly ONE entry per PR under ## [Unreleased] in CHANGELOG.md for feat: and fix: PRs. This is a feat: PR with no CHANGELOG.md changes in the diff.
| <activity | ||
| android:name=".appwidget.config.AppWidgetConfigActivity" | ||
| android:exported="true" | ||
| android:excludeFromRecents="true" | ||
| android:launchMode="singleInstance" | ||
| android:screenOrientation="portrait" | ||
| android:taskAffinity="" | ||
| android:theme="@style/Theme.App"> |
There was a problem hiding this comment.
Bug: singleInstance launch mode silently breaks setResult() — first-time widget placement will always be cancelled
singleInstance (and singleTask) activities live in their own task and cannot return a result to the calling activity — setResult() is silently ignored. The Android launcher starts widget configure activities via startActivityForResult with APPWIDGET_CONFIGURE and relies on RESULT_OK + EXTRA_APPWIDGET_ID in the response to finalize widget placement. With singleInstance, the launcher receives RESULT_CANCELED immediately and cancels the placement.
android:taskAffinity="" compounds the issue by forcing the activity into a separate isolated task with no affinity back to the caller.
Fix: Remove android:launchMode="singleInstance" and android:taskAffinity="". The default launch mode (standard) is correct for configure activities — they are started per widget instance and must be able to return a result.
See AppWidgetConfigActivity.kt#L38-L50 for the setResult(RESULT_OK, ...) calls that will be silently dropped.
| when (type) { | ||
| AppWidgetType.PRICE -> dataRepository.fetchPriceData() | ||
| .onSuccess { | ||
| preferencesStore.cachePriceData(it) | ||
| PriceGlanceWidget().updateAll(appContext) | ||
| } | ||
| .onFailure { Logger.warn("Failed to refresh price", e = it, context = TAG) } |
There was a problem hiding this comment.
Bug: Worker ignores each widget's configured GraphPeriod — periodic refreshes overwrite the cache with the wrong period
dataRepository.fetchPriceData() (no-arg overload) delegates to priceService.fetchData() which reads the period from the in-app widget store — not from each home screen widget's HomePricePreferences.period. Every 15-minute background refresh overwrites the single global cachedPrice with the in-app period regardless of what any individual home screen widget user has configured.
The initial fetch in saveAndFinish correctly uses pricePreferences.period, but all subsequent periodic refreshes ignore it.
Fix: Iterate over active widget entries and fetch using entry.pricePreferences.period. Since cachedPrice is a single PriceDTO? shared across all entries, the cache structure also needs to support per-period or per-entry data to avoid conflicts.
CLAUDE.md violation (line 44): NEVER add e = named parameter to Logger calls
Per CLAUDE.md.
| when (type) { | |
| AppWidgetType.PRICE -> dataRepository.fetchPriceData() | |
| .onSuccess { | |
| preferencesStore.cachePriceData(it) | |
| PriceGlanceWidget().updateAll(appContext) | |
| } | |
| .onFailure { Logger.warn("Failed to refresh price", e = it, context = TAG) } | |
| .onFailure { Logger.warn("Failed to refresh price", it, context = TAG) } |
| companion object { | ||
| private const val TAG = "AppWidgetRefreshWorker" | ||
| private const val WORK_NAME = "appwidget_refresh" | ||
|
|
||
| fun enqueue(context: Context) { | ||
| val constraints = Constraints.Builder() | ||
| .setRequiredNetworkType(NetworkType.CONNECTED) | ||
| .build() | ||
|
|
||
| val request = PeriodicWorkRequestBuilder<AppWidgetRefreshWorker>( | ||
| repeatInterval = 15, | ||
| repeatIntervalTimeUnit = TimeUnit.MINUTES, | ||
| ).setConstraints(constraints).build() |
There was a problem hiding this comment.
CLAUDE.md violations: companion object placement + TimeUnit instead of Kotlin Duration
1. companion object placement (CLAUDE.md: ALWAYS position companion object at the top of the class)
The companion object is declared after doWork(). It must be the first member in the class body.
2. TimeUnit instead of Kotlin Duration (CLAUDE.md: ALWAYS prefer Kotlin Duration for timeouts and delays)
PeriodicWorkRequestBuilder has an overload accepting kotlin.time.Duration. Replace with:
val request = PeriodicWorkRequestBuilder<AppWidgetRefreshWorker>(15.minutes)
.setConstraints(constraints)
.build()Also swap import java.util.concurrent.TimeUnit for import kotlin.time.Duration.Companion.minutes.
| fun cancelIfNoWidgets(context: Context) { | ||
| val manager = AppWidgetManager.getInstance(context) | ||
| val hasAny = manager.getAppWidgetIds( | ||
| ComponentName(context, PriceGlanceReceiver::class.java), | ||
| ).isNotEmpty() | ||
| if (!hasAny) { | ||
| WorkManager.getInstance(context).cancelUniqueWork(WORK_NAME) | ||
| } | ||
| } |
There was a problem hiding this comment.
Foundation flaw: cancelIfNoWidgets only checks PriceGlanceReceiver — will cancel refresh for all widget types once a second type is added
This PR introduces the widgets foundation for future widget types, but the shared WORK_NAME = "appwidget_refresh" worker is cancelled solely based on whether PriceGlanceReceiver has any widgets. When a second widget type is added and a user removes all Price widgets, PriceGlanceReceiver.onDisabled will cancel the shared refresh worker even if widgets of other types are still active.
Fix: Check all registered widget types before cancelling — e.g. maintain an AppWidgetType → KClass<out GlanceAppWidgetReceiver> mapping and verify every type has zero active instances before calling cancelUniqueWork.
| override fun onDisabled(context: Context) { | ||
| super.onDisabled(context) | ||
| AppWidgetRefreshWorker.cancelIfNoWidgets(context) | ||
| } |
There was a problem hiding this comment.
Bug: Missing onDeleted override — deleted widget instances leave orphaned preferences
onDeleted(context, appWidgetIds) fires when individual widget instances are removed; onDisabled only fires when the last instance is removed. Without overriding onDeleted, AppWidgetPreferencesStore.unregisterWidget() is never called for per-instance deletions, so stale AppWidgetEntry rows accumulate in the DataStore indefinitely.
When Android later reuses the same appWidgetId for a freshly placed widget, registerWidget's early-return guard means the new widget silently inherits the deleted widget's pricePreferences. Note: unregisterWidget is defined but has no callers in this PR.
Fix: Override onDeleted and call unregisterWidget for each deleted ID. Since it is suspend, use goAsync() to extend the broadcast lifecycle:
override fun onDeleted(context: Context, appWidgetIds: IntArray) {
super.onDeleted(context, appWidgetIds)
val result = goAsync()
val store = AppWidgetPreferencesStore.getInstance(context)
CoroutineScope(Dispatchers.IO).launch {
appWidgetIds.forEach { store.unregisterWidget(it) }
result.finish()
}
}| override suspend fun readFrom(input: InputStream): AppWidgetData { | ||
| return try { | ||
| json.decodeFromString(input.readBytes().decodeToString()) | ||
| } catch (e: SerializationException) { | ||
| Logger.error("Failed to deserialize: $e") | ||
| defaultValue | ||
| } |
There was a problem hiding this comment.
CLAUDE.md violations: try-catch instead of runCatching, $e interpolated in log message, missing context = TAG
Three rules from CLAUDE.md are violated here:
ALWAYS use the Result API instead of try-catch— replace withrunCatching { }.getOrElse { }.NEVER manually append the Throwable's message—"Failed to deserialize: $e"interpolates the exception directly into the string. Pass the throwable positionally to Logger instead.ALWAYS pass the TAG as context to Logger calls—context = TAGis missing. Since this is anobject, addprivate const val TAG = "AppWidgetDataSerializer"at the top of the object body.
Suggested fix for readFrom:
override suspend fun readFrom(input: InputStream): AppWidgetData {
return runCatching {
json.decodeFromString(input.readBytes().decodeToString())
}.getOrElse {
Logger.error("Failed to deserialize AppWidgetData", it, context = TAG)
defaultValue
}
}| } | ||
| dataRepository.fetchPriceData(pricePreferences.period ?: GraphPeriod.ONE_DAY) | ||
| .onSuccess { preferencesStore.cachePriceData(it) } | ||
| .onFailure { Logger.warn("Failed to fetch initial price data", e = it, context = TAG) } |
There was a problem hiding this comment.
CLAUDE.md violation: NEVER add e = named parameter to Logger calls
Per CLAUDE.md.
| .onFailure { Logger.warn("Failed to fetch initial price data", e = it, context = TAG) } | |
| .onFailure { Logger.warn("Failed to fetch initial price data", it, context = TAG) } |
1 participant
This PR:
Description
Introduces the foundation for Android home screen widgets (AppWidgets) using Jetpack Glance, starting with the Price widget. The widget displays enabled trading pairs with price, change percentage, and a line chart — matching the in-app Price widget layout.
The architecture reuses the existing
PriceServicefor data fetching (via a newfetchData(period)overload) while keeping widget preferences completely independent from the in-app widget system through a dedicatedAppWidgetPreferencesStorebacked by its own DataStore file.A Glance design system (
GlanceTextStyles,GlanceColors) ports the app's typography and color tokens to Glance equivalents, and shared components (GlanceWidgetScaffold,GlanceDataRow) provide consistent widget layouts.Preview
Screen_recording_20260421_153032.webm
QA Notes