Skip to content

feat(screenshot): accept --crop-on on physical iOS devices - #3839

Open
camchis wants to merge 1 commit into
callstack:mainfrom
camchis:feat/ios-device-crop-on
Open

camchis wants to merge 1 commit into
callstack:mainfrom
camchis:feat/ios-device-crop-on

Conversation

@camchis

@camchis camchis commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Accept screenshot --crop-on <selector> on physical iOS devices. Adds a measured geometry fixture and updates tests, help and documentation: 7 files. Refs #2276.

agent-device screenshot out.png --crop-on 'label="Continue"' --platform ios --udid "$UDID"

The ios-physical cell also admits physical iPadOS. No iPad, including Split View, was verified.

Validation

Retested unmodified 7ffc88bedf4dfe7459961e5a7d6dc3b0e8fdd130 on 2026-10-19: clean checkout, freshly rebuilt CLI, isolated daemon/session, normal CoreDevice capture route; no forced-runner patch.

Device: physical iPhone 12 (iPhone13,2), iOS 27.0.1 (24A446), Xcode 26.6 (17F113), wired. App: fiddl gallery. Snapshot viewport: 390×844 pt; 23 objects bounds: approximately (255.6667, 73.6667, 54.3333, 14.3333) pt.

From that checkout, with $UDID selecting the test phone:

node bin/agent-device.mjs screenshot crop.png --crop-on 'label="23 objects"' --platform ios --udid "$UDID" --session pr3839 --state-dir /private/tmp/ad-reverify-3839-state --json
xcrun devicectl device capture screenshot --device "$UDID" --destination raw.png

Both exited 0. The CLI returned success: true, width 163, height 43. Independent raw capture: 1170×2532.

Comparing decoded pixels against raw origin (767, 221): all 7,009 RGBA pixels identical, maximum channel difference 0, mean absolute RGB difference 0. Controls: one row down 8.0689; one crop-height down 21.9517. Best ±4 px offset: (0, 0).

Existing pnpm check:affected --run --head 7ffc88be receipt: passed, Node 22.20, exactCommit=true. GitHub-only/manual lanes were skipped, not passed.

This replaces the provisional base-plus-local-change evidence. The legacy xctest device backend remains unverified.

View guided diff

@thymikee

Copy link
Copy Markdown
Member

The code looks right at 7ffc88b. The change is small: a matrix flag, docs, help text and tests. The one CI check is green, and it covers these files. There are no conflicts.

One question before this goes further: the ios-physical cell in https://github.com/callstack/agent-device/blob/7ffc88b/packages/daemon/src/execution/screenshot-crop-target.ts#L49 covers every physical iPhone and iPad, plus the XCTest-backed physical backend and the devicectl-to-runner fallback, but the live run covers one portrait iPhone 12 on the default devicectl route. On an iPad the largest viewport root can be the app window, so the crop could be wrong. Would you give iPadOS its own rejected cell, or word the help and docs as iPhone-only verified?

Not blocking, and you can take or leave these: the matrix test in https://github.com/callstack/agent-device/blob/7ffc88b/packages/daemon/src/execution/__tests__/screenshot-crop-target.test.ts#L63 hard-codes the accepted cells in an inline if-chain, so one shared list would keep it in sync, and the new fractional fixture adds rounding coverage but repeats the 3x law that the simulator case already pins.

I did not run the tests locally. I took the iPhone 12 numbers from the PR body and could not re-inspect that run. Landscape, iPad, Split View, the XCTest-backed backend, the devicectl-to-runner fallback and Appium-produced physical iOS trees were not exercised.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants