feat(drag): Add precision drag and slider commands#49
Merged
Conversation
Add a selector-based slider command that sets controls to a requested 0-100 percentage and verifies the settled accessibility value before reporting success. Cover the behavior with playground-backed e2e tests that assert the visible slider position after the command runs, and document the new command in the CLI docs and skill references. Co-Authored-By: Codex <noreply@openai.com>
Map requested values onto the slider thumb-center range instead of the raw accessibility frame width. This keeps deterministic slider setting aligned with the visible SwiftUI slider position. Co-Authored-By: Codex <noreply@openai.com>
Add a first-class drag command that sends a single low-level HID gesture from one point to another using touch-down, touch-move, and touch-up events. Reuse the same composite drag primitive from the slider command so slider movement no longer owns its own gesture builder. Add playground-backed E2E coverage that runs the drag command and verifies recorded start and end coordinates with describe-ui. Keep slider verification based on observable AXValue tolerance because SwiftUI slider values are quantized. Co-Authored-By: OpenAI Codex <noreply@openai.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Call the FBSimulatorHIDEvent touch move class method through its Objective-C selector so release builds do not depend on Swift importer spelling for the convenience method. Co-Authored-By: OpenAI Codex <noreply@openai.com>
WalkthroughThis pull request adds two new gesture commands to AXe: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🧹 Nitpick comments (2)
CHANGELOG.md (1)
12-13: 💤 Low valueConsider varying the sentence structure.
Both entries begin with "Added", which creates repetition. While this follows the subsection naming convention, you could rephrase slightly for better readability:
- "Added
axe slider ..." → "Introducedaxe slider ..." or "axe slider ...command for..."- "Added
axe drag ..." → "axe drag ...command for..."As per coding guidelines, entries are correctly placed under
## [Unreleased]with proper subsections### Addedand### Changed.🤖 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 `@CHANGELOG.md` around lines 12 - 13, Reword the two changelog bullets to avoid repeating "Added" by rephrasing them to e.g. "Introduced `axe slider --id/--label --value 0...100` for selector-based slider setting with orientation-aware HID dragging and AXValue tolerance verification/failure reporting" and "`axe drag --start-x/--start-y --end-x/--end-y` command for raw point-to-point low-level HID drag validation using explicit touch move events"; keep the same technical details and placement under the `### Added` subsection but update the sentence starts to introduce the commands rather than repeating "Added".Tests/DragTests.swift (1)
60-97: ⚡ Quick winAssert that the gesture actually contains move events.
waitForRecordedDragonly proves there was adownnear the start and anupnear the end. A regression that turnsdraginto a press/release pair would still pass, even though the command contract here is a continuous drag. Please also require at least one recordedmove:event for the same gesture.🤖 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 `@Tests/DragTests.swift` around lines 60 - 97, waitForRecordedDrag currently only checks for a "down" and an "up" which lets a press/release pass as a drag; update it to also require at least one "move" event for the same gesture before returning. Add a check (e.g., compute didSeeMove) by reusing the UIStateParser.findElement pattern in hasTouchEvent (or add a new helper like hasTouchMove(in:type:near:)) that looks for value.hasPrefix("move:") and parses coordinates via CoordinateParser.parseNamedCoordinates, and then require didSeeStart && didSeeMove && didSeeEnd inside waitForRecordedDrag before returning; include the move presence in the timeout error message as well.
🤖 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 `@Sources/AXe/Commands/Slider.swift`:
- Around line 18-19: The code uses hard-coded
lowRangeCoordinateOffset/highRangeCoordinateOffset with frame.x + frame.width *
normalized which mis-maps values because it ignores the thumb center; replace
that math with thumb-aware bounds: compute thumbRadius = frame.height / 2,
startX = frame.x + thumbRadius, endX = frame.x + frame.width - thumbRadius,
usableWidth = endX - startX, and map coordinate = startX + normalized *
usableWidth (and clamp normalized to [0,1]); remove or stop using
lowRangeCoordinateOffset/highRangeCoordinateOffset and apply this replacement
wherever the old formula is used (refer to occurrences around the mapping logic
that reference frame, normalized,
lowRangeCoordinateOffset/highRangeCoordinateOffset in Slider.swift).
In `@Sources/AXe/Utilities/HIDInteractor.swift`:
- Line 106: The code uses a non-existent FBSimulatorHIDEvent.touchMoveAt(x:y:)
in HIDInteractor; replace that call with a valid approach: either invoke
FBSimulatorHIDEvent.swipe(x:yStart:xEnd:yEnd:duration:) with appropriate
start/end coords and duration, or implement a short interpolation loop that
emits multiple FBSimulatorHIDEvent.touchDownAt(x:y:)/touchUpAt(x:y:) (or tapAt
for very short steps) between the start and end points to simulate drag motion;
update the code path that currently appends .touchMoveAt to instead append the
swipe event or the sequence of interpolated touch events so the code compiles
against FBSimulatorHIDEvent.
In `@test-runner.sh`:
- Around line 115-116: The case selector block that sets TEST_FILTER is missing
suites that the script runs later (causing ./test-runner.sh DescribeUITests to
fall through); update the case pattern (the branch that currently matches
BatchTests|SwipeTests|...) to include DescribeUITests (and any other suites the
sequential path runs) so TEST_FILTER="$1" is accepted for those suite names;
locate the case arm that assigns TEST_FILTER and add the missing suite
identifiers to the pipe-separated list to keep the selectable suite list in sync
with the sequential-run list.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 12-13: Reword the two changelog bullets to avoid repeating "Added"
by rephrasing them to e.g. "Introduced `axe slider --id/--label --value 0...100`
for selector-based slider setting with orientation-aware HID dragging and
AXValue tolerance verification/failure reporting" and "`axe drag
--start-x/--start-y --end-x/--end-y` command for raw point-to-point low-level
HID drag validation using explicit touch move events"; keep the same technical
details and placement under the `### Added` subsection but update the sentence
starts to introduce the commands rather than repeating "Added".
In `@Tests/DragTests.swift`:
- Around line 60-97: waitForRecordedDrag currently only checks for a "down" and
an "up" which lets a press/release pass as a drag; update it to also require at
least one "move" event for the same gesture before returning. Add a check (e.g.,
compute didSeeMove) by reusing the UIStateParser.findElement pattern in
hasTouchEvent (or add a new helper like hasTouchMove(in:type:near:)) that looks
for value.hasPrefix("move:") and parses coordinates via
CoordinateParser.parseNamedCoordinates, and then require didSeeStart &&
didSeeMove && didSeeEnd inside waitForRecordedDrag before returning; include the
move presence in the timeout error message as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffda1b8d-4f61-4f04-be55-4687a190f0e6
📒 Files selected for processing (19)
AxePlaygroundApp/AxePlayground/Views/AccessibilityFixturesView.swiftCHANGELOG.mdREADME.mdSkills/CLI/axe/SKILL.mdSkills/CLI/axe/references/cli-quick-reference.mdSources/AXe/Commands/Drag.swiftSources/AXe/Commands/Slider.swiftSources/AXe/Resources/skills/axe/SKILL.mdSources/AXe/Utilities/AccessibilityElement.swiftSources/AXe/Utilities/AccessibilityPoller.swiftSources/AXe/Utilities/AccessibilityTargetResolver.swiftSources/AXe/Utilities/HIDInteractor.swiftSources/AXe/main.swiftTests/AccessibilityTargetResolverTests.swiftTests/DragTests.swiftTests/README.mdTests/SliderTests.swiftUSAGE_EXAMPLES.mdtest-runner.sh
Avoid a spurious verification failure when the slider reaches the requested AXValue tolerance just before the stability delay crosses the polling deadline. Return the already-valid observation instead of sampling again after timeout. Co-Authored-By: OpenAI Codex <noreply@openai.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Add coverage for composite drag move planning so the drag path cannot collapse to a press-and-release-only implementation. Keep the app-level E2E drag coverage focused on observable simulator behavior. Sync test-runner single-suite handling with the sequential suite list so listed suites can be run directly. Co-Authored-By: OpenAI Codex <codex@openai.com>
Owner
Author
|
Addressed the actionable review feedback in 6302573.
|
Keep the calibrated slider overdrive needed to reach edge values, but clamp the final drag endpoint to the application frame so extreme commands cannot produce off-screen coordinates. Add focused coverage for endpoint clamping and preserve the playground-backed slider tests for 0 and 100 percent values. Co-Authored-By: OpenAI Codex <codex@openai.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Tolerance gap causes unnecessary overshooting drag then failure
- Set alreadyAtTargetTolerance to 0.0007 (matching valueTolerance) to prevent drags when slider is already within acceptable tolerance.
Or push these changes by commenting:
@cursor push 8770b5574c
Preview (8770b5574c)
diff --git a/Sources/AXe/Commands/Slider.swift b/Sources/AXe/Commands/Slider.swift
--- a/Sources/AXe/Commands/Slider.swift
+++ b/Sources/AXe/Commands/Slider.swift
@@ -13,7 +13,7 @@
private static let verificationTimeout: TimeInterval = 1.5
private static let verificationPollInterval: TimeInterval = 0.1
private static let verificationStabilityDelay: TimeInterval = 0.3
- private static let alreadyAtTargetTolerance = 0.00005
+ private static let alreadyAtTargetTolerance = 0.0007
private static let valueTolerance = 0.0007
private static let lowRangeCoordinateOffset = 0.0268
private static let highRangeCoordinateOffset = 0.0271You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit fa98cde. Configure here.
Use the same tolerance for already-at-target detection and final value verification so values that already pass verification do not trigger a calibrated overdrive drag. Add focused coverage for the within-tolerance command path. Co-Authored-By: OpenAI Codex <codex@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a first-class
axe dragcommand for deterministic point-to-point HID drags, then route the slider command through the same composite drag primitive. This gives AXe a direct way to prove raw drag precision separately from control-specific behavior.The new drag path emits one continuous gesture made from touch-down, touch-move, and touch-up events. The playground-backed drag test runs the CLI command and then uses
describe-uito verify the simulator actually recorded the requested start and end coordinates.Slider support remains selector-based: it resolves a slider by id or label, computes a single drag from the current AXValue toward the requested 0-100 percentage, performs that drag once, and verifies the observable AXValue. The docs avoid claiming arbitrary decimal exactness because SwiftUI sliders quantize observable values; the command verifies within a tight tolerance instead of retrying or micro-adjusting.
Note
Medium Risk
Adds new simulator interaction commands and introduces a new low-level HID drag primitive (including private
touchMoveinvocation) plus refactors accessibility resolution/polling, which could affect interaction reliability across commands and simulator versions.Overview
Adds new CLI commands:
axe dragfor deterministic point-to-point low-level HID drags (touch down/move/up with configurable duration/steps) andaxe sliderfor selector-based slider setting to a 0–100 target with post-drag AXValue tolerance verification.Introduces a shared composite drag path in
HIDInteractor(including explicit touch-move events) and updates accessibility utilities to support slider targeting (isSliderLikeControl), return richer selector matches (AccessibilityMatch), and reuse polling for both tap and non-tap element resolution.Expands Playground fixtures and E2E/unit tests to validate drag precision and slider value verification, and updates docs/skills/changelog/test runner to document and surface the new commands and timing flags.
Reviewed by Cursor Bugbot for commit 32f56f9. Bugbot is set up for automated code reviews on this repo. Configure here.