feat: support iOS simulator transform gesture#586
Conversation
|
2a1dda9 to
f6ff245
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a1dda9fca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
6db01a0 to
c3cbc62
Compare
thymikee
left a comment
There was a problem hiding this comment.
Reviewed the iOS simulator transform changes. Using XCSynthesizedEventRecord/XCPointerEventPath is a nice step up from the old pan+pinch+rotate decomposition, and the fallback to .unsupported(message) when the private API throws is the right call.
Main concerns:
-
The implementation is not one continuous two-finger gesture: pan+scale runs through the synthesized path (which ends with
liftUpAtOffset:), and then rotation runs as a separateXCUIElement.rotate(...). That's at odds with the iOS smoke-suite contract you added ("Need one continuous two-finger gesture without lifting fingers"). Either:- Fold the rotation into the pointer path geometry (the Android helper already does this — interpolate
θ(t)and place fingers at(centerX + cos(θ)*radius, centerY + sin(θ)*radius)), or - Update the contract + docs to acknowledge that rotation is a follow-up phase.
- Fold the rotation into the pointer path geometry (the Android helper already does this — interpolate
-
Hardcoded pinch radius (80 pt) isn't tunable and may not work well on iPad / for
scale < 0.5. Either parameterize it or scale it off the app frame. -
Robustness for private API drift: explicit
nilchecks onrecordClass/pathClassand on eachNSSelectorFromStringwould give clearer "Xcode version broke this" messages instead of anobjc_msgSendcrash. -
Typedef width:
RunnerMsgSendLongLongclaimslong long, butprocessID/interfaceOrientationareNSInteger. Works today on LP64 but the typedef should match the selector signature. -
Codex bot's claim about the loop ending at
t < 1.0appears to be incorrect (the loop is<=). Suggest resolving that thread.
Nothing here is a hard block; the rotation-as-followup vs. atomic-gesture question is the one most worth aligning on before merge.
Generated by Claude Code
cf87b48 to
8f9c55b
Compare
8f9c55b to
fdc928f
Compare
Summary
Add iOS simulator support for
gesture transform.This wires transform to a private XCTest synthesized multi-touch path for pan/scale, applies XCTest rotation for the rotation component, removes the old handler-level decomposition helper, and updates help/docs plus the command-planning coverage.
Closes #582
Validation
Ran
pnpm format,pnpm build,pnpm build:xcuitest,pnpm exec vitest run src/utils/__tests__/args.test.ts,pnpm check:quick,git diff --check, and five transform math sanity variations.Manual verification on iPhone 17 with Metro on
8082:gesture transform 201 421 80 -40 2 35 700changed the app metrics fromx 0, y 0, scale 1.00, rotate 0with all change flagsnotox 57, y -28, scale 1.56, rotate 23with pan/pinch/rotate allyes. SkillGym was not run per request.