Skip to content

新首頁樣式#510

Draft
kamiya4047 wants to merge 8 commits intomainfrom
feat/new-home
Draft

新首頁樣式#510
kamiya4047 wants to merge 8 commits intomainfrom
feat/new-home

Conversation

@kamiya4047
Copy link
Copy Markdown
Member

這是什麼類型的 PR?

選擇所有符合的項目

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

描述

新首頁樣式

相關 issue

  • 相關問題 #
  • closes #

QA 指南、截圖、錄像

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

Screenshot_1777365209

UI 無障礙清單

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

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

@claude
Copy link
Copy Markdown

claude Bot commented Apr 28, 2026

Claude finished @kamiya4047's task in 3m 9s —— View job


PR Review: 新首頁樣式

  • Gather context and understand PR scope
  • Review new home feature files
  • Review model and widget files
  • Review settings and infrastructure changes
  • Review shader and asset additions
  • Compile and post final review feedback

Summary

This PR introduces a new experimental home screen behind a feature flag (experimental__newHomeScreen). The implementation is well-structured overall — clean ChangeNotifier model, good use of Selector for granular rebuilds, a GPU fragment shader for the animated sky background, and solid documentation coverage. A few bugs, dead code, and convention gaps need to be addressed before this is ready to graduate from experimental.


Bugs

1. Hardcoded sunrise/sunset times in DayCycle

lib/app/new_home/_widgets/day_cycle.dart:43–45

sunrise: const TimeOfDay(hour: 5, minute: 30),
sunset: const TimeOfDay(hour: 18, minute: 30),

These are hardcoded and never change regardless of the user's location or date. For a Taiwan disaster-prevention app, a fixed 5:30/18:30 can be off by 1–2 hours near solstices. Should be computed from coordinates + date, or sourced from the API. Fix this →

2. Time formatting doesn't zero-pad minutes

lib/app/new_home/_widgets/day_cycle.dart:66–68

String _formatTime(TimeOfDay t) {
  return '${t.hour}:${t.minute}';
}

Produces "5:5" instead of "5:05". Use t.format(context) (respects locale) or '${t.hour}:${t.minute.toString().padLeft(2, '0')}'. Fix this →

3. Navigation bar does nothing

lib/app/new_home/layout.dart:80–101

NavigationBarM3E has no onDestinationSelected callback. Tapping Map or 小工具 tabs is a no-op. Fix this →

4. Notifications button is a no-op

lib/app/new_home/layout.dart:54–55

onPressed: () {},

Either implement or remove this button to avoid confusing users. Fix this →


Memory / Lifecycle

5. context access in Radar.dispose()

lib/app/new_home/_widgets/radar.dart:211

context.home.removeListener(_onHomeModelChanged);

Accessing context in dispose() is unreliable — the element may be deactivated. Store the HomeModel reference obtained in didChangeDependencies:

HomeModel? _homeModel;

@override
void didChangeDependencies() {
  super.didChangeDependencies();
  // ...
  _homeModel ??= context.home;
  _homeModel!.addListener(_onHomeModelChanged);
}

@override
void dispose() {
  _homeModel?.removeListener(_onHomeModelChanged);
  // ...
}

Fix this →


Dead Code / Wasted Bundle Size

6. _scrollAnimator unused

lib/app/new_home/layout.dart:19

late final _scrollAnimator = AnimationController(vsync: this);

Created and disposed, but never animated or referenced anywhere. Remove it. Fix this →

7. NotoSansTC.ttf adds 11.9 MB to the bundle but is never used

assets/fonts/NotoSansTC.ttf

The file is present in assets/fonts/ but is not declared in pubspec.yaml under fonts: and is never referenced in any Dart file. The app still uses google_fonts (GoogleFonts.notoSansTc()) for the fallback font. This is dead weight — remove it. Fix this →


Code Quality / Conventions

8. Missing i18n for hint text strings

lib/app/new_home/_widgets/assistant_hint.dart:64–79

All strings in _buildHintText'夜深了', '早安', temperature comments — are hardcoded Chinese without .i18n. greeting.dart:18 correctly uses .i18n for the same strings. Fix this →

9. Duplicated greeting switch in assistant_hint.dart

lib/app/new_home/_widgets/assistant_hint.dart:64–67 and lib/app/new_home/_widgets/greeting.dart:17–22

Identical switch (hour) greeting logic exists in both files. Extract to a shared helper (e.g., lib/utils/greeting.dart). Fix this →

10. layout.dart missing library doc comment

lib/app/new_home/layout.dart:1

The file has no top-level library doc comment. Per the project convention in CLAUDE.md: every file must have /// ... before the first declaration. Fix this →

11. Navigator.of(context) instead of context.navigator

lib/app/new_home/_widgets/location_chip.dart:127

void dismissSheet() => Navigator.of(context).popUntil(...);

Per CLAUDE.md: use context.navigator instead of Navigator.of(context). Fix this →

12. Set<String>() constructor instead of literal

lib/app/new_home/_widgets/location_chip.dart:222

final quickCodes = Set<String>();  // should be <String>{}

Prefer the <String>{} literal form per Dart conventions. Fix this →

13. Potential crash in _buildTownList

lib/app/new_home/_widgets/location_chip.dart:173

label: Text(towns.first.$2.cityWithLevel),

towns.first throws StateError on an empty list. Since _selectedCity is set from tapping a city in _buildCityList, this should always have towns — but add a guard to be safe. Fix this →


Minor Notes (non-blocking)

  • WeatherBackground animation (weather_background.dart:40–43): The AnimationController with a 60s duration is used only as a repaint trigger — its .value is never consumed. A Ticker would be more semantically precise, though functionally this works.
  • GridView with shrinkWrap: true inside a ListView (weather_parameters.dart:41): Fine for 4 static items, but shrinkWrap forces layout of all children up front. Consider Column with Flexible if the item count stays fixed.
  • AllObservationAverage TREM column (all_observation_average.dart:28): Always shows "--". The widget doc acknowledges this is unimplemented, but a TODO annotation would make it more trackable.
  • _seedColor as a top-level function (page.dart:124): Used only by _NewHomePageState. Consider making it a static method on the state class or a local function to avoid polluting the file-level namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants