PadArchの画面サイズ取得をDimensions.getからuseWindowDimensionsに変更#5348
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/PadArch.tsx (1)
424-432: パス文字列のメモ化を検討してください。
pathD1、pathD2、pathD3はwindowWidthとwindowHeightに依存していますが、レンダリングごとに再計算されています。dynamicStylesと同様にuseMemoでラップすることで、不要な文字列生成を避けられます。♻️ 提案する修正
+ const paths = useMemo( + () => ({ + pathD1: `M -4 -60 A ${windowWidth / 1.5} ${windowHeight} 0 0 1 ${ + windowWidth / 1.5 - 4 + } ${windowHeight}`, + pathD2: `M 0 -64 A ${windowWidth / 1.5} ${windowHeight} 0 0 1 ${ + windowWidth / 1.5 + } ${windowHeight}`, + pathD3: `M 0 -64 A ${windowWidth / 1.5} ${windowHeight} 0 0 1 ${ + windowWidth / 1.5 + } ${windowHeight}`, + }), + [windowWidth, windowHeight] + ); - const pathD1 = `M -4 -60 A ${windowWidth / 1.5} ${windowHeight} 0 0 1 ${ - windowWidth / 1.5 - 4 - } ${windowHeight}`; - const pathD2 = `M 0 -64 A ${windowWidth / 1.5} ${windowHeight} 0 0 1 ${ - windowWidth / 1.5 - } ${windowHeight}`; - const pathD3 = `M 0 -64 A ${windowWidth / 1.5} ${windowHeight} 0 0 1 ${ - windowWidth / 1.5 - } ${windowHeight}`;使用箇所では
paths.pathD1、paths.pathD2、paths.pathD3に変更してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PadArch.tsx` around lines 424 - 432, pathD1, pathD2 and pathD3 are reconstructed on every render even though they only depend on windowWidth and windowHeight; wrap their creation in a useMemo (similar to dynamicStyles) and return an object like paths = { pathD1, pathD2, pathD3 } so the strings are only recomputed when windowWidth or windowHeight change, and update all usages to read from paths.pathD1 / paths.pathD2 / paths.pathD3.
🤖 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/PadArch.tsx`:
- Around line 424-432: pathD1, pathD2 and pathD3 are reconstructed on every
render even though they only depend on windowWidth and windowHeight; wrap their
creation in a useMemo (similar to dynamicStyles) and return an object like paths
= { pathD1, pathD2, pathD3 } so the strings are only recomputed when windowWidth
or windowHeight change, and update all usages to read from paths.pathD1 /
paths.pathD2 / paths.pathD3.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/PadArch.tsx (1)
395-402:⚠️ Potential issue | 🟡 Minorコメントと依存配列の不整合を修正してください。
行396の
biome-ignoreコメントは「到着状態の変化時のみ再実行」と明示していますが、実際の依存配列にはwindowHeightが含まれています。これによりウィンドウリサイズ時にアニメーションが最初から再開されます。コメントの意図を実装に反映させるため、以下いずれかの対応が必要です:
windowHeightを依存配列から削除するか- コメントを「到着状態またはウィンドウサイズ変更時に再実行」に更新するか
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PadArch.tsx` around lines 395 - 402, The useEffect's comment says it should only re-run on arrival state changes but the dependency array includes windowHeight, causing restarts on resize; fix by removing windowHeight from the dependency array so the effect depends only on arrived (and fillHeight if needed), and update/keep the biome-ignore lint directive to reflect "再実行は到着状態の変化時のみ" (or alternatively, if you want resizes to trigger the animation, change the comment to mention "到着状態またはウィンドウサイズ変更時に再実行"); ensure references are to the useEffect that manipulates fillHeight.value with withTiming(windowHeight, { duration: YAMANOTE_LINE_BOARD_FILL_DURATION }) and to the arrived, fillHeight, and windowHeight symbols when applying the change.
🧹 Nitpick comments (1)
src/components/PadArch.tsx (1)
261-275:transferLines?.lengthのオプショナルチェーンは不要です。
transferLinesはLine[]型(非nullableの配列)として定義されているため、オプショナルチェーンは不要です。Line 304でも同様のパターンがあります。♻️ 修正提案
- const isMany = transferLines?.length > MANY_LINES_THRESHOLD; + const isMany = transferLines.length > MANY_LINES_THRESHOLD;Line 304も同様:
- if (!transferLines?.length) { + if (!transferLines.length) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PadArch.tsx` around lines 261 - 275, transferLinesは非nullableのLine[]なので、オプショナルチェーンを外して明示的に長さを参照してください; 具体的にはisManyの定義で使われているtransferLines?.lengthをtransferLines.lengthに置き換え、ファイル内の同様のパターン(コメントで指摘されているもう一箇所、transferLines?.lengthを使っている箇所)も同様に?.を削除してtransferLines.lengthを使うよう修正してください(変数名: transferLines、判定式: isMany)。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/PadArch.tsx`:
- Around line 395-402: The useEffect's comment says it should only re-run on
arrival state changes but the dependency array includes windowHeight, causing
restarts on resize; fix by removing windowHeight from the dependency array so
the effect depends only on arrived (and fillHeight if needed), and update/keep
the biome-ignore lint directive to reflect "再実行は到着状態の変化時のみ" (or alternatively,
if you want resizes to trigger the animation, change the comment to mention
"到着状態またはウィンドウサイズ変更時に再実行"); ensure references are to the useEffect that
manipulates fillHeight.value with withTiming(windowHeight, { duration:
YAMANOTE_LINE_BOARD_FILL_DURATION }) and to the arrived, fillHeight, and
windowHeight symbols when applying the change.
---
Nitpick comments:
In `@src/components/PadArch.tsx`:
- Around line 261-275:
transferLinesは非nullableのLine[]なので、オプショナルチェーンを外して明示的に長さを参照してください;
具体的にはisManyの定義で使われているtransferLines?.lengthをtransferLines.lengthに置き換え、ファイル内の同様のパターン(コメントで指摘されているもう一箇所、transferLines?.lengthを使っている箇所)も同様に?.を削除してtransferLines.lengthを使うよう修正してください(変数名:
transferLines、判定式: isMany)。
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
リリースノート
Refactor
Style