feat: alarm widget + global font size#18
Merged
Conversation
applicationId and namespace: com.wassupluke.simpleweather → com.wassupluke.wasseswidgets App name: Simple Weather → Wasses Widgets
- DEFAULT_TEMP_UNIT: C → F - "Widget tap action" → "Weather widget tap action" - "Alarm widget" section header → "Alarm widget tap action" - Remove unused title_alarm_widget_tap_action string - Update alarm_widget_description and label_widget_tap_none for new app name
AlarmWidgetReceiver now calls updateAlarmText on APPWIDGET_UPDATE so the widget shows the correct next alarm immediately when first placed, rather than waiting for the next alarm-change broadcast.
… picker labels - All strings.xml keys follow settings_*, widget_temperature_*, widget_alarm_* prefixes - Add widget_temperature_label / widget_alarm_label shown in widget picker - Add android:label to both receivers so picker shows "Temperature" / "Next Alarm" - Update widget_info XMLs and all R.string references in SettingsScreen.kt - Add docs/adding-a-widget.md with conventions for future widgets
GlanceAppWidgetReceiver.super.onReceive() already calls goAsync() for APPWIDGET_UPDATE, leaving mPendingResult null. A second goAsync() call returns null and crashes in the finally block. Skip goAsync() for APPWIDGET_UPDATE; use it only for standalone alarm broadcasts.
ColorProvider(Color) is restricted to the Glance library group. Pass the @ColorInt from parseColorSafe() directly to ColorProvider(Int) instead of converting to Compose Color first.
Reviewer's GuideImplements a new Glance-based AlarmWidget with its own receiver and tap-action settings, introduces a global widget font-size setting shared between WeatherWidget and AlarmWidget, renames the app/package to com.wassupluke.wasseswidgets with updated strings and theme tweaks, and fixes Glance ColorProvider usage by switching to @ColorInt-based colors. Sequence diagram for global font size updatesequenceDiagram
actor User
participant SettingsScreen
participant SettingsViewModel
participant PreferencesDataStore
participant WeatherWidget
participant AlarmWidget
User->>SettingsScreen: Drag_font_size_Slider(newSize)
SettingsScreen->>SettingsViewModel: setFontSize(newSize)
SettingsViewModel->>PreferencesDataStore: write FONT_SIZE=newSize
PreferencesDataStore-->>SettingsViewModel: write_complete
SettingsViewModel->>WeatherWidget: updateAll(context)
SettingsViewModel->>AlarmWidget: updateAll(context)
WeatherWidget->>PreferencesDataStore: collect FONT_SIZE
AlarmWidget->>PreferencesDataStore: collect FONT_SIZE
PreferencesDataStore-->>WeatherWidget: FONT_SIZE=newSize
PreferencesDataStore-->>AlarmWidget: FONT_SIZE=newSize
WeatherWidget-->>User: Re-render_temperature(fontSize=newSize)
AlarmWidget-->>User: Re-render_alarm_text(fontSize=newSize)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- AlarmWidget and AlarmWidgetReceiver hardcode the "No alarm" text in Kotlin rather than using a string resource (e.g., from
strings.xml), which makes localization and future text changes harder; consider introducing a dedicated string and reusing it in both places. - The font size slider writes to DataStore on every
onValueChangestep, which could cause unnecessary I/O while dragging; consider debouncing this by usingonValueChangeFinished(or local state) and only persisting when the user releases the slider.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- AlarmWidget and AlarmWidgetReceiver hardcode the "No alarm" text in Kotlin rather than using a string resource (e.g., from `strings.xml`), which makes localization and future text changes harder; consider introducing a dedicated string and reusing it in both places.
- The font size slider writes to DataStore on every `onValueChange` step, which could cause unnecessary I/O while dragging; consider debouncing this by using `onValueChangeFinished` (or local state) and only persisting when the user releases the slider.
## Individual Comments
### Comment 1
<location path="app/src/main/java/com/wassupluke/wasseswidgets/widget/AlarmWidget.kt" line_range="36-37" />
<code_context>
+ override suspend fun provideGlance(context: Context, id: GlanceId) {
+ provideContent {
+ GlanceTheme {
+ val prefs by context.dataStore.data.collectAsState(initial = emptyPreferences())
+ val alarmText = prefs[WeatherDataStore.ALARM_TEXT] ?: "No alarm"
+ val colorString = prefs[WeatherDataStore.WIDGET_TEXT_COLOR] ?: "white"
+ val dynamicColor = prefs.resolveDynamicColor()
</code_context>
<issue_to_address>
**suggestion:** Avoid hardcoded fallback text for `alarmText` and use a string resource instead.
Using a hardcoded "No alarm" string bypasses localization and makes future copy changes harder. Please define a `settings_alarm_none` (or similar) in string resources and use `context.getString(...)` as the default, ideally via a reusable helper that the receiver can also use when writing `ALARM_TEXT`.
Suggested implementation:
```
import com.wassupluke.wasseswidgets.R
import com.wassupluke.wasseswidgets.data.dataStore
import com.wassupluke.wasseswidgets.data.parseColorSafe
import com.wassupluke.wasseswidgets.data.resolveDynamicColor
import com.wassupluke.wasseswidgets.ui.MainActivity
```
```
val prefs by context.dataStore.data.collectAsState(initial = emptyPreferences())
val alarmText = prefs[WeatherDataStore.ALARM_TEXT]
?: context.getString(R.string.settings_alarm_none)
```
1. Define the `settings_alarm_none` string in `app/src/main/res/values/strings.xml`, for example:
`<string name="settings_alarm_none">No alarm</string>`, and provide translations as needed.
2. Introduce a reusable helper (e.g., in `WeatherDataStore` or a small `AlarmTextDefaults` util) that returns the default alarm text via `context.getString(R.string.settings_alarm_none)`.
3. Update any code that writes `WeatherDataStore.ALARM_TEXT` (receiver, settings screen, etc.) to use that helper when it needs the “no alarm” fallback to keep behavior consistent with the widget.
</issue_to_address>
### Comment 2
<location path="app/src/main/java/com/wassupluke/wasseswidgets/widget/AlarmWidgetReceiver.kt" line_range="39-41" />
<code_context>
+ val pendingResult = if (action != AppWidgetManager.ACTION_APPWIDGET_UPDATE) goAsync() else null
+ CoroutineScope(Dispatchers.IO).launch {
+ try {
+ val alarmManager = context.getSystemService(Context.ALARM_SERVICE) as AlarmManager
+ val nextAlarm = alarmManager.nextAlarmClock
+ val alarmText = if (nextAlarm == null) {
+ "No alarm"
+ } else {
</code_context>
<issue_to_address>
**suggestion:** Deduplicate alarm text generation and avoid hardcoded copy in the receiver.
This mapping from `nextAlarm` to a user-facing string is duplicated here and in `AlarmWidget`, both hardcoding "No alarm". That duplication can lead to inconsistent text if only one side is updated. Please extract a shared helper (e.g., in a utils file) that returns the localized string from `nextAlarm`, and use it both for display and when persisting `ALARM_TEXT`.
Suggested implementation:
```
val alarmManager = context.getSystemService(Context.ALARM_SERVICE) as AlarmManager
val nextAlarm = alarmManager.nextAlarmClock
val alarmText = formatAlarmText(context, nextAlarm)
```
```
}
}
private fun formatAlarmText(context: Context, nextAlarm: AlarmManager.AlarmClockInfo?): String {
return if (nextAlarm == null) {
// TODO: replace with localized string resource, e.g. context.getString(R.string.no_alarm)
"No alarm"
} else {
DateFormat.getTimeInstance(DateFormat.SHORT).format(Date(nextAlarm.triggerTime))
}
}
}
```
To fully apply your review comment across the codebase:
1. Update `AlarmWidget` (likely in `app/src/main/java/com/wassupluke/wasseswidgets/widget/AlarmWidget.kt`) to replace its own `nextAlarm` → string mapping with a call to `formatAlarmText(context, nextAlarm)` so both display and persisted `ALARM_TEXT` use the same logic.
2. Consider moving `formatAlarmText` into a shared utils file (e.g., `AlarmTextUtils.kt`) as a top-level `fun formatAlarmText(...)` in the same package, then adjust both `AlarmWidgetReceiver` and `AlarmWidget` to import and call that helper.
3. Replace the hardcoded `"No alarm"` in `formatAlarmText` with a localized string resource (e.g., `R.string.no_alarm`) once you have the appropriate entry in `strings.xml`.
</issue_to_address>
### Comment 3
<location path="app/src/main/java/com/wassupluke/wasseswidgets/widget/AlarmWidget.kt" line_range="79-84" />
<code_context>
+ }
+ }
+ if (icon != null) {
+ Image(
+ bitmap = icon!!,
+ contentDescription = null,
</code_context>
<issue_to_address>
**suggestion:** Icon size tied directly to font size may lead to layout issues at larger values.
At high slider values (e.g., 96), `fontSize.dp` can make the icon disproportionately large and cause clipping or awkward layouts. Consider sizing the icon independently from the text (for example, applying a smaller scale factor or clamping it to a max size) so large font sizes don’t blow up the icon.
```suggestion
Image(
provider = ImageProvider(R.drawable.ic_alarm),
contentDescription = null,
// Clamp icon size so very large font sizes don't blow up the icon
modifier = GlanceModifier.size(fontSize.coerceAtMost(32).dp),
colorFilter = ColorFilter.tint(textColorProvider)
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ColorProvider(Color) is restricted to the Glance library group with no public equivalent. Annotate both widget classes with @SuppressLint and use Color(argb) to correctly convert the @ColorInt before passing it in. The previous attempt used ColorProvider(Int) which takes a @ColorRes, not a @ColorInt, and would have produced wrong colors at runtime.
- Replace hardcoded "No alarm" in AlarmWidget and AlarmWidgetReceiver with R.string.widget_alarm_none for localization consistency - Clamp alarm icon size to 32dp max so large font values don't blow up the icon disproportionately
Update applicationId, namespace, all package declarations, imports, directory structure, and display name from "Wasses Widgets" to "Widgets".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
simpleweather→wasseswidgetsColorProvider(Color)@RestrictTolint violation in both widgets — now usesColorProvider(Int)with the raw@ColorIntdirectlyTest Plan
./gradlew :app:testDebugUnitTest— all 24 tests passSummary by Sourcery
Introduce a next-alarm home screen widget, add a global widget font size setting shared across widgets, and rename the app/package to Wasses Widgets.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: