Delay picker and select opens until the keyboard dismisses#143
Conversation
📝 WalkthroughWalkthroughThe change adds keyboard-dismiss coordination to the new entry form in ChangesKeyboard-Aware Modal Opening in Entry Form
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/entry/new.tsx`:
- Around line 300-333: The openAfterKeyboardDismiss function queues modal opens
that may execute after the user navigates to a different step, causing stale
callbacks to modify the wrong step's state. You need to call
clearPendingModalOpen() before all step-changing operations (anywhere setStep is
called in this component) to ensure pending modal opens are cancelled when the
user navigates away. This prevents the queued open callback from executing after
the step context has changed and setting pickerTarget or selectTarget in the
wrong step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| useEffect(() => clearPendingModalOpen, [clearPendingModalOpen]); | ||
|
|
||
| const openAfterKeyboardDismiss = useCallback( | ||
| (open: () => void) => { | ||
| clearPendingModalOpen(); | ||
| const isKeyboardVisible = Keyboard.isVisible(); | ||
|
|
||
| if (!isKeyboardVisible) { | ||
| Keyboard.dismiss(); | ||
| open(); | ||
| return; | ||
| } | ||
|
|
||
| let didOpen = false; | ||
| const finishOpen = () => { | ||
| if (didOpen) return; | ||
| didOpen = true; | ||
| clearPendingModalOpen(); | ||
| keyboardDismissFrameRef.current = requestAnimationFrame(() => { | ||
| keyboardDismissFrameRef.current = null; | ||
| open(); | ||
| }); | ||
| }; | ||
|
|
||
| keyboardHideSubscriptionRef.current = Keyboard.addListener( | ||
| "keyboardDidHide", | ||
| finishOpen, | ||
| ); | ||
| keyboardDismissFallbackRef.current = setTimeout( | ||
| finishOpen, | ||
| KEYBOARD_DISMISS_FALLBACK_MS, | ||
| ); | ||
| Keyboard.dismiss(); | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cancel queued opens before changing steps.
Line 302 queues the modal open outside React state, but step transitions still call setStep(...) without clearing that queue. If the keyboard is hiding and the user taps Back/Weiter before keyboardDidHide or the fallback fires, the stale open() can set pickerTarget/selectTarget after the UI has moved to another step.
🐛 Proposed fix
const handleBack = useCallback(() => {
+ clearPendingModalOpen();
+
if (selectTarget) {
setSelectTarget(null);
return true;
}
@@
goBackOrReplace(router, "/home");
return true;
-}, [pickerTarget, router, selectTarget, step]);
+}, [clearPendingModalOpen, pickerTarget, router, selectTarget, step]);Also clear before other step-changing paths:
if (isHomework) {
+ clearPendingModalOpen();
setStep("success");
return;
}
@@
if (step === "basics") {
+ clearPendingModalOpen();
setStep("planning");
return;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/entry/new.tsx` around lines 300 - 333, The openAfterKeyboardDismiss
function queues modal opens that may execute after the user navigates to a
different step, causing stale callbacks to modify the wrong step's state. You
need to call clearPendingModalOpen() before all step-changing operations
(anywhere setStep is called in this component) to ensure pending modal opens are
cancelled when the user navigates away. This prevents the queued open callback
from executing after the step context has changed and setting pickerTarget or
selectTarget in the wrong step.
2 participants
Summary
Issue
Fixes #128
Testing
pnpm typecheckpnpm lintgit diff --checkcoderabbit review --agent -t uncommitted -c AGENTS.mdraised 0 issuesManual Verification
Notizen->Schulfachkeyboard overlap flow.Summary by CodeRabbit