小田急テーマを追加#5699
Merged
TinyKitten merged 6 commits intodevfrom Mar 27, 2026
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthrough小田急テーマ「ODAKYU」を追加。翻訳、テーマ定義、テーマ選択肢、ヘッダー/ライン表示のコンポーネント追加・拡張、TTSテンプレート調整、関連テストを導入しました。 Changes
Sequence Diagram(s)（該当条件に合致しないため省略） Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/utils/resolveThemeForLine.test.ts (1)
124-130: prefix仕様を直接確認するテストを1件足すと安心です。
startsWithを使っているため、小田急電鉄のような前方一致ケースも追加すると意図がより固定化できます。💡 追加テスト例
+ it('小田急電鉄のような前方一致でもODAKYUを返す', () => { + expect( + resolveThemeForLine( + makeLine({ id: 25002, company: makeCompany('小田急電鉄') }) + ) + ).toBe(APP_THEME.ODAKYU); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/resolveThemeForLine.test.ts` around lines 124 - 130, Add a second test that verifies prefix matching by calling resolveThemeForLine with makeLine({ id: 25001, company: makeCompany('小田急電鉄') }) and asserting it returns APP_THEME.ODAKYU; this ensures the startsWith behavior in resolveThemeForLine is locked down (add a new it block similar to the existing '小田急の路線はODAKYUを返す' test).src/components/BarTerminalOdakyu.tsx (1)
14-18:hasTerminusの利用方針を明確化したいです。
Propsで必須ですが現状未使用です。未使用ならAPIから外す、互換目的なら描画分岐で使う、のどちらかに寄せると契約が明確になります。Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/BarTerminalOdakyu.tsx` around lines 14 - 18, Props currently declares hasTerminus but BarTerminalOdakyu never uses it; decide one of two fixes and apply it consistently: (A) remove hasTerminus from the Props type and any callers (or update call sites) if it is truly unused, or (B) keep hasTerminus for backward compatibility and add a rendering branch inside the BarTerminalOdakyu component that uses hasTerminus (and barHighlightOffset) to toggle the terminus marker/visual state (e.g., alter the SVG path/group when hasTerminus is true); update Prop defaults/types and any tests/callers accordingly. Ensure you update the Props type and component render logic (BarTerminalOdakyu) and any places that pass hasTerminus so the API contract is clear.src/components/TrainTypeBox.tsx (1)
91-91:fontSizeScaleに下限ガードを入れると安全です。
設定値が 0 以下の場合に文字が潰れるため、最小値をクランプしておくと堅牢です。💡 提案パッチ
const TrainTypeBox: React.FC<Props> = ({ trainType, localTypePrefix = '', nextTrainTypeColor = '#444', darkenColor = false, fontSizeScale = 1, }: Props) => { + const safeFontSizeScale = Math.max(0.5, fontSizeScale); + @@ - fontSize: (isTablet ? 18 * 1.5 : 18) * fontSizeScale, + fontSize: (isTablet ? 18 * 1.5 : 18) * safeFontSizeScale, @@ - fontSize: (isTablet ? 18 * 1.5 : 18) * fontSizeScale, + fontSize: (isTablet ? 18 * 1.5 : 18) * safeFontSizeScale,Also applies to: 301-302, 321-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TrainTypeBox.tsx` at line 91, The prop/variable fontSizeScale in the TrainTypeBox component can be <= 0 which collapses text; clamp it to a small positive minimum before use (e.g. const safeFontSizeScale = Math.max(fontSizeScale ?? 1, 0.1)) and replace direct uses of fontSizeScale with safeFontSizeScale; apply the same change to the other occurrences referenced (the other fontSizeScale usages at the spots corresponding to lines ~301-302 and ~321-322) so all calculations (style/font-size computations) use the clamped value.src/utils/themeInfo.ts (1)
67-72: TODOコメント: 小田急テーマ用スクリーンショットの差し替え現在Tokyo Metroのスクリーンショットがプレースホルダーとして使用されています。TODOコメントが残っているので、小田急テーマ専用のスクリーンショットを後で追加する必要があります。
この作業を追跡するためのIssueを作成しましょうか？
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/themeInfo.ts` around lines 67 - 72, Replace the Tokyo Metro placeholder screenshots used for APP_THEME.ODAKYU by adding/committing Odakyu-specific images and updating the spImage and tabletImage require paths (and keep descriptionKey: 'themeDescriptionOdakyu'); remove the TODO comment. Specifically, add new assets (e.g., odakyu-sp.webp and odakyu-tablet.webp) to the assets/images/themes folder and update the require(...) values for APP_THEME.ODAKYU's spImage and tabletImage accordingly, then commit and, if desired, create an issue to track adding finalized screenshots.src/components/LineBoardEast.tsx (1)
142-143: ハイライト位置の0.35は共通化したいです。
renderBarGradients()とBarTerminalOdakyuで別管理なので、片方だけ調整するとバー本体と終端 SVG のハイライトがずれます。定数か共通 prop に寄せたいです。Also applies to: 320-334
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LineBoardEast.tsx` around lines 142 - 143, The hard-coded 0.35 highlight value is duplicated between barHighlightOffset, renderBarGradients(), and BarTerminalOdakyu causing misalignment; extract this magic number into a single shared constant or prop (e.g., HIGHLIGHT_RATIO or highlightOffset prop) and replace the literal 0.35 in barHighlightOffset, the gradient generation in renderBarGradients, and the highlight position used in BarTerminalOdakyu so both bar body and terminal SVG consume the same source of truth; update function/component signatures as needed to pass the shared value rather than keeping separate literals.src/hooks/useBusTTSText.ts (1)
34-39: ODAKYU の TTS 解決が二重定義です。
resolveTemplateTheme()がODAKYU -> TOKYO_METROを返すので、ここで足したAPP_THEME.ODAKYUの空テンプレートは実行時に参照されません。片方に寄せておくと将来のズレを防げます。Also applies to: 499-500, 700-700
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useBusTTSText.ts` around lines 34 - 39, Duplicate definition: APP_THEME.ODAKYU is added to the theme templates but resolveTemplateTheme() already maps ODAKYU -> TOKYO_METRO, so the added empty template is never used; remove the redundant APP_THEME.ODAKYU entry from the theme templates (and the other duplicated spots referenced) so the mapping is only defined in one place (resolveTemplateTheme) or alternatively remove the remapping in resolveTemplateTheme if you prefer the template object to be authoritative — pick one approach and apply consistently across resolveTemplateTheme and the template objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/HeaderOdakyu.test.tsx`:
- Around line 117-127: Update the test that asserts what HeaderOdakyu passes to
TrainTypeBox to also check for the new ODAKYU-specific props: include
darkenColor and fontSizeScale in the expect.objectContaining passed to
toHaveBeenCalledWith for TrainTypeBox in the HeaderOdakyu test (the test that
uses createMockHeaderProps and requires './TrainTypeBox'). Ensure the assertion
verifies the expected values (or defaults) for darkenColor and fontSizeScale
alongside localTypePrefix and nextTrainTypeColor.
---
Nitpick comments:
In `@src/components/BarTerminalOdakyu.tsx`:
- Around line 14-18: Props currently declares hasTerminus but BarTerminalOdakyu
never uses it; decide one of two fixes and apply it consistently: (A) remove
hasTerminus from the Props type and any callers (or update call sites) if it is
truly unused, or (B) keep hasTerminus for backward compatibility and add a
rendering branch inside the BarTerminalOdakyu component that uses hasTerminus
(and barHighlightOffset) to toggle the terminus marker/visual state (e.g., alter
the SVG path/group when hasTerminus is true); update Prop defaults/types and any
tests/callers accordingly. Ensure you update the Props type and component render
logic (BarTerminalOdakyu) and any places that pass hasTerminus so the API
contract is clear.
In `@src/components/LineBoardEast.tsx`:
- Around line 142-143: The hard-coded 0.35 highlight value is duplicated between
barHighlightOffset, renderBarGradients(), and BarTerminalOdakyu causing
misalignment; extract this magic number into a single shared constant or prop
(e.g., HIGHLIGHT_RATIO or highlightOffset prop) and replace the literal 0.35 in
barHighlightOffset, the gradient generation in renderBarGradients, and the
highlight position used in BarTerminalOdakyu so both bar body and terminal SVG
consume the same source of truth; update function/component signatures as needed
to pass the shared value rather than keeping separate literals.
In `@src/components/TrainTypeBox.tsx`:
- Line 91: The prop/variable fontSizeScale in the TrainTypeBox component can be
<= 0 which collapses text; clamp it to a small positive minimum before use (e.g.
const safeFontSizeScale = Math.max(fontSizeScale ?? 1, 0.1)) and replace direct
uses of fontSizeScale with safeFontSizeScale; apply the same change to the other
occurrences referenced (the other fontSizeScale usages at the spots
corresponding to lines ~301-302 and ~321-322) so all calculations
(style/font-size computations) use the clamped value.
In `@src/hooks/useBusTTSText.ts`:
- Around line 34-39: Duplicate definition: APP_THEME.ODAKYU is added to the
theme templates but resolveTemplateTheme() already maps ODAKYU -> TOKYO_METRO,
so the added empty template is never used; remove the redundant APP_THEME.ODAKYU
entry from the theme templates (and the other duplicated spots referenced) so
the mapping is only defined in one place (resolveTemplateTheme) or alternatively
remove the remapping in resolveTemplateTheme if you prefer the template object
to be authoritative — pick one approach and apply consistently across
resolveTemplateTheme and the template objects.
In `@src/utils/resolveThemeForLine.test.ts`:
- Around line 124-130: Add a second test that verifies prefix matching by
calling resolveThemeForLine with makeLine({ id: 25001, company:
makeCompany('小田急電鉄') }) and asserting it returns APP_THEME.ODAKYU; this ensures
the startsWith behavior in resolveThemeForLine is locked down (add a new it
block similar to the existing '小田急の路線はODAKYUを返す' test).
In `@src/utils/themeInfo.ts`:
- Around line 67-72: Replace the Tokyo Metro placeholder screenshots used for
APP_THEME.ODAKYU by adding/committing Odakyu-specific images and updating the
spImage and tabletImage require paths (and keep descriptionKey:
'themeDescriptionOdakyu'); remove the TODO comment. Specifically, add new assets
(e.g., odakyu-sp.webp and odakyu-tablet.webp) to the assets/images/themes folder
and update the require(...) values for APP_THEME.ODAKYU's spImage and
tabletImage accordingly, then commit and, if desired, create an issue to track
adding finalized screenshots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 517859ff-7288-4955-9f92-6aaac5b44790
📒 Files selected for processing (23)
assets/translations/en.jsonassets/translations/ja.jsonsrc/components/BarTerminalEast.tsxsrc/components/BarTerminalOdakyu.tsxsrc/components/Header.tsxsrc/components/HeaderEast/HeaderEast.tsxsrc/components/HeaderEast/config.tssrc/components/HeaderOdakyu.test.tsxsrc/components/HeaderOdakyu.tsxsrc/components/LineBoard.tsxsrc/components/LineBoard/shared/components/LineDot.tsxsrc/components/LineBoardEast.tsxsrc/components/PassChevronTY.tsxsrc/components/TrainTypeBox.tsxsrc/components/TransfersHeading.tsxsrc/constants/theme.tssrc/hooks/useBusTTSText.tssrc/hooks/useTTSText.tssrc/models/Theme.tssrc/utils/resolveThemeForLine.test.tssrc/utils/resolveThemeForLine.tssrc/utils/theme.tssrc/utils/themeInfo.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/TrainTypeBox.tsx (1)
106-109:trainTypeColormemo の中間変数baseは不要です。この変更では
base変数に代入して即座にreturn baseしているため、冗長です。元のコードのままで問題ありません。♻️ 簡略化の提案
const trainTypeColor = useMemo(() => { - const base = trainType?.color ?? '#1f63c6'; - return base; + return trainType?.color ?? '#1f63c6'; }, [trainType]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TrainTypeBox.tsx` around lines 106 - 109, The useMemo for trainTypeColor unnecessarily assigns to an intermediate variable "base"; simplify the callback by returning trainType?.color ?? '#1f63c6' directly. Update the useMemo in TrainTypeBox (the trainTypeColor constant) to remove the redundant "base" variable and keep the dependency array as [trainType].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/TrainTypeBox.tsx`:
- Around line 106-109: The useMemo for trainTypeColor unnecessarily assigns to
an intermediate variable "base"; simplify the callback by returning
trainType?.color ?? '#1f63c6' directly. Update the useMemo in TrainTypeBox (the
trainTypeColor constant) to remove the redundant "base" variable and keep the
dependency array as [trainType].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75566c23-3fa1-4308-b0cf-0a57a6421dc5
📒 Files selected for processing (5)
src/components/BarTerminalOdakyu.tsxsrc/components/HeaderOdakyu.test.tsxsrc/components/LineBoardEast.tsxsrc/components/TrainTypeBox.tsxsrc/utils/resolveThemeForLine.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils/resolveThemeForLine.test.ts
- src/components/BarTerminalOdakyu.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
closes #5120
Test plan
npm run typecheckパスnpm run lintパスnpm test全146 suites / 1372 tests パス🤖 Generated with Claude Code
Summary by CodeRabbit