Feat Weather widget

[lowrt]

這是什麼類型的 PR？

選擇所有符合的項目

• 重構
• 新功能
• 錯誤修復
• 最佳化
• 技術文件更新

描述

新增天氣小工具

相關 issue

• 相關問題 #
• closes #

QA 指南、截圖、錄像

請將這行替換成：如何測試您的 PR 的步驟，已測試的裝置註釋，以及任何相關的 UI 更改圖片。

UI 無障礙清單

如果您的 PR 包含 UI 更改，請使用此清單：

• 變數名稱實現語意化命名？
• 測試通過 AA 顏色對比？

[lowrt]

@claude review

[claude]

Claude finished @lowrt's task —— View job

---

Code Review: Weather Widget Feature

I've completed a comprehensive review of this weather widget PR. Overall, this is a well-structured implementation with both Android and iOS support. Below are my findings organized by severity.

---

🔴 Critical Issues

1. Security: Hardcoded Weather Icons Using System Resources

Location: WeatherWidgetProvider.kt:126-136

fun getWeatherIcon(code: Int): Int {
    return when (code) {
        1 -> android.R.drawable.ic_menu_day  // 晴天
        2, 3 -> android.R.drawable.ic_partial_secure  // 多雲
        // ... using system icons
    }
}

Issue: Using android.R.drawable icons (system resources) is unreliable and inappropriate:

• These icons are not designed for weather and may not visually represent weather conditions
• System resources can vary between Android versions and manufacturers
• Icons like ic_partial_secure (a lock icon) and ic_dialog_alert don't match weather semantics
• The comment at line 135 acknowledges this is temporary

Recommendation: Replace with custom drawable resources in res/drawable/ or use vector drawables.

---

2. Error Handling: Silent Failures in Background Tasks

Location: widget_service.dart:195-206, widget_background.dart:32-34

static Future<void> _forceAndroidWidgetUpdate() async {
  try {
    const platform = MethodChannel('com.exptech.dpip/widget');
    await platform.invokeMethod('updateWidgets');
  } catch (e) {
    // Falls back silently without user feedback
    talker.debug('[WidgetService] 方法通道不可用，使用備用更新方式: $e');
  }
}

Issue: Background task failures return false but there's no mechanism to:

• Notify users of persistent failures
• Retry with exponential backoff beyond the WorkManager default
• Track failure rates for debugging

Recommendation: Implement failure tracking and surface critical errors to users when appropriate.

---

🟡 High Priority Issues

3. Data Race Condition: Timestamp Conversion Inconsistency

Location: WidgetDataExtensions.kt:33-36 vs WeatherWidget.swift:106

Kotlin:

fun SharedPreferences.readTimestampMillis(key: String): Long? {
    val value = readNumber(key)?.toLong() ?: return null
    return if (value < 1_000_000_000_000L) value * 1000L else value
}

Swift:

let updateRaw = defaults.numberValue(forKey: "update_time") ?? 0
let updateSeconds = updateRaw >= 1_000_000_000_000 ? updateRaw / 1000 : updateRaw

Issue: The threshold logic is identical, but:

• Kotlin converts to milliseconds (multiplies by 1000)
• Swift converts to seconds (divides by 1000)
• Both work with the same stored value but interpret it differently
• This creates platform-dependent behavior that could cause confusion

Recommendation: Standardize on one timestamp format (preferably milliseconds) across all platforms and document it clearly.

---

4. Performance: No Caching or Deduplication

Location: widget_service.dart:22-84, home/page.dart:143-149

await Future.wait([
  _fetchWeather(code),
  _fetchRealtimeRegion(code),
  _fetchHistory(code, isOutOfService),
  WidgetService.updateWidget(),  // Makes another weather API call
]);

Issue: The widget service makes independent API calls without:

• Checking if fresh data already exists in memory
• Deduplicating concurrent requests
• Rate limiting API calls

Recommendation: Implement a caching layer with TTL or share data between the app and widget to reduce API calls.

---

