Feat replay And fix show latest report

[bamboo0403]

這是什麼類型的 PR？

選擇所有符合的項目

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

描述

• 新增重播
• 修復在第一次加載報告時，無法正確顯示的問題

相關 issue

• 相關問題 #
• closes [Bug]: 地震報告框被切掉 #361

QA 指南、截圖、錄像

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

UI 無障礙清單

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

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

[bamboo0403]

@claude review

[claude]

Claude finished @bamboo0403's task in 1m 6s —— View job

---

Code Review Complete

I've reviewed the changes in this PR. Overall, the implementation looks good and adds the replay functionality while fixing the report display issue. Here are my findings:

---

Summary

This PR successfully implements:

1. Replay feature: Allows users to replay earthquake events from report pages
2. Report display fix: Fixes the issue where reports weren't showing correctly on first load by adding a reactive data notifier

---

Findings

✅ Positive Changes

1. Well-structured replay implementation - The MapMonitorPage widget is clean and follows Flutter best practices
2. Proper data reactivity - Adding dataNotifier (report.dart:47) ensures the UI updates when report data changes
3. Consistent parameter naming - Changed from data to replayTimestamp for better clarity
4. Internationalization - Properly using .i18n for the replay button label in report.dart:627

⚠️ Issues to Address

1. Missing newline at end of file (report.dart:844)

   • The file ends without a newline character
   • Most style guides require files to end with a newline
   • Location: lib/app/map/_lib/managers/report.dart:844

2. Inconsistent i18n usage (report_sheet_content.dart:68)

   • In report_sheet_content.dart, the replay label uses const Text('重播') (hardcoded)
   • In report.dart:627, it correctly uses Text('重播'.i18n)
   • These should be consistent for proper internationalization
   • Location: lib/route/report/report_sheet_content.dart:68

3. Potential null safety issue (page.dart:32)

   • Using int.tryParse(queryParameters['replay'] ?? '') will always return null when the parameter is missing
   • The empty string '' will cause tryParse to return null, which is correct behavior
   • However, it's more idiomatic to write: int.tryParse(queryParameters['replay'] ?? '0') or just queryParameters['replay'] directly (since tryParse handles null)
   • Location: lib/app/map/page.dart:32

4. Indentation inconsistency (report.dart:346-508)

   • The nested ValueListenableBuilder has unusual indentation after wrapping with the outer ValueListenableBuilder
   • The inner builder's children maintain their original indentation but appear misaligned in context
   • Consider reformatting this section for better readability
   • Locations: lib/app/map/_lib/managers/report.dart:346-508

5. Magic number (both files using replay)

   • The -5000 (5 seconds before earthquake) is a magic number used in multiple places
   • Consider extracting to a named constant like const kReplayLeadTimeMs = 5000;
   • Locations: report.dart:632, report_sheet_content.dart:73

💡 Suggestions

1. Consider adding documentation

   • The replayTimestamp parameter could benefit from a documentation comment explaining it represents the replay start time in milliseconds since epoch

2. Data notifier naming

   • dataNotifier is generic; consider reportDataUpdateNotifier or reportListUpdateNotifier for clarity

3. Error handling

   • The replay navigation doesn't have try-catch blocks. Consider wrapping in error handling in case the MapMonitorPage fails to load

---

Testing Recommendations

Please ensure the following scenarios are tested:

• First-time loading of reports displays correctly
• Replay button works from both report views (report.dart and report_sheet_content.dart)
• Replay starts 5 seconds before the earthquake time
• URL query parameters with replay parameter work correctly
• Switching between normal map mode and replay mode works smoothly
• Internationalization works for all languages supported by the app

---

Verdict

The code is functional and the implementation is solid. The issues found are mostly minor style/consistency problems. I recommend:

1. Must fix: Add newline at end of file (report.dart)
2. Should fix: Make i18n usage consistent across both files
3. Nice to have: Extract magic number to constant and improve null handling

---

[lowrt]

@claude review

[claude]

Claude finished @lowrt's task in 1m 38s —— View job

---

