Adding support for accessibility custom actions #1295

RoyalPineapple · 2024-07-09T18:40:10Z

Custom actions allow developers to provide custom interactions that can be initiated through assistive technologies like VoiceOver and Switch Control. For example, VoiceOver lets users access actions quickly using the Actions rotor.
Custom actions are relied on heavily by SwiftUI's .accessibilityElement(children:.combine) view modifier.

This PR introduces two methods on KIFUIViewTestActor to enable validation and activation of an element's custom actions within KIF tests.

- (instancetype)usingCustomActionWithName:(NSString *)name; adds a predicate that searches elements for one that provides a custom action with a specified name.

- (void)activateCustomActionWithName:(NSString *)name; first performs a search for an element that provides a custom action with a specified name and then activates the action.

Sources/KIF/Additions/UIAccessibilityCustomAction+KIFAdditions.m

RoyalPineapple · 2024-07-16T21:53:08Z

@justinseanmartin What do you think of this proposed API? We're going to start relying on Custom Actions much more frequently in Market and im expecting I'll need this capability a fair amount to update our existing tests.

justinseanmartin

Mostly NITs or questions, but I think there probably are some minor changes to be made at least for prefixing the method in the category, the autoreleasepool and potentially the respondsToSelector check.

As to the question about the API. overall it seems good to me. I'm not sure if there is a scenario where we'll want to use usingCustomActionWithName: directly, as opposed to it being an implicit part of the action. I'd consider removing that from the public header if there isn't a scenario you're thinking of where we'll need to use that. I don't have a strong feeling on that though. It just seems kind of weird to have a [[viewTester usingCustomActionWithName:@"action"] waitForAppearance] or maybe even wierder [[viewTester usingCustomActionWithName:@"action"] tap]. We also don't have any tests exercising usingCustomActionWithName directly, which feels potentially like another indicator that we might not need that exposed?

justinseanmartin · 2024-07-19T06:14:03Z

Sources/KIF/Classes/KIFUIViewTestActor.m