5. Resource Management: Potential Memory Leak

Location: widget_background.dart:12-35

@pragma('vm:entry-point')
void callbackDispatcher() {
  Workmanager().executeTask((task, inputData) async {
    // ...
    await Global.init();  // Initializes global state
    await WidgetService.updateWidget();
    // No cleanup of Global.init() resources
  });
}

Issue: Global.init() is called in background tasks but there's no corresponding cleanup. If this initializes resources (HTTP clients, streams, etc.), they may leak.

Recommendation: Ensure proper cleanup after background task completion or make Global.init() idempotent with shared resources.

---

🟢 Medium Priority Issues

6. Code Duplication: Widget Update Logic

Locations: WeatherWidgetProvider.kt and WeatherWidgetSmallProvider.kt

Both providers duplicate 95% of the same logic with minor layout differences.

Recommendation: Extract common logic into a shared helper class or use composition to reduce duplication.

---

7. Hardcoded Magic Numbers

Location: Multiple files

// widget_background.dart:76
final frequency = frequencyMinutes < 15 ? 15 : frequencyMinutes;

// WeatherWidget.swift:64
let nextUpdate = Calendar.current.date(byAdding: .minute, value: 15, to: currentDate)!

Recommendation: Define constants like MIN_UPDATE_INTERVAL_MINUTES = 15 with documentation explaining the Android WorkManager limitation.

---

8. Missing Null Safety

Location: WeatherWidget.swift:337

formatter.timeZone = TimeZone.current  // Could be nil in rare cases

Recommendation: Add nil-coalescing with fallback to UTC.

---

9. Accessibility: Missing Content Descriptions

Location: weather_widget.xml:26

<ImageView
    android:id="@+id/weather_icon"
    android:contentDescription="天氣圖示"  

Recommendation: Set dynamic content descriptions based on actual weather conditions (e.g., "晴天" instead of generic "天氣圖示").

---

10. Error Messages Not Localized

Location: widget_service.dart:39, 48, 52

await _saveErrorState('位置未設定');
await _saveErrorState('無法獲取天氣');

Issue: Error messages are hardcoded in Traditional Chinese without using the i18n system.

Recommendation: Use i18n for all user-facing strings to support multiple languages.

---

🔵 Low Priority / Style Issues

11. Inconsistent Comment Languages

Mixed Traditional Chinese and English comments throughout. Consider standardizing to one language for maintainability.

---

12. Dead Code in iOS Widget

Location: WeatherWidgetControl.swift, WeatherWidgetLiveActivity.swift

These files appear to be Xcode template boilerplate and aren't used. Consider removing if not needed.

---

13. Missing Documentation

• No README or documentation for widget setup
• No comments explaining the SharedPreferences data contract between platforms
• Missing JSDoc/KDoc for public APIs

---

✅ Positive Observations

1. Good separation of concerns between background tasks, widget service, and UI
2. Proper use of MethodChannels for platform communication
3. Comprehensive error states with user-friendly error messages
4. Responsive layouts with different widget sizes (small and medium)
5. Background update mechanism properly implemented for both platforms
6. Proper use of App Groups for iOS widget data sharing

---

📋 Testing Recommendations

1. Test background updates when the app is fully closed
2. Verify widget behavior during network failures
3. Test data synchronization between app and widget
4. Validate memory usage during periodic background updates
5. Test on various Android manufacturers (Samsung, Xiaomi, etc.) due to aggressive battery optimization
6. Verify accessibility with TalkBack (Android) and VoiceOver (iOS)

---

📊 Summary

Category	Count
Critical Issues	2
High Priority	3
Medium Priority	5
Low Priority	3

Overall Assessment: This is a solid implementation with good architecture. The critical issues around hardcoded icons and error handling should be addressed before merge. The data race condition with timestamps needs investigation to ensure consistent behavior across platforms.

Recommendation: Address critical and high-priority issues, then this PR will be ready to merge. The medium and low priority items can be addressed in follow-up PRs if desired.

---