CustomModalにonShowコールバックを追加しiOSフォーカス遅延のマジックナンバーを除去#5575
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughモーダル表示後のフォーカス処理を、固定300msタイマーから CustomModal の新しい Changes
Sequence Diagram(s)sequenceDiagram
participant SaveModal as SavePresetNameModal
participant Modal as CustomModal
participant Input as TextInput
rect rgba(135,206,235,0.5)
SaveModal->>Modal: open()
Modal-->>Modal: run show animation
Modal->>SaveModal: onShow() -- after animation completes
end
SaveModal->>Input: focus()
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/SavePresetNameModal.tsx (1)
113-117:180msはCustomModal側と共有したいです。ここで再ハードコードすると、モーダルの duration を次に変えたときにまたズレます。共通定数か「表示完了」コールバック経由にしておくと保守しやすいです。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SavePresetNameModal.tsx` around lines 113 - 117, The focus timing uses a hardcoded 180ms (in the Platform.OS === 'ios' block where textInputRef.current?.focus() is delayed), which should be synchronized with CustomModal; remove the magic number and either import and use a shared constant (e.g., MODAL_ANIMATION_DURATION from CustomModal) or, better, wire an onShow/onAnimationEnd callback prop on CustomModal and call textInputRef.current?.focus() from that handler to guarantee focus happens when the modal animation completes (referencing textInputRef, CustomModal, and the Platform.OS conditional as needed).
🤖 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/SavePresetNameModal.tsx`:
- Around line 113-117: The focus timing uses a hardcoded 180ms (in the
Platform.OS === 'ios' block where textInputRef.current?.focus() is delayed),
which should be synchronized with CustomModal; remove the magic number and
either import and use a shared constant (e.g., MODAL_ANIMATION_DURATION from
CustomModal) or, better, wire an onShow/onAnimationEnd callback prop on
CustomModal and call textInputRef.current?.focus() from that handler to
guarantee focus happens when the modal animation completes (referencing
textInputRef, CustomModal, and the Platform.OS conditional as needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bba2bfe5-3f8c-4a81-adf7-bff268c0c0f8
📒 Files selected for processing (1)
src/components/SavePresetNameModal.tsx
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/CustomModal.tsx`:
- Around line 78-82: The effect currently depends on the onShow prop and can
re-run when a new function reference is passed even if visible stays true;
change to store the latest onShow in a ref (e.g., const onShowRef =
useRef(onShow)) and update it in a small useEffect that runs when onShow
changes, then remove onShow from the animation effect's dependency array so it
only depends on visible; inside the animation callback (the start(({ finished })
=> { ... }) block) call onShowRef.current?.() instead of onShow() so the latest
handler is invoked without retriggering the effect when parent passes a new
function reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 654e167e-9747-4174-aa84-1cc091a643b4
📒 Files selected for processing (2)
src/components/CustomModal.tsxsrc/components/SavePresetNameModal.tsx
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
CustomModalにonShowコールバック prop を追加(開くアニメーション完了時に発火)SavePresetNameModalのsetTimeout(() => focus(), 180)を削除し、onShow経由でフォーカスするよう変更onShow/onCloseAnimationEndを ref パターンに変更し、親の再レンダーによる不要なエフェクト再実行を防止Test plan
autoFocusによる従来のフォーカス動作が維持されていることを確認🤖 Generated with Claude Code