Feat typhoon page

[lowrt]

這是什麼類型的 PR？

選擇所有符合的項目

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

描述

新增颱風圖層頁面

相關 issue

• 相關問題 #
• closes #

QA 指南、截圖、錄像

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

UI 無障礙清單

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

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

[claude]

Claude finished @lowrt's task —— View job

---

Code Review: Feat typhoon page

I've completed a thorough review of this PR. Here's my analysis:

---

Summary

This PR adds a typhoon layer visualization feature to the map, allowing users to view typhoon paths, forecast data, and wind circles. The implementation follows the existing pattern for weather layers (radar, temperature, etc.) and integrates well with the codebase.

---

✅ Strengths

1. Consistent Architecture: Follows the existing MapLayerManager pattern used by other weather layers
2. Proper Error Handling: Uses try-catch blocks with TalkerManager for logging
3. State Management: Proper use of ValueNotifier and Provider pattern
4. Integration: Well-integrated with existing layer toggle system and time synchronization

---

🔍 Issues Found

Critical Issues

1. Incomplete Implementation in typhoon.dart manager (lib/app/map/_lib/managers/typhoon.dart)

The manager has several critical bugs:

• Line 74: Sets typhoonList but this variable is never populated from the API response

  GlobalProviders.data.setTyphoon(typhoonList);
  currentTyphoonTime.value ??= typhoonList.first;

  This will crash with "Bad state: No element" if typhoonList is empty.

• Lines 91-98: Uses hardcoded source ID 'typhoon-geojson' instead of dynamic ID based on time

  const sourceId = 'typhoon-geojson';

  This conflicts with the MapSourceIds.typhoon(time) pattern used in remove() method (line 214).

• Line 201: _fetchData() is called but never actually updates the map layers with new data

2. Missing Data Parsing (lib/app/map/_lib/managers/typhoon.dart:68-79)

The _fetchData() method fetches GeoJSON but doesn't:

• Parse typhoon IDs/names from the response
• Update typhoonList with available timestamps
• Store typhoon data in the global provider

3. Unused Variables (lib/app/map/_lib/managers/typhoon.dart:24-30)

Map<String, dynamic> typhoonData = {};
List<String> typhoonList = [];
int selectedTyphoonId = -1;
List<String> sourceList = [];
List<String> layerList = [];
List<String> typhoon_name_list = [];
List<int> typhoon_id_list = [];

Most of these instance variables are declared but never used. The implementation uses GlobalProviders.data.typhoon instead.

High Priority Issues

4. Missing typhoon.g.dart Generated File (lib/api/model/weather/typhoon.dart:4)

The model uses @JsonSerializable() but the generated file isn't included:

part 'typhoon.g.dart';

You need to run: dart run build_runner build

5. Inconsistent Layer Visibility Logic (lib/app/map/_lib/managers/typhoon.dart:168-183)

The hide() and show() methods reference MapLayerIds.typhoon(time) but the setup() method creates layers with hardcoded IDs like 'typhoon-path', 'typhoon-points', etc.

6. Empty UI Sheet (lib/app/map/_lib/managers/typhoon.dart:230-272)

The TyphoonMapLayerSheet widget builds but displays almost nothing - just an icon and title. There's no time selector, typhoon info, or interactive controls like other weather layers have.

Medium Priority Issues

7. Naming Convention Violation (lib/app/map/_lib/managers/typhoon.dart:29-30)

List<String> typhoon_name_list = [];
List<int> typhoon_id_list = [];

Dart uses camelCase, not snake_case. Should be typhoonNameList and typhoonIdList.

8. Magic Numbers Without Documentation (lib/app/map/_lib/managers/typhoon.dart:105-111)

Color codes lack comments explaining what they represent:

0, '#1565C0', // 藍色
1, '#4CAF50', // 綠色
2, '#FFC107', // 黃色
3, '#FF5722', // 橙色

What do types 0-3 mean? (Tropical depression, tropical storm, severe tropical storm, typhoon?)

9. Potential Null Reference (lib/app/map/_lib/managers/typhoon.dart:75)

currentTyphoonTime.value ??= typhoonList.first;

If typhoonList is empty, .first will throw. Should check isEmpty first.

10. Unused Import (lib/api/model/weather/typhoon.dart:1)

import 'package:dpip/utils/geojson.dart';

The Location class doesn't use anything from this import.

---

🎯 Recommendations

Must Fix Before Merge:

1. Parse typhoon data from API response in _fetchData():

   • Extract typhoon IDs and timestamps from GeoJSON
   • Populate typhoonList correctly
   • Handle empty response gracefully

2. Fix source/layer ID inconsistency:

   • Either use dynamic IDs everywhere or static IDs everywhere
   • Ensure setup(), show(), hide(), and remove() all reference the same layer IDs

3. Generate missing files:

   dart run build_runner build

4. Implement the UI sheet or remove it:

   • Add time selection controls like other weather layers
   • Display typhoon information
   • Or simplify if not needed for MVP

5. Remove unused variables to reduce confusion

Should Fix:

6. Add null safety checks before accessing .first on potentially empty lists
7. Fix snake_case variable names to camelCase
8. Add comments explaining color coding system
9. Remove unused imports

Nice to Have:

10. Add unit tests for the new Typhoon model
11. Add integration tests for the layer manager
12. Document the expected GeoJSON structure
13. Add error states in the UI for when no typhoon data is available

---

🔒 Security & Performance

• ✅ No security concerns found
• ✅ Proper error handling in API calls
• ⚠️ Consider caching strategy - currently refetches every 5 minutes which is good
• ✅ Async operations properly handled with try-catch

---

📝 Test Coverage

• ❌ No tests included for new typhoon model
• ❌ No tests for typhoon manager
• ❌ No integration tests for map layer interaction

Recommendation: Add at least basic unit tests for the Typhoon and Location models.

---

🎨 Code Quality

• ✅ Generally follows Flutter/Dart best practices
• ✅ Consistent with existing codebase patterns
• ⚠️ Some naming convention violations (snake_case)
• ⚠️ Incomplete implementation (hardcoded values, unused variables)

---

Summary

This PR has a good foundation but requires significant fixes before merging. The main issues are:

1. Incomplete data parsing from API
2. Layer ID inconsistencies causing hide/show to fail
3. Missing generated files
4. Empty UI implementation

Once these critical issues are addressed, this will be a solid addition to the weather layer system.

Status: ⚠️ Changes Requested - Critical bugs need to be fixed before merge

---