Add lightning page

[lowrt]

這是什麼類型的 PR？

選擇所有符合的項目

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

描述

新增閃電頁面

相關 issue

• 相關問題 #
• closes #

QA 指南、截圖、錄像

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

UI 無障礙清單

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

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

[claude]

Claude finished @lowrt's task —— View job

---

Code Review Complete ✅

I've completed a thorough review of the lightning page implementation. Here's my detailed feedback:

---

✅ Positive Aspects

1. Consistent Architecture: The implementation follows the established pattern used in other weather managers (temperature, precipitation, wind), ensuring maintainability and consistency
2. Proper State Management: Uses ValueNotifier appropriately for reactive UI updates
3. Good Data Caching: Leverages GlobalProviders.data.lightningData to cache fetched data and avoid redundant API calls
4. Time-based Categorization: The time-based intensity levels (5, 10, 30, 60 minutes) in Lightning.toFeatureBuilder() provide good visual context
5. Integration: Properly integrated into the layer toggle system and map page structure

---

⚠️ Issues & Improvements Needed

1. Bug in Legend Icon Matching (High Priority) 🐛

Location: lib/app/map/_lib/managers/lightning.dart:197

There's a mismatch between the default case in the layer properties and the legend. The layer definition has:

'0-60',
'lightning-0-60',

But the legend shows the icon as:

const OutlinedIcon(Symbols.navigation_rounded, fill: Color(0xffffff00), size: 20),

Problem: The default case 'lightning-0-60' is the only icon specified, but there's no explicit default in the match expression. This could cause rendering issues if the type doesn't match.

Recommendation: Add an explicit default case:

iconImage: [
  Expressions.match,
  ['get', 'type'],
  '1-5', 'lightning-1-5',
  '1-10', 'lightning-1-10',
  '1-30', 'lightning-1-30',
  '1-60', 'lightning-1-60',
  '0-5', 'lightning-0-5',
  '0-10', 'lightning-0-10',
  '0-30', 'lightning-0-30',
  '0-60', 'lightning-0-60',
  'lightning-0-60', // explicit default
],

2. Inconsistent Error Handling

Location: lib/app/map/_lib/managers/lightning.dart:200

} catch (e, s) {
  TalkerManager.instance.error('LightningMapLayerManager.dispose', e, s);
}

The error message says dispose but this is in the remove() method. This will make debugging confusing.

Fix: Change to 'LightningMapLayerManager.remove'

3. Missing Focus Animation

Locations:

• lib/app/map/_lib/managers/lightning.dart:174-188 (show() method)
• Compare with lib/app/map/_lib/managers/temperature.dart:77-89

The temperature manager includes a _focus() method that animates the camera when showing the layer. The lightning manager is missing this feature, which provides a better UX by centering the view.

Recommendation: Add a _focus() method similar to the temperature manager.

4. Potential Null Safety Issue

Location: lib/app/map/_lib/managers/lightning.dart:162, 177, 193

The code uses currentLightningTime.value which could theoretically be null when calling methods like hide() if the data fetch fails. While the setup guards against this, it would be safer to add null checks in these methods.

Recommendation:

@override
Future<void> hide() async {
  if (!visible) return;
  
  final time = currentLightningTime.value;
  if (time == null) return; // Add this guard
  
  final layerId = MapLayerIds.lightning(time);
  // ...
}

5. Unused Class Definition

Location: lib/app/map/_lib/managers/lightning.dart:26-33

class LightningData {
  final double latitude;
  final double longitude;
  final int type;
  final int time;
  // ...
}

This class is defined but never used in the new implementation. The Lightning model from the API is used directly instead. This is dead code.

Recommendation: Remove this unused class definition.

6. Missing Sync with Lightning Time

Location: lib/app/map/page.dart:105-145

The page syncs temperature, precipitation, and wind times with the radar layer when they're combined. However, there's no similar sync logic for lightning. This means when lightning is shown alongside radar, they might display different timestamps.

Recommendation: Add lightning to the sync logic:

void _setupWeatherLayerTimeSync() {
  // ... existing code ...
  
  final lightningManager = getLayerManager<LightningMapLayerManager>(MapLayer.lightning);
  lightningManager?.onTimeChanged = (time) {
    syncTimeToRadar(time);
  };
}

And update _syncRadarTimeOnCombination() to handle MapLayer.lightning.

7. No Test Coverage 🧪

Location: N/A

There are no unit or widget tests for the lightning functionality. Given that this is a new feature with time-based calculations and map interactions, test coverage would help prevent regressions.

Recommendation: Add tests for:

• Time-based categorization logic in Lightning.toFeatureBuilder()
• State management in LightningMapLayerManager
• UI interactions in LightningMapLayerSheet

---

🎨 Code Quality & Best Practices

Positive:

• Proper use of const constructors
• Good separation of concerns (manager vs. sheet widget)
• Follows Flutter naming conventions
• Appropriate use of async/await
• Good error logging with context

Minor Suggestions:

1. Hard-coded strings: Consider extracting legend labels to i18n:

   // Current
   label: '5 分鐘內對地閃電',
   
   // Better
   label: 'lightning.legend.ground_5min'.i18n,

2. Magic numbers: The positioned legend coordinates are hard-coded:

   Positioned(
     top: 24 + 48 + 16,  // Consider named constants
     left: 24,
     // ...
   )

---

🔒 Security Considerations

No security issues identified. The implementation:

• ✅ Properly validates data before use
• ✅ Uses the existing API client with appropriate error handling
• ✅ No direct user input that could lead to injection attacks

---

⚡ Performance Considerations

1. Good: Data caching prevents redundant API calls
2. Good: Proper cleanup in remove() method
3. Concern: Large datasets - if there are thousands of lightning strikes, rendering all of them on the map could impact performance. Consider adding viewport-based filtering or clustering for dense data.

---

📝 Summary & Recommendations

Must Fix (High Priority):

1. Fix the icon matching default case to prevent rendering issues
2. Correct the error message in remove() method
3. Add null safety guards in hide(), show(), and remove()
4. Remove unused LightningData class

Should Fix (Medium Priority):
5. Add _focus() method for better UX
6. Implement time sync with radar layer
7. Add test coverage

Nice to Have (Low Priority):
8. Extract hard-coded strings to i18n
9. Use named constants for positioning

Overall, this is a solid implementation that follows the established patterns in the codebase. The main concerns are the potential bugs that should be addressed before merging. The architecture is sound and the code is maintainable.

---