+- (instancetype)usingCustomActionWithName:(NSString *)name
+{
+    NSPredicate *predicate = [NSPredicate predicateWithBlock:^BOOL(id evaluatedObject, NSDictionary *bindings) {
+        NSArray *actions = [evaluatedObject accessibilityCustomActions];


Every element that we iterate over in the view & accessibility element hierarchy is guaranteed to respond to this selector? Or should we add a respondsToSelector: check?

This is a property on NSObject defined thusly in UIAccessibility.h
@property (nullable, nonatomic, strong) NSArray <UIAccessibilityCustomAction *> *accessibilityCustomActions API_AVAILABLE(ios(8.0));

Though a respondsToSelector: check couldn't hurt.

justinseanmartin · 2024-07-19T06:17:33Z

Sources/KIF/Classes/KIFUIViewTestActor.m

+        for (UIAccessibilityCustomAction *action in actions) {
+            NSString *actionName = [action name];
+            if ([actionName isKindOfClass:[NSAttributedString class]]) {
+                actionName = [(NSAttributedString *)actionName string];
+            }
+
+            if ([actionName isEqualToString: name]) {
+                return true;
+            }
+        }


NIT: Up to you, but maybe worth a helper method for getting the specified action by name? Its identical here and below and might make these methods slightly simpler.

justinseanmartin · 2024-07-19T06:18:17Z

Sources/KIF/Classes/KIFUIViewTestActor.m

@@ -385,6 +406,31 @@ - (void)swipeFromEdge:(UIRectEdge)edge
    [self.actor swipeFromEdge:edge];
 }

+- (void)activateCustomActionWithName:(NSString *)name;
+{
+    KIFUIObject *found = [[self usingCustomActionWithName:name] _predicateSearchWithRequiresMatch:YES mustBeTappable:NO];


We generally wrap the body of these methods that return a KIFUIObject in an @autoreleasepool block so as to not accidentally extend the lifetime of the view object beyond this action.

Sources/KIF/Classes/KIFUIViewTestActor.m

justinseanmartin · 2024-07-19T06:27:20Z

Sources/KIF/Additions/UIAccessibilityCustomAction+KIFAdditions.h

+
+@interface UIAccessibilityCustomAction (KIFAdditions)
+
+- (BOOL)activate;


Use a prefix to avoid collisions, e.g. KIF_activate

oh man, i honestly do love objective c

justinseanmartin · 2024-07-19T06:36:53Z

Sources/KIF/Additions/UIAccessibilityCustomAction+KIFAdditions.m

+        [invocation getReturnValue:&returnValue];
+        return returnValue;
+    }
+    return NO;


Would it make more sense to raise if we attempted to perform an action on an element that says it supports it, but hasn't implemented the selector? I'd kind of expect this to crash or raise an error in some way that is distinct from the selector returning false from the action.

Ohh, nice idea

justinseanmartin · 2024-07-19T06:48:07Z

Sources/KIF/Classes/KIFUIViewTestActor.m

+            }
+
+            if ([actionName isEqualToString: name]) {
+                if([action activate]) {


How do VoiceOver and other accessibility tools respond if a custom action returns false? Are there valid use cases where a custom action might want to return false and that be a valid test scenario rather than treating it as a test failure? We don't need to extend to support that right now, but it seemed worth some thought/discussion.

Is it reasonable that a custom action might fail in some cases? Or is the expectation that custom actions should generally only be present on an element if they're expected to be able to be performed? Looking at the Firefox iOS codebase as an example, it seems like they filter the actions to ones that are expected to be usable in the current context, and then always return true. That's just one example though.

theres no official dogma on this particular question, but as we interpret the default behavior of UIKit and SwiftUI as guidance it would suggest that custom actions that are known to fail should be omitted entirely. Disabled buttons for example.

From the point of view of the person in front of the device, a failed action will play an additional "error" sound when activated.

You're right, we should consider the case where an action is expected to occasionally fail and allow for testing that assumption.

I've added an "expectedResult boolean" to allow this.

Sources/KIF/Additions/UIView-Debugging.m

justinseanmartin

Looks great!

…ector

* silencing non-exhaustive switch warning (kif-framework#1297) silencing non-exhaustive switch warning * Adding support for accessibility custom actions (kif-framework#1295) * adding support for accessibility custom actions * introduce tryFindingViewInFrame - avoid scroll when looking for a view visibility * remove wrong comment * improve comments * add missing disableScroll argument * introduce usingCurrentFrame() method on KIFUIViewTestActor to disable automatic scroll if needed * fixes: - fix wrong parameter name in comments - add missing methods - add missing things that prevent us to build for tests * fix to scrollView behavior: - with scrollDisabled = YES it now consider only views visible in the current frame, as it was done with TableView and CollectionViews - make explicit the scrollDisabled argument in recursive calls = NO when we are inside the scrolling section * add new integration tests for tryFindingViewInFrameWithAccessibilityIdentifier() and usingCurrentFrame() * Fix bug that was considering an occluded view as tappable * simplify boolean check and add integration test for broken case * Revert "simplify boolean check and add integration test for broken case" This reverts commit 6827225. * simplify boolean logic of the tappable check * add missing fromRootView argument Co-authored-by: Justin Martin <jmartin@squareup.com> * Change duplicated accessibilityLabel that was causing flakiness for 2 tests --------- Co-authored-by: Alex Odawa <aodawa@squareup.com> Co-authored-by: Simone Scionti <simone.scionti@qonto.com> Co-authored-by: Justin Martin <jmartin@squareup.com>

RoyalPineapple force-pushed the alex/CustomActions branch from c9d359f to 5c25cc7 Compare July 9, 2024 18:47

RoyalPineapple marked this pull request as ready for review July 9, 2024 19:53

RoyalPineapple changed the title ~~[DNM][DNR] adding support for accessibility custom actions~~ Adding support for accessibility custom actions Jul 9, 2024

RoyalPineapple requested review from kyleve and justinseanmartin July 9, 2024 19:55

kyleve reviewed Jul 10, 2024

View reviewed changes

Sources/KIF/Additions/UIAccessibilityCustomAction+KIFAdditions.m Outdated Show resolved Hide resolved

kyleve approved these changes Jul 10, 2024

View reviewed changes

justinseanmartin reviewed Jul 19, 2024

View reviewed changes

RoyalPineapple requested a review from justinseanmartin July 30, 2024 16:48

RoyalPineapple commented Jul 30, 2024

View reviewed changes

Sources/KIF/Additions/UIView-Debugging.m Outdated Show resolved Hide resolved

RoyalPineapple force-pushed the alex/CustomActions branch from 7903f6f to fb7f89f Compare July 30, 2024 16:57

justinseanmartin approved these changes Jul 30, 2024

View reviewed changes

RoyalPineapple added 11 commits August 1, 2024 16:47

adding support for custom actions

14ddfba

handling attributedstrings

928b63b

fixing includes

f293bbe

private

8572f3e

returning action handler result directly

028f998

respondsToSelector check

595e39f

refactoring custom action category

2fc2336

now raising excepton if custom action target doesnt implement the sel…

0db322e

…ector

refactoring and adding autorelease pool

5bf657b

allowing testing of actions that are expected to fail

3a9a6ec

adding tests

4675a70

RoyalPineapple force-pushed the alex/CustomActions branch from fb7f89f to 4675a70 Compare August 1, 2024 14:48

spelling

69d1753

RoyalPineapple merged commit 8757ba8 into master Aug 1, 2024
4 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for accessibility custom actions #1295

Adding support for accessibility custom actions #1295

RoyalPineapple commented Jul 9, 2024 •

edited

Loading

RoyalPineapple commented Jul 16, 2024 •

edited

Loading

justinseanmartin left a comment •

edited

Loading

justinseanmartin Jul 19, 2024

RoyalPineapple Jul 30, 2024 •

edited

Loading

justinseanmartin Jul 19, 2024

justinseanmartin Jul 19, 2024

justinseanmartin Jul 19, 2024

RoyalPineapple Jul 30, 2024

justinseanmartin Jul 19, 2024

RoyalPineapple Jul 30, 2024

justinseanmartin Jul 19, 2024

RoyalPineapple Jul 30, 2024 •

edited

Loading

justinseanmartin left a comment


		@interface UIAccessibilityCustomAction (KIFAdditions)

		- (BOOL)activate;

Adding support for accessibility custom actions #1295

Adding support for accessibility custom actions #1295

Conversation

RoyalPineapple commented Jul 9, 2024 • edited Loading

RoyalPineapple commented Jul 16, 2024 • edited Loading

justinseanmartin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyalPineapple Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyalPineapple Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

justinseanmartin left a comment

Choose a reason for hiding this comment

RoyalPineapple commented Jul 9, 2024 •

edited

Loading

RoyalPineapple commented Jul 16, 2024 •

edited

Loading

justinseanmartin left a comment •

edited

Loading

RoyalPineapple Jul 30, 2024 •

edited

Loading

RoyalPineapple Jul 30, 2024 •

edited

Loading