Fix iOS PathPicker closing/reopening while typing#78
Fix iOS PathPicker closing/reopening while typing#78happier-bot wants to merge 1 commit intohappier-dev:devfrom
Conversation
WalkthroughA new test file validates that header options remain stable across re-renders in PathPickerScreen, and the component implementation is refactored to use refs and derived state to maintain stable header actions and improve path resolution fallback handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
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 |
Greptile SummaryFixed iOS modal instability by stabilizing header options across keystrokes. The PathPicker modal was dismissing/reopening while typing because The fix uses a ref to capture the latest A comprehensive unit test validates that Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| apps/ui/sources/app/(app)/new/pick/path.tsx | Stabilized header options by using ref for customPath and depending on machineHomeDir primitive instead of full machine object; removed disabled state from confirm button |
| apps/ui/sources/tests/app/new/pick/path.headerOptions.stability.test.tsx | Added new unit test verifying headerRight remains referentially stable across keystroke-driven re-renders |
Last reviewed commit: 0b2e5e8
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/ui/sources/app/(app)/new/pick/path.tsx (1)
1-1: Inconsistent hook import style.
useStateanduseMemoare destructured from'react', whileuseRef,useEffect, anduseCallbackare accessed via theReact.namespace throughout the file. Consider making the style uniform — either destructure all used hooks on Line 1 or useReact.*consistently.Proposed fix
-import React, { useState, useMemo } from 'react'; +import React, { useState, useMemo, useRef, useEffect, useCallback } from 'react';Then replace
React.useRef,React.useEffect,React.useCallback, andReact.useMemo(line 118) with the direct names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/sources/app/`(app)/new/pick/path.tsx at line 1, The file mixes import styles: solve by updating the import line to destructure all hooks used (add useRef, useEffect, useCallback alongside useState and useMemo) and then replace React.useRef, React.useEffect, React.useCallback and the React.useMemo usage with the direct names useRef, useEffect, useCallback, and useMemo respectively so all hook calls use the same destructured import style; locate usages of React.useRef, React.useEffect, React.useCallback and the React.useMemo call around the component (e.g., where useState/useMemo are used) and update them to the direct identifiers.apps/ui/sources/__tests__/app/new/pick/path.headerOptions.stability.test.tsx (2)
49-49: Untypedanyin mock fixtures.Lines 49 and 58 use
anywithout justification. Per coding guidelines, broadas anycasts are forbidden except in boundary fixtures with a one-line rationale. These are mock fixtures, so they qualify for the exception, but consider adding a brief comment or narrowing the types.Example narrowing
- setParams: (params: any) => ({ type: 'SET_PARAMS', payload: { params } }), + setParams: (params: Record<string, unknown>) => ({ type: 'SET_PARAMS', payload: { params } }),- return { - useUnistyles: () => ({ theme }), - StyleSheet: { create: (input: any) => (typeof input === 'function' ? input(theme) : input) }, - }; + return { + useUnistyles: () => ({ theme }), + StyleSheet: { create: (input: unknown) => (typeof input === 'function' ? (input as (t: typeof theme) => unknown)(theme) : input) }, + };Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/sources/__tests__/app/new/pick/path.headerOptions.stability.test.tsx` at line 49, The mock fixture uses an untyped any for setParams (and the other mock at line 58); either narrow the type to the real params shape (e.g., a Params interface or unknown) on the setParams function signature, or if you deliberately allow a broad cast in this test fixture, add a one-line justification comment directly above the mock (e.g., "// test fixture: simplified broad cast allowed") and keep the cast limited to the fixture only; update the setParams declaration (and the other mock) accordingly so reviewers can locate and verify the change.
3-3:react-test-rendereris deprecated in React 19 — the entire test suite (20 tests) uses it, and@testing-libraryis not yet adopted in the project.The deprecation is soft (logs warnings in React 19, no removal yet), so a migration is a project-wide decision rather than a per-file concern. This test's use of
react-test-rendereris consistent with the existing codebase pattern and does not require immediate changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/sources/__tests__/app/new/pick/path.headerOptions.stability.test.tsx` at line 3, This test imports react-test-renderer via "import renderer, { act } from 'react-test-renderer';" which is known to be soft-deprecated in React 19; do not change the import or test logic here—leave the usage as-is to stay consistent with the existing suite, but add a short inline comment above the import noting the React 19 deprecation and that migration to `@testing-library` is a planned project-wide effort (add a TODO/ticket reference) so reviewers know this file was considered and intentionally left unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/ui/sources/__tests__/app/new/pick/path.headerOptions.stability.test.tsx`:
- Line 49: The mock fixture uses an untyped any for setParams (and the other
mock at line 58); either narrow the type to the real params shape (e.g., a
Params interface or unknown) on the setParams function signature, or if you
deliberately allow a broad cast in this test fixture, add a one-line
justification comment directly above the mock (e.g., "// test fixture:
simplified broad cast allowed") and keep the cast limited to the fixture only;
update the setParams declaration (and the other mock) accordingly so reviewers
can locate and verify the change.
- Line 3: This test imports react-test-renderer via "import renderer, { act }
from 'react-test-renderer';" which is known to be soft-deprecated in React 19;
do not change the import or test logic here—leave the usage as-is to stay
consistent with the existing suite, but add a short inline comment above the
import noting the React 19 deprecation and that migration to `@testing-library` is
a planned project-wide effort (add a TODO/ticket reference) so reviewers know
this file was considered and intentionally left unchanged.
In `@apps/ui/sources/app/`(app)/new/pick/path.tsx:
- Line 1: The file mixes import styles: solve by updating the import line to
destructure all hooks used (add useRef, useEffect, useCallback alongside
useState and useMemo) and then replace React.useRef, React.useEffect,
React.useCallback and the React.useMemo usage with the direct names useRef,
useEffect, useCallback, and useMemo respectively so all hook calls use the same
destructured import style; locate usages of React.useRef, React.useEffect,
React.useCallback and the React.useMemo call around the component (e.g., where
useState/useMemo are used) and update them to the direct identifiers.
Ports and hardens the bot-generated PR changes against current dev worktree. - #60: clarify default model label + prune Claude model suggestions - #64: centralize change_title aliases + normalize across UI/CLI providers - #71: document skipping capability probe on Codex ACP resume - #78: stabilize iOS PathPicker header options while typing - #83: remove synthetic stop+explain follow-up; unify stop button copy/i18n
1 participant
Root cause
On iOS (
containedModalpresentation), the PathPicker screen was re-creatingStack.Screenheader options on every keystroke becauseheaderRightdepended on input state (and a non-stablemachineobject identity). This could cause the modal to briefly dismiss/re-present, which remounted the screen and reset the TextInput value.Fix
customPathfrom a ref inside the confirm handlermachineHomeDir(a primitive) instead of the fullmachineobjectTests
headerRightremains stable across a keystroke-driven re-render.Ran:
NODE_ENV=development yarn -s test:unit sources/__tests__/app/new/pick/path.presentation.test.ts sources/__tests__/app/new/pick/path.headerOptions.stability.test.tsxSummary by CodeRabbit
Release Notes
Bug Fixes
Tests