Skip to content

feat(driver-mobilecli): surface isSelected/isChecked/isFocused from dump.ui - #252

Open
sjy wants to merge 1 commit into
mobile-next:mainfrom
sjy:driver-surface-selection-state
Open

feat(driver-mobilecli): surface isSelected/isChecked/isFocused from dump.ui#252
sjy wants to merge 1 commit into
mobile-next:mainfrom
sjy:driver-surface-selection-state

Conversation

@sjy

@sjy sjy commented Jul 29, 2026

Copy link
Copy Markdown

Summary

ViewNode already declares isSelected / isChecked / isFocused, and @mobilewright/core's expect ships toBeSelected() / toBeChecked() — but elementToViewNode never populated those fields, so the matchers read false for every node.

This maps the three flags through from the mobilecli element. Reads are guarded on typeof === 'boolean', so they stay undefined when the agent omits them — no behavior change until the on-device agent emits them.

Diff

  • MobilecliElement: add optional selected / checked / focused.
  • elementToViewNode: map them to isSelected / isChecked / isFocused.

Scope / follow-up

This is the driver half only. For the values to become non-undefined, the on-device agent must include selected / checked / focused (from AccessibilityNodeInfo) in each device.dump.ui element — and ideally Android stateDescription, which Jetpack Compose populates for a selected semantic on non-Role.Tab nodes. Tracking that separately.

Refs #250

Test

elementToViewNode is a private function with no unit-test seam today (only the live-server integration.test.ts), so no unit test is added here to avoid exporting internals. tsc -b builds clean. Happy to add a fixture/test if you'd prefer to expose it.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8306e2e2-b144-45f1-8af2-aca0ee0f05a5

📥 Commits

Reviewing files that changed from the base of the PR and between 738b36b and 4e3eda8.

📒 Files selected for processing (1)
  • packages/driver-mobilecli/src/driver.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/driver-mobilecli/src/driver.ts

Walkthrough

The mobile CLI element interface now supports optional selected, checked, and focused boolean fields. The elementToViewNode conversion maps these fields to isSelected, isChecked, and isFocused, leaving each value undefined when the source field is not explicitly boolean.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes do not address the directly linked issue #39 about removing sharding; they implement a different mobilecli state feature. Update the PR to satisfy issue #39's sharding-removal requirements, or relink the correct issue for this state-mapping work.
Out of Scope Changes check ⚠️ Warning All code changes are unrelated to the linked issue #39 and therefore appear out of scope for this PR. Remove the unrelated state-mapping changes or retarget the PR to the correct issue and scope.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change in driver-mobilecli state mapping.
Description check ✅ Passed The description matches the implemented change and explains the behavior and scope accurately.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sjy

sjy commented Jul 29, 2026

Copy link
Copy Markdown
Author

The CI run shows action_required — that's just the first-time-contributor approval gate; nothing has run yet. FWIW I ran the full CI pipeline locally against this change and it's green:

  • npm run build (tsc -b) — clean
  • npm run lint (tsc --noEmit && eslint .) — clean
  • npm test — 558 passed, 1 skipped

The change is additive (three optional fields + guarded mapping), so it's a no-op until the on-device agent emits selected/checked/focused. Happy to approve-and-run whenever a maintainer has a moment.

@gmegidish

Copy link
Copy Markdown
Member

@sjy hey! thanks for the pull request! I can't believe it was missing tbh. thank you for finding this. Can you please sign your commit so I can merge?

…ump.ui

The `ViewNode` protocol already declares `isSelected` / `isChecked` /
`isFocused`, and `@mobilewright/core`'s `expect` ships `toBeSelected()` /
`toBeChecked()`, but `elementToViewNode` never populated them — so those
matchers read `false` for every node.

Map the fields through from the mobilecli element (guarded on `typeof
=== 'boolean'`, so they stay `undefined` when the agent omits them — no
behavior change until the agent emits them).

The on-device agent must still include `selected`/`checked`/`focused` (and
ideally Android `stateDescription`) in each `device.dump.ui` element for these
to be non-undefined; tracking that separately.

Refs mobile-next#250
@sjy
sjy force-pushed the driver-surface-selection-state branch from 738b36b to 4e3eda8 Compare July 29, 2026 15:05
@sjy

sjy commented Jul 29, 2026

Copy link
Copy Markdown
Author

Done — the commit is now SSH-signed and shows Verified. Ready to merge whenever you are. Thanks for the quick look! 🙏

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