Code Review Complete

I've reviewed the changes in this PR. The implementation successfully adds replay functionality and fixes the report display issue. Here are my findings:

---

Summary

This PR implements:

1. Replay feature: Allows users to replay earthquake events from report pages
2. Report display fix: Fixes the issue where reports weren't showing correctly on first load by adding a reactive data notifier
3. Responsive layout improvements: Better tablet support for report panels

---

Findings

✅ Positive Changes

1. Clean replay implementation - The MapMonitorPage widget (page.dart:300-314) is well-structured and follows Flutter best practices
2. Proper data reactivity - Adding dataNotifier (report.dart:48) and wrapping the UI with ValueListenableBuilder (report.dart:334-336) ensures the UI updates when report data changes
3. Consistent parameter handling - The replayTimestamp parameter flows cleanly through MapPageOptions → MapPage → MonitorMapLayerManager
4. Good visual feedback - Orange color indicator for replay mode in the monitor display (monitor.dart:1392)
5. Internationalization improvements - Using .i18n for the replay button label

⚠️ Issues Found

1. Hardcoded default timestamp (monitor.dart:79)

   replayTimestamp = 1762892804468,

   • This appears to be a hardcoded test timestamp
   • Should either be removed or documented as a fallback
   • Location: lib/app/map/_lib/managers/monitor.dart:79

2. Different time offsets for replay (Inconsistency)

   • report_sheet_content.dart:74 uses -2000 (2 seconds before)
   • report.dart:634 uses -2000 (2 seconds before)
   • This is actually consistent now (both use 2000ms), but different from what the previous review mentioned (which showed -5000)
   • Recommendation: Extract this to a named constant like const kReplayLeadTimeMs = 2000; for maintainability

3. Missing file ending newline (report.dart:848)

   • The file should end with a newline character according to most Dart style guides
   • Location: lib/app/map/_lib/managers/report.dart:848

4. Potential performance consideration (report.dart:334-336)

   • The outer ValueListenableBuilder on dataNotifier wraps the entire ResponsiveContainer
   • Since dataNotifier only changes when report data is fetched, this should be fine
   • However, it causes a full rebuild of the entire sheet
   • Consideration: This is probably acceptable since data fetches are infrequent

5. Tablet layout fix looks good (responsive_container.dart:34-37)

   • The fix properly handles tablet layouts by checking isLargeTablet
   • This resolves the issue where panels might have been sized incorrectly on tablets

💡 Suggestions

1. Extract magic number to constant

   // At top of file or in constants file
   const kReplayLeadTimeMs = 2000; // Start replay 2 seconds before earthquake
   
   // Then use:
   replayTimestamp: report.time.millisecondsSinceEpoch - kReplayLeadTimeMs

2. Add documentation for replayTimestamp

   /// The timestamp in milliseconds since epoch to start replay from.
   /// When set, the monitor will operate in replay mode starting from this time.
   final int? replayTimestamp;

3. Consider removing or documenting the default value (monitor.dart:79)

   • Either remove the default value entirely
   • Or document why this specific timestamp is used as a default

4. Consider optimization (Optional)

   • If performance becomes an issue, the ValueListenableBuilder could be moved deeper to only rebuild the necessary parts
   • Current implementation is fine for now

---

Testing Recommendations

Please verify:

• First-time loading of reports displays correctly
• Replay button works from both report views
• Replay starts 2 seconds before earthquake time
• URL query parameters with replay parameter work correctly
• Tablet layout displays correctly (both large and small tablets)
• Replay mode shows orange indicator in monitor display
• Data updates trigger UI refresh properly

---

Verdict

The implementation is solid and functional. The main issues are:

Must fix:

1. Add newline at end of file (report.dart:848)
2. Remove or document the hardcoded default timestamp (monitor.dart:79)

Should fix:
3. Extract magic number -2000 to a named constant

Nice to have:
4. Add documentation comments for the replayTimestamp parameter

The code follows good practices and the reactive data pattern fix is well-implemented. After addressing the "must fix" items, this is ready to merge.

---