routeTypesクエリへの移行とモーダルUIブラッシュアップ#16
Merged
TinyKitten merged 6 commits intomainfrom Feb 25, 2026
Merged
Conversation
- 経路検索をroutesからrouteTypesに変更し、駅データはlineGroupStationsでバッチ取得 - RouteInfoModalのデザイン改善: セクション分け、停車条件アイコン追加、フッター固定 - 各線の種別を会社単位でグルーピングし、trainTypeがnullの場合のフォールバック改善 - 路線カラードットがflex-wrap対応で多路線でも崩れないように修正 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughGraphQLクエリを Changes
Sequence Diagram(s)sequenceDiagram
participant Page as app/page.tsx
participant Hook as useFetchRoutes
participant Aggregator as Chunk Aggregator
participant API as GraphQL API
Page->>Hook: useFetchRoutes()
Hook->>API: query ROUTE_TYPES
API-->>Hook: trainTypes (id, groupId)
Hook->>Aggregator: chunk groupIds (CHUNK_SIZE=10)
Aggregator->>API: buildLineGroupStationsQuery (batched aliases)
API-->>Aggregator: lineGroupStations per alias
Aggregator-->>Hook: Map<groupId, Station[]>
Hook-->>Page: [{id: trainTypeId, stops: Station[]}, ...]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/RouteInfoModal.tsx (1)
170-174: 閉じるボタンにtype="button"を明示してください。将来的にフォーム配下で再利用された場合の意図しない submit を防げます。
差分案
<button + type="button" onClick={onClose} aria-label="閉じる" className="p-1 rounded-full hover:bg-gray-100 transition-colors" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/RouteInfoModal.tsx` around lines 170 - 174, In the RouteInfoModal component, the close button (the element with onClick={onClose} and aria-label="閉じる") needs an explicit type attribute to avoid accidental form submits when this component is used inside a form; add type="button" to that button element so it does not default to "submit".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useFetchRoutes.ts`:
- Around line 55-58: The code currently only reads the first page's trainTypes
(const trainTypes = res.routeTypes.trainTypes) and returns early, ignoring
RouteTypesResponse.nextPageToken; update the logic in useFetchRoutes.ts to loop
(or recurse) fetching pages while nextPageToken is truthy, appending each page's
res.routeTypes.trainTypes to a single accumulator array (e.g.,
accumulatedTrainTypes) and only return after nextPageToken is empty; ensure you
pass the token into the GraphQL call (where the request uses nextPageToken) and
preserve existing behavior when there is no pagination.
- Around line 60-66: The current mapping in useFetchRoutes.ts assigns Route.id =
tt.groupId causing potential id collisions; update the returned Route objects to
preserve both identifiers (set id = tt.id and include groupId: tt.groupId) so
routes remain unique, then update consumers (app/page.tsx) to consistently
compare the correct identifier—either change stop.trainType?.groupId ===
Number(selectedRouteId) to stop.trainType?.id === Number(selectedRouteId) if you
choose to make selectedRouteId mean trainType.id, or keep comparing groupId
everywhere and use selectedRouteId as a groupId; ensure the Route shape (id and
groupId) and the meaning of selectedRouteId are unified across useFetchRoutes.ts
and app/page.tsx.
---
Nitpick comments:
In `@components/RouteInfoModal.tsx`:
- Around line 170-174: In the RouteInfoModal component, the close button (the
element with onClick={onClose} and aria-label="閉じる") needs an explicit type
attribute to avoid accidental form submits when this component is used inside a
form; add type="button" to that button element so it does not default to
"submit".
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/page.tsxcomponents/RouteInfoModal.tsxgraphql/queries.tshooks/useFetchRoutes.tshooks/useFetchStationsByGroupId.tstypes/stationapi.tsutils/generateSWRKey.ts
Comment on lines
+55
to
+58
| const trainTypes = res.routeTypes.trainTypes; | ||
| if (trainTypes.length === 0) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== nextPageToken の参照箇所確認 =="
rg -n -C2 'nextPageToken' hooks/useFetchRoutes.ts graphql/queries.ts types/stationapi.ts 2>/dev/null || echo "nextPageToken references: none found or files not present"
echo
echo "== routeTypes 取得後ロジック確認 =="
rg -n -C3 'routeTypes|trainTypes.length|return \[\]' hooks/useFetchRoutes.ts 2>/dev/null || echo "routeTypes patterns: none found"
echo
echo "== useFetchRoutes.ts ファイル全体確認 =="
cat -n hooks/useFetchRoutes.tsRepository: TrainLCD/Go
Length of output: 3557
nextPageToken が未使用のまま、ページング結果の欠落リスク
RouteTypesResponse に nextPageToken フィールドが定義されており、GraphQL クエリも返していますが、コード内で参照されていません。現在の実装は1回の API 呼び出しで終わり、ページング環境では最初のページのみが処理されます。nextPageToken が空になるまで繰り返し取得するループが必要です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useFetchRoutes.ts` around lines 55 - 58, The code currently only reads
the first page's trainTypes (const trainTypes = res.routeTypes.trainTypes) and
returns early, ignoring RouteTypesResponse.nextPageToken; update the logic in
useFetchRoutes.ts to loop (or recurse) fetching pages while nextPageToken is
truthy, appending each page's res.routeTypes.trainTypes to a single accumulator
array (e.g., accumulatedTrainTypes) and only return after nextPageToken is
empty; ensure you pass the token into the GraphQL call (where the request uses
nextPageToken) and preserve existing behavior when there is no pagination.
Comment on lines
+60
to
+66
| const groupIds = trainTypes.map((tt) => tt.groupId); | ||
| const grouped = await fetchStationsByChunks(groupIds); | ||
|
|
||
| return trainTypes.map((tt) => ({ | ||
| id: tt.groupId, | ||
| stops: grouped.get(tt.groupId) ?? [], | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and locate the target file
find . -name "useFetchRoutes.ts" -o -name "page.tsx" | head -20Repository: TrainLCD/Go
Length of output: 95
🏁 Script executed:
# Read the target file to see the code at lines 60-66
wc -l hooks/useFetchRoutes.tsRepository: TrainLCD/Go
Length of output: 81
🏁 Script executed:
# Get more context around the mentioned lines
sed -n '50,75p' hooks/useFetchRoutes.tsRepository: TrainLCD/Go
Length of output: 559
🏁 Script executed:
# Check the Route type definition
rg -n "type Route|interface Route" --type ts --type tsxRepository: TrainLCD/Go
Length of output: 82
🏁 Script executed:
# Search for how Route.id is used throughout the codebase
rg -n "\.id\s*==|\.id\s*===|selectedRouteId" --type ts --type tsxRepository: TrainLCD/Go
Length of output: 82
🏁 Script executed:
# Search for Route type definition
rg -n "type Route\s*=|interface Route\s*{" -A 5Repository: TrainLCD/Go
Length of output: 186
🏁 Script executed:
# Search for selectedRouteId usage
rg -n "selectedRouteId" -B 2 -A 2Repository: TrainLCD/Go
Length of output: 1955
🏁 Script executed:
# Check app/page.tsx for the groupId comparison mentioned in review
head -100 app/page.tsx | tail -50Repository: TrainLCD/Go
Length of output: 1679
🏁 Script executed:
# Look for how trainTypes is structured
rg -n "trainTypes|RouteTypesResponse" -B 2 -A 5 -C 1Repository: TrainLCD/Go
Length of output: 1494
🏁 Script executed:
# Get full RouteTypeTrainType structure
rg -n "RouteTypeTrainType|interface RouteType" -A 10Repository: TrainLCD/Go
Length of output: 445
🏁 Script executed:
# Get Route type definition
rg -n "^type Route\s*=|^export type Route\s*=|^export interface Route" -A 10Repository: TrainLCD/Go
Length of output: 627
🏁 Script executed:
# Check graphql query to see what fields are selected
sed -n '220,240p' graphql/queries.tsRepository: TrainLCD/Go
Length of output: 594
🏁 Script executed:
# Check the entire useFetchRoutes function to understand flow
cat -n hooks/useFetchRoutes.ts | head -70Repository: TrainLCD/Go
Length of output: 2412
🏁 Script executed:
# Check if stops/Station has trainType property
rg -n "interface Station|type Station" -A 10Repository: TrainLCD/Go
Length of output: 1699
🏁 Script executed:
# Verify the logic - check if multiple trainTypes can have same groupId
rg -n "trainType\?\.id|trainType\?.groupId" -B 2 -A 2Repository: TrainLCD/Go
Length of output: 585
🏁 Script executed:
# Check if there's any documentation or test showing multiple trainTypes with same groupId
rg -n "groupId.*trainTypes|trainTypes.*groupId"Repository: TrainLCD/Go
Length of output: 138
🏁 Script executed:
# Check what STATION_FRAGMENT includes to understand if trainType is in stops
rg -n "STATION_FRAGMENT|StationFields" -A 20Repository: TrainLCD/Go
Length of output: 2749
🏁 Script executed:
# Verify the complete flow - check what properties Station/stops actually have when returned
rg -n "stop\." app/page.tsx | head -20Repository: TrainLCD/Go
Length of output: 697
🏁 Script executed:
# Check if there's any type that extends Station with trainType
rg -n "trainType" types/stationapi.tsRepository: TrainLCD/Go
Length of output: 152
🏁 Script executed:
# Check STATION_FIELDS to see what's included
sed -n '1,110p' graphql/queries.ts | tail -30Repository: TrainLCD/Go
Length of output: 464
🏁 Script executed:
# Find TrainType definition to understand its structure
rg -n "type TrainType|interface TrainType" -A 5Repository: TrainLCD/Go
Length of output: 801
🏁 Script executed:
# Check if trainType has id field
rg -n "trainType\?.id" app/page.tsxRepository: TrainLCD/Go
Length of output: 37
🏁 Script executed:
# Check if there are multiple trainTypes with same groupId in real usage
# Look for patterns or comments that explain the trainType selection logic
rg -n "trainType|groupId" components/RouteInfoModal.tsx -B 2 -A 2 | head -40Repository: TrainLCD/Go
Length of output: 1189
🏁 Script executed:
# Get full context of how selectedRouteId is being used end-to-end
cat -n app/page.tsx | sed -n '610,650p'Repository: TrainLCD/Go
Length of output: 1484
🏁 Script executed:
# Check if there's a reason why trainType.id isn't used currently
git log --oneline hooks/useFetchRoutes.ts | head -5Repository: TrainLCD/Go
Length of output: 95
Route.id に groupId を使うと識別子衝突が発生するリスクがあります。
trainTypes は id と groupId を別で返しているため、複数の trainType が同じ groupId を持つ場合、id: tt.groupId では Route.id が衝突する可能性があります。ただし、提案されている修正には注意が必要です。
現在、app/page.tsx の行 641 と 666 では stop.trainType?.groupId === Number(selectedRouteId) で trainType を検索しており、修正後に id: tt.id を使用すると、このロジックが機能しなくなります。修正時には以下の対応が必要です：
RouteにgroupIdを保持させるか、別の方法で groupId を渡す- 検索ロジックを
trainType.id比較に更新する selectedRouteIdの意味を統一する（trainType.id か groupId か明確にする）
現在のコードは groupId で station 取得と trainType 検索の両方を行っており、単に id に変更するだけでは不十分です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useFetchRoutes.ts` around lines 60 - 66, The current mapping in
useFetchRoutes.ts assigns Route.id = tt.groupId causing potential id collisions;
update the returned Route objects to preserve both identifiers (set id = tt.id
and include groupId: tt.groupId) so routes remain unique, then update consumers
(app/page.tsx) to consistently compare the correct identifier—either change
stop.trainType?.groupId === Number(selectedRouteId) to stop.trainType?.id ===
Number(selectedRouteId) if you choose to make selectedRouteId mean trainType.id,
or keep comparing groupId everywhere and use selectedRouteId as a groupId;
ensure the Route shape (id and groupId) and the meaning of selectedRouteId are
unified across useFetchRoutes.ts and app/page.tsx.
useFetchStationsByGroupIdからデータが取得できない場合に スケルトンが永続する問題を、routesデータからの フォールバック取得で解決。不要なFragmentも除去。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
trainTypeが未設定(undefined)の停車駅を除外してから 種別の一意性を判定するよう変更。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
方向の0/1が逆だった問題を修正し、groupIdの参照元を undefinedになり得るfromStation/toStationから selectedFromStationId/selectedToStationIdに変更。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/page.tsx (1)
662-679: 駅名フォールバック処理は共通化して重複を減らしたいです。
fromStationDisplayNameとtoStationDisplayNameはほぼ同一ロジックなので、ヘルパー化すると今後の修正漏れを防げます。💡 リファクタ例
+const resolveStationDisplayName = ( + stationName: string | undefined, + selectedGroupId: string, + routes: Route[] | undefined, +) => { + if (stationName) return stationName; + const groupId = Number(selectedGroupId); + const stop = routes?.flatMap((r) => r.stops).find((s) => s.groupId === groupId); + return stop ? `${stop.name}駅` : undefined; +}; -const fromStationDisplayName = useMemo(() => { - if (fromStation?.name) return fromStation.name; - const fromGroupId = Number(selectedFromStationId); - const stop = routes - ?.flatMap((r) => r.stops) - .find((s) => s.groupId === fromGroupId); - return stop ? `${stop.name}駅` : undefined; -}, [fromStation?.name, routes, selectedFromStationId]); +const fromStationDisplayName = useMemo( + () => resolveStationDisplayName(fromStation?.name, selectedFromStationId, routes), + [fromStation?.name, routes, selectedFromStationId], +); -const toStationDisplayName = useMemo(() => { - if (toStation?.name) return toStation.name; - const toGroupId = Number(selectedToStationId); - const stop = routes - ?.flatMap((r) => r.stops) - .find((s) => s.groupId === toGroupId); - return stop ? `${stop.name}駅` : undefined; -}, [toStation?.name, routes, selectedToStationId]); +const toStationDisplayName = useMemo( + () => resolveStationDisplayName(toStation?.name, selectedToStationId, routes), + [toStation?.name, routes, selectedToStationId], +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 662 - 679, Extract the duplicated fallback logic in the useMemo blocks for fromStationDisplayName and toStationDisplayName into a single helper (e.g., getStationDisplayName(selectedStationId, station, routes)) that: returns station.name if present, otherwise converts selectedStationId to Number, finds the matching stop via routes.flatMap(r=>r.stops).find(s=>s.groupId===groupId) and returns `${stop.name}駅` or undefined; then replace both useMemo bodies to call this helper and ensure each useMemo's dependency array still references the same inputs (selectedFromStationId/selectedToStationId, fromStation?.name/toStation?.name, routes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/page.tsx`:
- Around line 789-800: The subtitle logic currently renders nothing when only
one of fromStationDisplayName or toStationDisplayName is available; update the
conditional rendering so the subtitle paragraph (the <p> that uses
fromStationDisplayName and toStationDisplayName) displays when at least one of
fromStationDisplayName or toStationDisplayName is truthy and params.get("mode")
!== "line", and adjust its content to show a fallback or only the available name
(e.g., show "Unknown" or omit the missing side with the existing separator) so
users see meaningful text when only one station is resolved; keep the Skeleton
rendering when neither name exists by leaving its condition as the case where
both are falsy.
---
Nitpick comments:
In `@app/page.tsx`:
- Around line 662-679: Extract the duplicated fallback logic in the useMemo
blocks for fromStationDisplayName and toStationDisplayName into a single helper
(e.g., getStationDisplayName(selectedStationId, station, routes)) that: returns
station.name if present, otherwise converts selectedStationId to Number, finds
the matching stop via routes.flatMap(r=>r.stops).find(s=>s.groupId===groupId)
and returns `${stop.name}駅` or undefined; then replace both useMemo bodies to
call this helper and ensure each useMemo's dependency array still references the
same inputs (selectedFromStationId/selectedToStationId,
fromStation?.name/toStation?.name, routes).
重複したフォールバックロジックをgetStationDisplayNameに統合し、 片方の駅名のみ取得できた場合にもサブタイトルを表示するよう修正。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/page.tsx (2)
713-720:⚠️ Potential issue | 🟠 Major
useCallbackの依存配列にselectedFromStationIdとselectedToStationIdが不足しています
handleLaunchApp内でfromIndex/toIndexの算出にselectedFromStationIdとselectedToStationIdを直接使用していますが、依存配列には含まれていません。これにより、これらの値が変化した際に古い closure が使われ、directionの計算が stale な値に基づいて行われます。🛠 修正案
}, [ devMode, fromStation?.groupId, fromStop?.line?.id, route?.stops, + selectedFromStationId, + selectedToStationId, selectedRouteId, toStation?.groupId, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 713 - 720, handleLaunchApp を定義している useCallback の依存配列に selectedFromStationId と selectedToStationId が抜けているため、これらが変化しても古いクロージャが使われ direction 計算が stale になります；handleLaunchApp 内で fromIndex/toIndex を算出する際に参照している selectedFromStationId と selectedToStationId を useCallback の依存配列に追加して、クロージャがこれらの値変更時に再作成されるようにしてください（関数名: handleLaunchApp、変数: selectedFromStationId, selectedToStationId、場所: useCallback の依存配列）。
700-710:⚠️ Potential issue | 🟠 Major
fromStation?.groupIdがundefinedの場合、深リンクにsgid=undefinedが含まれます
fromStationsByGroupIdの取得が完了していない・またはエラーになった場合、fromStationがundefinedとなり、sgid=undefinedの URL が生成されます。lineId && lineGroupIdのガードは通過してしまうため、アプリ側で不正なパラメータを受け取ります。🛠 修正案
- if (lineId && lineGroupId) { + if (lineId && lineGroupId && fromStation?.groupId != null) { window.open( `${appScheme}?sgid=${fromStation?.groupId}&lid=${lineId}&lgid=${lineGroupId}&dir=${direction}`, ); return; } - if (lineId) { + if (lineId && fromStation?.groupId != null) { window.open( `${appScheme}?sgid=${fromStation?.groupId}&lid=${lineId}&dir=${direction}`, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 700 - 710, The deep-link URL includes fromStation?.groupId which can be undefined; update the window.open logic in app/page.tsx (the block using lineId, lineGroupId, fromStation, appScheme, direction) to guard against missing fromStation: compute a sgid segment only when fromStation and fromStation.groupId are defined (e.g. const sgidSegment = fromStation?.groupId ? `sgid=${fromStation.groupId}&` : ''), then build the URL using sgidSegment so you never emit "sgid=undefined"; apply this change to both branches (the lineId && lineGroupId branch and the lineId-only branch) and ensure parameters are correctly concatenated/encoded before calling window.open.
♻️ Duplicate comments (1)
app/page.tsx (1)
792-805: 片側のみ駅名が解決できた場合のサブタイトル表示が改善されました以前のレビューで指摘した「片側のみ解決時に空白になる問題」が修正されています。
(fromStationDisplayName || toStationDisplayName)条件と"..."フォールバックにより、一方のみ取得できた場合も適切に表示されます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 792 - 805, The subtitle rendering should show a Skeleton when neither fromStationDisplayName nor toStationDisplayName is available and show a single-line subtitle using the available name(s) (with "..." fallback) when at least one is present; update the JSX around params.get("mode") checks so the Skeleton branch uses condition (params.get("mode") !== "line" && !fromStationDisplayName && !toStationDisplayName) and the subtitle paragraph uses (params.get("mode") !== "line" && (fromStationDisplayName || toStationDisplayName)), rendering {fromStationDisplayName ?? "..."} - {toStationDisplayName ?? "..."} in the <p> and keep the Skeleton component as shown to cover the neither-resolved case.
🧹 Nitpick comments (1)
app/page.tsx (1)
263-266: TypeScript の型絞り込みに型述語を使用してください
.filter((id) => id != null)は実行時には正しく機能しますが、TypeScript 的には(number | null | undefined)[]の型が絞り込まれず、new Set(typeIds)に渡す配列の型が不正確になる可能性があります。♻️ 修正案
- const typeIds = targetRoute?.stops - .map((s) => s.trainType?.typeId) - .filter((id) => id != null); + const typeIds = targetRoute?.stops + .map((s) => s.trainType?.typeId) + .filter((id): id is number => id != null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 263 - 266, The array typeIds isn't narrowed from (number | null | undefined)[] for TypeScript; change the filter on targetRoute?.stops.map(s => s.trainType?.typeId) to use a type predicate such as .filter((id): id is number => id != null) so typeIds becomes number[] before calling new Set(typeIds).size > 1; update the filter only (referencing typeIds, targetRoute?.stops, trainType?.typeId) to perform proper compile-time narrowing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/page.tsx`:
- Around line 824-831: handleStationClick is currently a no-op (returns () =>
{}) but is passed into StationListBox as onTrainTypeClick when selectedLineId is
set, so tapping a station does nothing; implement handleStationClick to perform
the intended action (e.g., open the route/station info modal or navigate to
station details) by setting the relevant state and invoking the modal/display
function used elsewhere in the component (for example, setSelectedStation(...)
and setIsRouteModalOpen(true) or call the existing
showRouteModal/showStationDetails helper), and replace the noop with that
implementation so StationListBox.onTrainTypeClick triggers the UI action in
route (selectedLineId) mode.
- Around line 691-697: The calculation of direction using fromIndex and toIndex
is unsafe because Array.prototype.findIndex can return -1; update the logic
around fromIndex, toIndex and direction (computed from route?.stops,
selectedFromStationId, selectedToStationId) to guard against -1 values: first
check if either fromIndex or toIndex is -1 and handle that case explicitly
(e.g., bail out/return early, set direction to null/undefined, or choose a safe
default) so you don't compute a spurious direction and generate invalid deep
links; ensure all downstream code that uses direction handles the new sentinel
(null/undefined) case.
- Around line 662-672: getStationDisplayName may produce duplicated "駅" (e.g.,
"東京駅駅") because it blindly appends "駅" to stop.name; update
getStationDisplayName to check stop.name (from the found stop in
routes.flatMap(...).find(...)) and only append "駅" when stop.name does not
already end with "駅" (use a string suffix check on stop.name) so returned value
is either stop.name (if already endsWith "駅") or `${stop.name}駅`, otherwise fall
back to undefined.
---
Outside diff comments:
In `@app/page.tsx`:
- Around line 713-720: handleLaunchApp を定義している useCallback の依存配列に
selectedFromStationId と selectedToStationId が抜けているため、これらが変化しても古いクロージャが使われ
direction 計算が stale になります；handleLaunchApp 内で fromIndex/toIndex を算出する際に参照している
selectedFromStationId と selectedToStationId を useCallback
の依存配列に追加して、クロージャがこれらの値変更時に再作成されるようにしてください（関数名: handleLaunchApp、変数:
selectedFromStationId, selectedToStationId、場所: useCallback の依存配列）。
- Around line 700-710: The deep-link URL includes fromStation?.groupId which can
be undefined; update the window.open logic in app/page.tsx (the block using
lineId, lineGroupId, fromStation, appScheme, direction) to guard against missing
fromStation: compute a sgid segment only when fromStation and
fromStation.groupId are defined (e.g. const sgidSegment = fromStation?.groupId ?
`sgid=${fromStation.groupId}&` : ''), then build the URL using sgidSegment so
you never emit "sgid=undefined"; apply this change to both branches (the lineId
&& lineGroupId branch and the lineId-only branch) and ensure parameters are
correctly concatenated/encoded before calling window.open.
---
Duplicate comments:
In `@app/page.tsx`:
- Around line 792-805: The subtitle rendering should show a Skeleton when
neither fromStationDisplayName nor toStationDisplayName is available and show a
single-line subtitle using the available name(s) (with "..." fallback) when at
least one is present; update the JSX around params.get("mode") checks so the
Skeleton branch uses condition (params.get("mode") !== "line" &&
!fromStationDisplayName && !toStationDisplayName) and the subtitle paragraph
uses (params.get("mode") !== "line" && (fromStationDisplayName ||
toStationDisplayName)), rendering {fromStationDisplayName ?? "..."}
- {toStationDisplayName ?? "..."} in the <p> and keep the Skeleton
component as shown to cover the neither-resolved case.
---
Nitpick comments:
In `@app/page.tsx`:
- Around line 263-266: The array typeIds isn't narrowed from (number | null |
undefined)[] for TypeScript; change the filter on targetRoute?.stops.map(s =>
s.trainType?.typeId) to use a type predicate such as .filter((id): id is number
=> id != null) so typeIds becomes number[] before calling new Set(typeIds).size
> 1; update the filter only (referencing typeIds, targetRoute?.stops,
trainType?.typeId) to perform proper compile-time narrowing.
Comment on lines
+824
to
+831
| {selectedLineId ? ( | ||
| <StationListBox | ||
| isLoading={isStationsLoading} | ||
| stations={stationsByLineId} | ||
| onTrainTypeClick={handleStationClick} | ||
| error={fetchStationsError} | ||
| /> | ||
| ) : ( |
There was a problem hiding this comment.
handleStationClick が no-op のため、路線モードで駅をタップしても何も起きません
handleStationClick（Line 621）は () => {} を返すだけです。StationListBox の onTrainTypeClick に渡されており、路線モード（selectedLineId が存在するケース）で駅をクリックしても一切のアクションが発生しません。意図的な未実装であれば TODO コメントを残すことを推奨します。
未実装の handleStationClick の動作（ルート情報モーダルを開くなど）を実装する issue を作成しましょうか？
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/page.tsx` around lines 824 - 831, handleStationClick is currently a no-op
(returns () => {}) but is passed into StationListBox as onTrainTypeClick when
selectedLineId is set, so tapping a station does nothing; implement
handleStationClick to perform the intended action (e.g., open the route/station
info modal or navigate to station details) by setting the relevant state and
invoking the modal/display function used elsewhere in the component (for
example, setSelectedStation(...) and setIsRouteModalOpen(true) or call the
existing showRouteModal/showStationDetails helper), and replace the noop with
that implementation so StationListBox.onTrainTypeClick triggers the UI action in
route (selectedLineId) mode.
- typeIds filterにtype predicateを追加 - getStationDisplayNameの"駅"重複付与を防止 - handleLaunchAppのfindIndex -1ガードとsgid undefinedガードを追加 - handleLaunchAppのuseCallback依存配列にselectedFrom/ToStationIdを追加 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/page.tsx (1)
829-835:⚠️ Potential issue | 🟠 Major路線モードのリストタップが無反応のままです。
Line 833 で
onTrainTypeClickにhandleStationClickを渡していますが、Line 621 のhandleStationClickが no-op なので、路線モードで項目タップ時に何も起きません。結果画面の主要導線が機能していない状態です。🛠 修正例（既存のルート選択導線に合わせる）
- const handleStationClick = useCallback((stationId: number) => () => {}, []); + const handleStationClick = useCallback( + (routeId: number, typeId: number | undefined) => () => { + const targetRouteId = typeId ?? routeId; + setValue("selectedRouteId", targetRouteId.toString()); + params.update({ rid: targetRouteId.toString() }); + onRouteInfoOpenChange(); + }, + [onRouteInfoOpenChange, params, setValue], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.tsx` around lines 829 - 835, The StationListBox is wired to onTrainTypeClick with handleStationClick, but handleStationClick at line 621 is a no-op so tapping items does nothing; fix by wiring the prop to the actual route-selection handler (e.g., replace handleStationClick with the real handler used elsewhere such as handleTrainTypeClick or the route selection function that triggers navigation/result rendering), or implement handleStationClick to call the existing route selection logic (e.g., invoke the same function that navigates to the results or sets the selected route/station); update the StationListBox prop assignment and remove/replace the noop implementation so taps execute the intended flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/page.tsx`:
- Around line 829-835: The StationListBox is wired to onTrainTypeClick with
handleStationClick, but handleStationClick at line 621 is a no-op so tapping
items does nothing; fix by wiring the prop to the actual route-selection handler
(e.g., replace handleStationClick with the real handler used elsewhere such as
handleTrainTypeClick or the route selection function that triggers
navigation/result rendering), or implement handleStationClick to call the
existing route selection logic (e.g., invoke the same function that navigates to
the results or sets the selected route/station); update the StationListBox prop
assignment and remove/replace the noop implementation so taps execute the
intended flow.
Member
Author
これはこのPRの問題ではない |
|
Tip For best results, initiate chat on the files or code changes.
|
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.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
新機能
スタイル
バグ修正