fix(tap): Support switch toggle activation#46
Merged
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
WalkthroughThis pull request adds comprehensive support for reliably tapping UISwitch and SwiftUI Toggle controls in the AXe iOS simulator CLI. The changes introduce a new 🚥 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. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AXe/Commands/Tap.swift (1)
8-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate help text now that taps don't always go to the centre.
The abstract on Line 8 ("tap its center") and the
--id/--label/--valuehelp strings on Lines 17, 20, 23 still describe the old behaviour. With the new resolver, switch-like targets are dispatched at the activation point rather than the geometric centre, so the documented behaviour and actual behaviour diverge for the most interesting case introduced by this PR.📝 Suggested wording
- abstract: "Tap on a specific point on the screen, or locate an element by accessibility and tap its center." + abstract: "Tap on a specific point on the screen, or locate an element by accessibility and tap its activation point (centre for most controls; trailing edge for switches/toggles)." @@ - `@Option`(name: [.customLong("id")], help: "Tap the center of the element matching AXUniqueId (accessibilityIdentifier). Ignored if -x and -y are provided.") + `@Option`(name: [.customLong("id")], help: "Tap the element matching AXUniqueId (accessibilityIdentifier) at its activation point. Ignored if -x and -y are provided.") @@ - `@Option`(name: [.customLong("label")], help: "Tap the center of the element matching AXLabel (accessibilityLabel). Ignored if -x and -y are provided.") + `@Option`(name: [.customLong("label")], help: "Tap the element matching AXLabel (accessibilityLabel) at its activation point. Ignored if -x and -y are provided.") @@ - `@Option`(name: [.customLong("value")], help: "Tap the center of the element matching AXValue (the current value of a control). Ignored if -x and -y are provided.") + `@Option`(name: [.customLong("value")], help: "Tap the element matching AXValue (the current value of a control) at its activation point. Ignored if -x and -y are provided.")🤖 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 `@Sources/AXe/Commands/Tap.swift` around lines 8 - 24, Update the help text in Tap.swift to reflect that element-targeted taps may go to the element's activation point (not always the geometric center): change the command abstract and the Option help strings for pointX/pointY (if needed) and for elementID, elementLabel, and elementValue to remove "center" and instead say "tap the element (at its activation point)" or similar; ensure the -x/-y help still documents coordinate-based tapping while the --id/--label/--value helps explain element-based taps use the element's activation point (or center when appropriate).
🧹 Nitpick comments (6)
Sources/AXe/Commands/Tap.swift (1)
140-163: 💤 Low valueUnreachable
.automaticbranch — consider an exhaustiveness-friendly shape.
resolvedTapStyle(for:)is documented and implemented to return only.simulatoror.physical, so the.automaticarm here is dead code that throws a runtimeCLIError. IfTapStyleever grows another case, the compiler would still be happy here, masking the regression.A small refactor that has the type system enforce the resolution would make this safer — e.g. let
resolvedTapStylereturn a private enumResolvedTapStyle { case simulator; case physical }, or useprecondition(false, ...)/fatalErrorto make the unreachable nature obvious.🤖 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 `@Sources/AXe/Commands/Tap.swift` around lines 140 - 163, The switch on resolvedTapStyle(for:) has a dead .automatic case; change resolvedTapStyle to return a private enum (e.g. private enum ResolvedTapStyle { case simulator, physical }) so the compiler enforces only those two possibilities, update resolvedTapStyle signature and its implementation to return ResolvedTapStyle, update callers (including this switch in Tap.swift) to switch over ResolvedTapStyle and remove the .automatic arm, and ensure HIDInteractor.performPhysicalTap and HIDInteractor.performHIDEvent call sites remain unchanged aside from using the new enum type.Sources/AXe/Utilities/HIDInteractor.swift (1)
77-122: 💤 Low valueLGTM with a minor note on the recovery flag.
The pre-delay → touch-down → hold → touch-up → post-delay sequence is correct and uses the shared
TapTiming.defaultHoldDuration, and the best-effort touch-up on failure prevents leaving the simulator in a stuck-down state.One small observation:
didTouchDownis only reset tofalseaftertouchUpEventsucceeds (Line 109), so if the touch-up itself throws, thecatchwill retry the same touch-up viatry?. It's harmless because of thetry?, but you could move the reset before the touch-up call (or use adefer-based guard) to make the intent — "only recover if we actually have an unbalanced down" — clearer.🤖 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 `@Sources/AXe/Utilities/HIDInteractor.swift` around lines 77 - 122, The catch concern is that didTouchDown is only set to false after the touch-up succeeds inside performPhysicalTap(in:session:logger:), making intent unclear if touchUp throws; update performPhysicalTap to reset didTouchDown before attempting the touchUp (or use a defer immediately after setting didTouchDown = true to clear it) so the recovery branch still only runs when a down actually occurred but the flag is not left set when touchUp itself errors; reference performPhysicalTap(at:preDelay:postDelay:in:session:logger:), the didTouchDown variable, and the touchUpEvent/performHIDEvent call to locate the change.Sources/AXe/Utilities/AccessibilityTargetResolver.swift (1)
100-112: 💤 Low valueConsider naming the activation-point heuristic constants.
The literals
100(wide-frame threshold) and31.0(trailing inset) encode iOS UISwitch/SwiftUI Toggle geometry but read as opaque magic numbers. Hoisting them to file/type-scopeprivate static letconstants with a one-line comment about the iOS switch widget geometry would make the heuristic easier to tune and document, and would let the tests reference the same constants if desired.♻️ Suggested extraction
struct AccessibilityTargetResolver { static let describeUITip = "Make sure the app is on the expected screen, then run `axe describe-ui --udid <SIMULATOR_UDID>` and prefer --id when available." + + /// Frames wider than this are treated as a row containing a trailing-edge + /// switch widget rather than a bare switch widget. + private static let wideSwitchFrameThreshold: Double = 100 + /// Approximate distance from the trailing edge of a wide switch row to the + /// centre of the iOS switch thumb/track. + private static let switchTrailingActivationInset: Double = 31.0 @@ - if element.isSwitchLikeControl, frame.width > 100 { - let switchTrailingActivationInset = 31.0 - return (x: frame.x + frame.width - switchTrailingActivationInset, y: centerY) - } + if element.isSwitchLikeControl, frame.width > wideSwitchFrameThreshold { + return (x: frame.x + frame.width - switchTrailingActivationInset, y: centerY) + }🤖 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 `@Sources/AXe/Utilities/AccessibilityTargetResolver.swift` around lines 100 - 112, The activationPoint(for:frame:) method uses opaque magic numbers 100 and 31.0; extract them into named file/type-scoped private static let constants (e.g., wideFrameThreshold and switchTrailingActivationInset) with a one-line comment describing that they reflect iOS UISwitch/SwiftUI Toggle geometry, then replace the literals in activationPoint and any tests to reference those constants; keep the existing behavior but improve readability and allow future tuning.Sources/AXe/Utilities/Batch/Command+BatchConvertible.swift (2)
26-55: 💤 Low valueRetry loop logging timing is slightly misleading.
On line 41 the log reads
"Element not found, retrying in \(context.pollInterval)s…"and is emitted before the sleep on every iteration, including iterations where the previous retry failed. Consider phrasing as"Element not found, waiting \(pollInterval)s before retry…"or moving the log to indicate a retry attempt count, so the message is unambiguous when investigating timing in verbose logs.Otherwise the retry/deadline logic looks correct: only
ElementResolutionError.isNotFoundtriggers retries, all other errors propagate immediately, andBatch.validate()already enforcespollInterval > 0whenwaitTimeout > 0, so this won't hot-loop.🤖 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 `@Sources/AXe/Utilities/Batch/Command`+BatchConvertible.swift around lines 26 - 55, The retry log message in resolveBatchTapPoint is misleading because logger.info().log("Element not found, retrying in \(context.pollInterval)s…") is emitted before sleeping on every loop; change the logging to either announce waiting (e.g., "Element not found, waiting \(context.pollInterval)s before retry…") or move the log to just after the sleep to indicate an actual retry attempt; update the logger invocation (logger.info().log) and keep use of context.pollInterval and the existing loop/deadline handling unchanged so only the phrasing/location of the log is modified.
110-119: 💤 Low valueDefensive
.automaticthrow is unreachable; consider tightening the type.
resolvedTapStyle(for:context:)is guaranteed to return only.physicalor.simulator. Thecase .automaticarm on line 117-119 exists only to satisfy exhaustiveness and throws a runtimeCLIErrorthat can never fire. Two cleaner options:
- Have
resolvedTapStylereturn a dedicated nestedenum ResolvedTapStyle { case physical, simulator }, eliminating the dead branch entirely.- Or, keep the current shape but downgrade the
.automaticarm toassertionFailure+ a sensible fallback (e.g..simulator), so a future reviewer isn't misled into thinking this is reachable.Not blocking — the current code is correct — just a small clarity improvement.
♻️ Suggested refactor using a narrower return type
+ private enum ResolvedTapStyle { + case physical + case simulator + } + - private func resolvedTapStyle(for resolution: TapResolution, context: BatchContext) -> TapStyle { + private func resolvedTapStyle(for resolution: TapResolution, context: BatchContext) -> ResolvedTapStyle { let requestedStyle = tapStyle ?? context.tapStyle switch requestedStyle { case .automatic: return resolution.isSwitchLikeControl ? .physical : .simulator case .simulator: return .simulator case .physical: return .physical } }Then the
switch styleat the call site only needs.physicaland.simulatorarms.🤖 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 `@Sources/AXe/Utilities/Batch/Command`+BatchConvertible.swift around lines 110 - 119, The switch handling the result of resolvedTapStyle(for:context:) includes an unreachable .automatic arm; tighten the return type or make the branch a debug-only fallback. Option A: change resolvedTapStyle(for:context:) to return a narrower enum (e.g., ResolvedTapStyle { case physical, simulator }) so the caller only switches on .physical and .simulator and remove the .automatic case. Option B: keep the current signature but replace the .automatic throw with an assertionFailure and a sensible fallback (e.g., treat as .simulator) so the code still returns the HID/physical tap via .physicalTap(...) or .hidMergeable(buildDelayedEvent(preDelay:mainEvent:postDelay:)) while surfacing a debug-time failure via assertionFailure. Ensure callers using FBSimulatorHIDEvent.tapAt(...) and buildDelayedEvent(...) continue to receive correct events.Tests/BatchTests.swift (1)
159-164: 💤 Low valueUse the local
waitForLabelhelper for consistency.These new tests call
TestHelpers.waitForLabel, but all other tests in this file use the privatewaitForLabelhelper defined at line 218. For consistency with the existing test suite, prefer the local helper here (or migrate the entire file to useTestHelpers.waitForLabeland remove the local helper).🤖 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/BatchTests.swift` around lines 159 - 164, Replace the two calls to TestHelpers.waitForLabel with the file-local private helper waitForLabel to keep consistency with other tests: change usages of TestHelpers.waitForLabel(...) to waitForLabel(...) for the swiftUIState and uiKitState assignments (or alternatively migrate all other calls to TestHelpers.waitForLabel and remove the private waitForLabel helper), ensuring the same arguments and timeout behavior are preserved when updating the references.
🤖 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 `@CHANGELOG.md`:
- Line 12: The changelog bullet about selector-based `tap` and batch tap steps
(mentioning `UISwitch`, SwiftUI `Toggle`, and `--tap-style`) must be rewritten
to follow the repo contribution format: if this is an internal change reference
it as "Fixed selector-based tap and batch tap steps so UIKit UISwitch and
SwiftUI Toggle controls can be activated reliably (#[ISSUE])" with the issue
link; if it’s an external PR reference it as "Added --tap-style so switch/toggle
taps can use physical touch automatically (#[PR] by [`@username`])" with the PR
link and author—update the line to include the correct bracketed tracking number
and attribution per the guidelines.
In `@Sources/AXe/Utilities/AccessibilityTargetResolver.swift`:
- Around line 27-28: The message returned from the enum case
multipleSwitchDescendants in AccessibilityTargetResolver.swift suggests using
`--element-type Switch`, but `isSwitchLikeControl` accepts more than `type ==
"Switch"` (e.g., CheckBox with `AXSwitch` subrole), so change the wording to
prioritize `--id` and coordinates and make `--element-type` an optional
suggestion only when applicable; update the string returned for case
.multipleSwitchDescendants(let count, let selectorDescription) to mention
targeting by `--id` or coordinates first, and either remove or qualify the
`--element-type Switch` suggestion (or add a clause that `--element-type` only
helps when the element’s type is actually Switch) so the tip aligns with the
behavior of `isSwitchLikeControl`.
---
Outside diff comments:
In `@Sources/AXe/Commands/Tap.swift`:
- Around line 8-24: Update the help text in Tap.swift to reflect that
element-targeted taps may go to the element's activation point (not always the
geometric center): change the command abstract and the Option help strings for
pointX/pointY (if needed) and for elementID, elementLabel, and elementValue to
remove "center" and instead say "tap the element (at its activation point)" or
similar; ensure the -x/-y help still documents coordinate-based tapping while
the --id/--label/--value helps explain element-based taps use the element's
activation point (or center when appropriate).
---
Nitpick comments:
In `@Sources/AXe/Commands/Tap.swift`:
- Around line 140-163: The switch on resolvedTapStyle(for:) has a dead
.automatic case; change resolvedTapStyle to return a private enum (e.g. private
enum ResolvedTapStyle { case simulator, physical }) so the compiler enforces
only those two possibilities, update resolvedTapStyle signature and its
implementation to return ResolvedTapStyle, update callers (including this switch
in Tap.swift) to switch over ResolvedTapStyle and remove the .automatic arm, and
ensure HIDInteractor.performPhysicalTap and HIDInteractor.performHIDEvent call
sites remain unchanged aside from using the new enum type.
In `@Sources/AXe/Utilities/AccessibilityTargetResolver.swift`:
- Around line 100-112: The activationPoint(for:frame:) method uses opaque magic
numbers 100 and 31.0; extract them into named file/type-scoped private static
let constants (e.g., wideFrameThreshold and switchTrailingActivationInset) with
a one-line comment describing that they reflect iOS UISwitch/SwiftUI Toggle
geometry, then replace the literals in activationPoint and any tests to
reference those constants; keep the existing behavior but improve readability
and allow future tuning.
In `@Sources/AXe/Utilities/Batch/Command`+BatchConvertible.swift:
- Around line 26-55: The retry log message in resolveBatchTapPoint is misleading
because logger.info().log("Element not found, retrying in
\(context.pollInterval)s…") is emitted before sleeping on every loop; change the
logging to either announce waiting (e.g., "Element not found, waiting
\(context.pollInterval)s before retry…") or move the log to just after the sleep
to indicate an actual retry attempt; update the logger invocation
(logger.info().log) and keep use of context.pollInterval and the existing
loop/deadline handling unchanged so only the phrasing/location of the log is
modified.
- Around line 110-119: The switch handling the result of
resolvedTapStyle(for:context:) includes an unreachable .automatic arm; tighten
the return type or make the branch a debug-only fallback. Option A: change
resolvedTapStyle(for:context:) to return a narrower enum (e.g., ResolvedTapStyle
{ case physical, simulator }) so the caller only switches on .physical and
.simulator and remove the .automatic case. Option B: keep the current signature
but replace the .automatic throw with an assertionFailure and a sensible
fallback (e.g., treat as .simulator) so the code still returns the HID/physical
tap via .physicalTap(...) or
.hidMergeable(buildDelayedEvent(preDelay:mainEvent:postDelay:)) while surfacing
a debug-time failure via assertionFailure. Ensure callers using
FBSimulatorHIDEvent.tapAt(...) and buildDelayedEvent(...) continue to receive
correct events.
In `@Sources/AXe/Utilities/HIDInteractor.swift`:
- Around line 77-122: The catch concern is that didTouchDown is only set to
false after the touch-up succeeds inside performPhysicalTap(in:session:logger:),
making intent unclear if touchUp throws; update performPhysicalTap to reset
didTouchDown before attempting the touchUp (or use a defer immediately after
setting didTouchDown = true to clear it) so the recovery branch still only runs
when a down actually occurred but the flag is not left set when touchUp itself
errors; reference performPhysicalTap(at:preDelay:postDelay:in:session:logger:),
the didTouchDown variable, and the touchUpEvent/performHIDEvent call to locate
the change.
In `@Tests/BatchTests.swift`:
- Around line 159-164: Replace the two calls to TestHelpers.waitForLabel with
the file-local private helper waitForLabel to keep consistency with other tests:
change usages of TestHelpers.waitForLabel(...) to waitForLabel(...) for the
swiftUIState and uiKitState assignments (or alternatively migrate all other
calls to TestHelpers.waitForLabel and remove the private waitForLabel helper),
ensuring the same arguments and timeout behavior are preserved when updating the
references.
🪄 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: 722af337-94b1-4fd5-8c5f-3f1b594e6909
📒 Files selected for processing (26)
AxePlaygroundApp/AxePlayground/ContentView.swiftAxePlaygroundApp/AxePlayground/Views/SwitchTestView.swiftBATCHING.mdCHANGELOG.mdREADME.mdSkills/CLI/axe/SKILL.mdSkills/CLI/axe/references/batch-reference.mdSkills/CLI/axe/references/cli-quick-reference.mdSources/AXe/Commands/Batch.swiftSources/AXe/Commands/Tap.swiftSources/AXe/Commands/Touch.swiftSources/AXe/Resources/skills/axe/SKILL.mdSources/AXe/Types/TapStyle.swiftSources/AXe/Utilities/AccessibilityElement.swiftSources/AXe/Utilities/AccessibilityPoller.swiftSources/AXe/Utilities/AccessibilityTargetResolver.swiftSources/AXe/Utilities/Batch/BatchContext.swiftSources/AXe/Utilities/Batch/BatchPlan.swiftSources/AXe/Utilities/Batch/BatchPlanRunner.swiftSources/AXe/Utilities/Batch/Command+BatchConvertible.swiftSources/AXe/Utilities/HIDInteractor.swiftTests/AccessibilityTargetResolverTests.swiftTests/BatchTests.swiftTests/TapTests.swiftTests/TestUtilities.swiftUSAGE_EXAMPLES.md
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Sibling switch logic redirects taps from actionable elements
- Added guard to check if matched element is actionable before attempting sibling switch lookup, preventing incorrect redirection while preserving intended behavior for non-actionable labels.
Or push these changes by commenting:
@cursor push 76dfef03bf
Preview (76dfef03bf)
diff --git a/Sources/AXe/Utilities/AccessibilityTargetResolver.swift b/Sources/AXe/Utilities/AccessibilityTargetResolver.swift
--- a/Sources/AXe/Utilities/AccessibilityTargetResolver.swift
+++ b/Sources/AXe/Utilities/AccessibilityTargetResolver.swift
@@ -174,6 +174,10 @@
return switchDescendants[0]
}
+ if matchedElement.isActionable {
+ return matchedElement
+ }
+
if let ancestor = nearestAncestor(of: matchedElement, in: roots) {
let siblingSwitches = ancestor.switchLikeDescendantsIncludingSelf()
if siblingSwitches.count == 1 {You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Sibling switch redirection causes test assertion failure
- Added query type awareness to selectActivationElement so sibling switch redirection only applies to label queries, not value or id queries.
- ✅ Fixed: Duplicated polling logic between batch and direct tap
- Extracted polling retry logic into shared AccessibilityPoller.pollForResolution function with root-fetching closure to eliminate duplication.
Or push these changes by commenting:
@cursor push 76246a431e
Preview (76246a431e)
diff --git a/Sources/AXe/Utilities/AccessibilityPoller.swift b/Sources/AXe/Utilities/AccessibilityPoller.swift
--- a/Sources/AXe/Utilities/AccessibilityPoller.swift
+++ b/Sources/AXe/Utilities/AccessibilityPoller.swift
@@ -10,7 +10,26 @@
elementType: String? = nil,
logger: AxeLogger
) async throws -> TapResolution {
- let roots = try await AccessibilityFetcher.fetchAccessibilityElements(for: simulatorUDID, logger: logger)
+ try await pollForResolution(
+ query: query,
+ waitTimeout: waitTimeout,
+ pollInterval: pollInterval,
+ elementType: elementType,
+ logger: logger
+ ) {
+ try await AccessibilityFetcher.fetchAccessibilityElements(for: simulatorUDID, logger: logger)
+ }
+ }
+
+ static func pollForResolution(
+ query: AccessibilityQuery,
+ waitTimeout: TimeInterval,
+ pollInterval: TimeInterval,
+ elementType: String?,
+ logger: AxeLogger,
+ rootsFetcher: () async throws -> [AccessibilityElement]
+ ) async throws -> TapResolution {
+ let roots = try await rootsFetcher()
do {
return try AccessibilityTargetResolver.resolveTap(roots: roots, query: query, elementType: elementType)
} catch let error as ElementResolutionError where error.isNotFound && waitTimeout > 0 {
@@ -22,7 +41,7 @@
logger.info().log("Element not found, retrying in \(pollInterval)s…")
try await Task.sleep(for: .seconds(pollInterval))
- let freshRoots = try await AccessibilityFetcher.fetchAccessibilityElements(for: simulatorUDID, logger: logger)
+ let freshRoots = try await rootsFetcher()
do {
return try AccessibilityTargetResolver.resolveTap(roots: freshRoots, query: query, elementType: elementType)
} catch let retryError as ElementResolutionError where retryError.isNotFound {
diff --git a/Sources/AXe/Utilities/AccessibilityTargetResolver.swift b/Sources/AXe/Utilities/AccessibilityTargetResolver.swift
--- a/Sources/AXe/Utilities/AccessibilityTargetResolver.swift
+++ b/Sources/AXe/Utilities/AccessibilityTargetResolver.swift
@@ -4,6 +4,15 @@
case id(String)
case label(String)
case value(String)
+
+ var allowsSiblingRedirection: Bool {
+ switch self {
+ case .label:
+ return true
+ case .id, .value:
+ return false
+ }
+ }
}
enum ElementResolutionError: LocalizedError {
@@ -81,7 +90,8 @@
let activationElement = try selectActivationElement(
from: matchedElement,
roots: roots,
- selectorDescription: selectorDescription
+ selectorDescription: selectorDescription,
+ allowSiblingRedirection: query.allowsSiblingRedirection
)
guard let frame = activationElement.frame else {
@@ -157,7 +167,8 @@
private static func selectActivationElement(
from matchedElement: AccessibilityElement,
roots: [AccessibilityElement],
- selectorDescription: String
+ selectorDescription: String,
+ allowSiblingRedirection: Bool
) throws -> AccessibilityElement {
if matchedElement.isSwitchLikeControl {
return matchedElement
@@ -178,7 +189,7 @@
return matchedElement
}
- if let ancestor = nearestAncestor(of: matchedElement, in: roots) {
+ if allowSiblingRedirection, let ancestor = nearestAncestor(of: matchedElement, in: roots) {
let siblingSwitches = ancestor.switchLikeDescendantsIncludingSelf()
if siblingSwitches.count == 1 {
return siblingSwitches[0]
diff --git a/Sources/AXe/Utilities/Batch/Command+BatchConvertible.swift b/Sources/AXe/Utilities/Batch/Command+BatchConvertible.swift
--- a/Sources/AXe/Utilities/Batch/Command+BatchConvertible.swift
+++ b/Sources/AXe/Utilities/Batch/Command+BatchConvertible.swift
@@ -29,28 +29,17 @@
elementType: String?,
logger: AxeLogger
) async throws -> TapResolution {
- let roots = try await context.accessibilityRoots(logger: logger)
- do {
- return try AccessibilityTargetResolver.resolveTap(roots: roots, query: query, elementType: elementType)
- } catch let error as ElementResolutionError where error.isNotFound && context.waitTimeout > 0 {
- let clock = ContinuousClock()
- let deadline = clock.now + .seconds(context.waitTimeout)
-
- var lastError = error
- while clock.now < deadline {
- logger.info().log("Element not found, retrying in \(context.pollInterval)s…")
- try await Task.sleep(for: .seconds(context.pollInterval))
-
- let freshRoots = try await context.accessibilityRoots(logger: logger, forceRefresh: true)
- do {
- return try AccessibilityTargetResolver.resolveTap(roots: freshRoots, query: query, elementType: elementType)
- } catch let retryError as ElementResolutionError where retryError.isNotFound {
- lastError = retryError
- continue
- }
- }
-
- throw lastError
+ var isFirstFetch = true
+ return try await AccessibilityPoller.pollForResolution(
+ query: query,
+ waitTimeout: context.waitTimeout,
+ pollInterval: context.pollInterval,
+ elementType: elementType,
+ logger: logger
+ ) {
+ let forceRefresh = !isFirstFetch
+ isFirstFetch = false
+ return try await context.accessibilityRoots(logger: logger, forceRefresh: forceRefresh)
}
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 681ac45. Configure here.
Recognize UIKit and SwiftUI switch-like controls from richer accessibility metadata and resolve selector taps to the switch activation point. Add an automatic tap style that keeps normal taps on simulator tapAt while using physical touch down/up for switch and toggle activations. Mirror the behavior in batch taps and add playground coverage for SwiftUI Toggle and UIKit UISwitch controls. Co-Authored-By: Codex <noreply@openai.com>
Add guard to prevent sibling switch lookup when matched element is already actionable (Button, Link, etc). This ensures explicit --id or --label targeting of actionable elements is respected, while preserving sibling switch behavior for non-actionable labels. Applied via @cursor push command
Elements with same type/frame but nil labels/values are now distinguished by role, roleDescription, and subrole properties to prevent incorrect identity matches in nearestAncestor tree traversal. Co-authored-by: Cameron Cooke <web@cameroncooke.com>
…ed polling logic - Only apply sibling switch redirection for label-based queries, not value or id queries - Extract polling retry logic into shared AccessibilityPoller.pollForResolution function - Fixes test assertion failure in elementsWithSameTypeAndFrameButDifferentRolesAreDistinct - Eliminates code duplication between batch and direct tap polling paths Applied via @cursor push command
cameroncooke
force-pushed
the
cameroncooke/fix/switch-toggle-tap-style
branch
from
11d6588 to
bb4b6f9
Compare
Tighten switch activation resolution by using stable AXUniqueId identity, limiting sibling switch detection to direct children, and clarifying ambiguous selector guidance. Update tap help text, changelog attribution, and regression coverage for identifier collisions and nested sibling switch lookup. Refs GH-46 Co-Authored-By: Codex <noreply@openai.com>
Owner
Author
|
Follow-up from review comments, fixed in e373973:
I left the lower-value type-shape/logging/refactor suggestions out of this PR because they are not required for the switch activation fix. |
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
2 participants
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.
Fix selector-based switch and toggle activation in AXe.
Weather-style switch controls can appear as wide accessibility rows or as
CheckBoxelements with switch role metadata, so resolving to the geometric center can miss the actual activation target. This updates tap resolution to recognize richer switch metadata, choose the switch activation point, and handle both descendant and sibling switch layouts.Normal taps keep the existing simulator
tapAtpath by default. The new--tap-style automaticbehavior only switches to physical touch down/up for matched switch/toggle activations, with--tap-style simulatorand--tap-style physicalavailable for explicit control. Batch tap steps mirror direct tap semantics and can be configured at either the batch or step level.Adds AxePlayground coverage for SwiftUI
Toggleand UIKitUISwitch, plus resolver and E2E regression tests. The docs, changelog, and bundled AXe skill references are updated for the new behavior.Note
Medium Risk
Changes core tap dispatch and accessibility resolution logic (including new physical touch path and new resolution heuristics), which can affect interaction correctness across many UI elements and batch flows.
Overview
Selector-based
tapnow resolves an activation point (not just element center) and reliably redirects row/label matches to a single contained or sibling switch/toggle when present. Accessibility decoding/normalization is expanded (e.g.,Toggle, switch role metadata,AXIdentifierfallback) and element comparison is tightened to avoid misidentifying distinct ancestors.Adds
--tap-style(automatic|simulator|physical) totapandbatchso switch/toggle activations default to physical touch down/up while normal taps continue using simulatortapAt; batch plans gain aphysicalTapprimitive and runner support.Adds a new Playground
SwitchTestViewplus unit + E2E tests covering switch/toggle activation and tap-style overrides, and updates docs/skill references/README/changelog to reflect the new behavior.Reviewed by Cursor Bugbot for commit e373973. Bugbot is set up for automated code reviews on this repo. Configure here.