-
Notifications
You must be signed in to change notification settings - Fork 6k
[iOS] Full keyboard access scrolling #56606
Changes from all commits
de4715d
93f866a
24eb36d
7450322
035b476
a57d56e
0703665
63776e6
bc501a2
b91b878
4196be8
b072522
15dedf5
97a1cb1
e14b808
1bf0f5c
fc6586a
3073925
e3afee0
7d7b59b
2423887
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,10 @@ | |
| // found in the LICENSE file. | ||
|
|
||
| #import "SemanticsObject.h" | ||
| #include "flutter/lib/ui/semantics/semantics_node.h" | ||
| #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterCodecs.h" | ||
| #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" | ||
| #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h" | ||
|
|
||
| FLUTTER_ASSERT_ARC | ||
|
|
||
|
|
@@ -27,10 +30,19 @@ | |
| // translated to calls such as -[NSObject accessibilityActivate]), while most | ||
| // other key events are dispatched to the framework. | ||
| @interface SemanticsObject (UIFocusSystem) <UIFocusItem, UIFocusItemContainer> | ||
| /// The `UIFocusItem` that represents this SemanticsObject. | ||
| /// | ||
| /// For regular `SemanticsObject`s, this method returns `self`, | ||
| /// for `FlutterScrollableSemanticsObject`s, this method returns its scroll view. | ||
| - (id<UIFocusItem>)focusItem; | ||
| @end | ||
|
|
||
| @implementation SemanticsObject (UIFocusSystem) | ||
|
|
||
| - (id<UIFocusItem>)focusItem { | ||
| return self; | ||
| } | ||
|
|
||
| #pragma mark - UIFocusEnvironment Conformance | ||
|
|
||
| - (void)setNeedsFocusUpdate { | ||
|
|
@@ -49,7 +61,7 @@ - (void)didUpdateFocusInContext:(UIFocusUpdateContext*)context | |
|
|
||
| - (id<UIFocusEnvironment>)parentFocusEnvironment { | ||
| // The root SemanticsObject node's parent is the FlutterView. | ||
| return self.parent ?: self.bridge->view(); | ||
| return self.parent.focusItem ?: self.bridge->view(); | ||
| } | ||
|
|
||
| - (NSArray<id<UIFocusEnvironment>>*)preferredFocusEnvironments { | ||
|
|
@@ -71,8 +83,57 @@ - (BOOL)canBecomeFocused { | |
| return self.node.HasAction(flutter::SemanticsAction::kTap); | ||
| } | ||
|
|
||
| // The frame is described in the `coordinateSpace` of the | ||
| // `parentFocusEnvironment` (all `parentFocusEnvironment`s are `UIFocusItem`s). | ||
| // | ||
| // See also the `coordinateSpace` implementation. | ||
| // TODO(LongCatIsLooong): use CoreGraphics types. | ||
| - (CGRect)frame { | ||
| return self.accessibilityFrame; | ||
| SkPoint quad[4] = {SkPoint::Make(self.node.rect.left(), self.node.rect.top()), | ||
| SkPoint::Make(self.node.rect.left(), self.node.rect.bottom()), | ||
| SkPoint::Make(self.node.rect.right(), self.node.rect.top()), | ||
| SkPoint::Make(self.node.rect.right(), self.node.rect.bottom())}; | ||
|
|
||
| SkM44 transform = self.node.transform; | ||
| FlutterSemanticsScrollView* scrollView; | ||
| for (SemanticsObject* ancestor = self.parent; ancestor; ancestor = ancestor.parent) { | ||
| if ([ancestor isKindOfClass:[FlutterScrollableSemanticsObject class]]) { | ||
| scrollView = ((FlutterScrollableSemanticsObject*)ancestor).scrollView; | ||
| break; | ||
| } | ||
cbracken marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| transform = ancestor.node.transform * transform; | ||
| } | ||
|
|
||
| for (auto& vertex : quad) { | ||
| SkV4 vector = transform.map(vertex.x(), vertex.y(), 0, 1); | ||
| vertex = SkPoint::Make(vector.x / vector.w, vector.y / vector.w); | ||
| } | ||
|
|
||
| SkRect rect; | ||
| rect.setBounds(quad, 4); | ||
cbracken marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // If this UIFocusItemContainer's coordinateSpace is a UIScrollView, offset | ||
| // the rect by `contentOffset` because the contentOffset translation is | ||
| // incorporated into the paint transform at different node depth in UIKit | ||
| // and Flutter. In Flutter, the translation is added to the cells | ||
| // while in UIKit the viewport's bounds is manipulated (IOW, each cell's frame | ||
| // in the UIScrollView coordinateSpace does not change when the UIScrollView | ||
| // scrolls). | ||
| CGRect unscaledRect = | ||
| CGRectMake(rect.x() + scrollView.bounds.origin.x, rect.y() + scrollView.bounds.origin.y, | ||
| rect.width(), rect.height()); | ||
| if (scrollView) { | ||
| return unscaledRect; | ||
| } | ||
| // `rect` could be in physical pixels since the root RenderObject ("RenderView") | ||
| // applies a transform that turns logical pixels to physical pixels. Undo the | ||
| // transform by dividing the coordinates by the screen's scale factor, if this | ||
| // UIFocusItem's reported `coordinateSpace` is the root view (which means this | ||
| // UIFocusItem is not inside of a scroll view). | ||
| // | ||
| // Screen can be nil if the FlutterView is covered by another native view. | ||
| CGFloat scale = (self.bridge->view().window.screen ?: UIScreen.mainScreen).scale; | ||
| return CGRectMake(unscaledRect.origin.x / scale, unscaledRect.origin.y / scale, | ||
| unscaledRect.size.width / scale, unscaledRect.size.height / scale); | ||
| } | ||
|
|
||
| #pragma mark - UIFocusItemContainer Conformance | ||
|
|
@@ -87,16 +148,94 @@ - (CGRect)frame { | |
| // | ||
| // This method is only supposed to return items within the given | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just a note that I have run into weird bug before because i returned child something outside of the rect of accessibilityContainer. I am not sure if it applies here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I can try adding culling here but I'll have to test it separately (and it seems to be working fine w/o the culling). |
||
| // rect but returning everything in the subtree seems to work fine. | ||
| NSMutableArray<SemanticsObject*>* reversedItems = | ||
| NSMutableArray<id<UIFocusItem>>* reversedItems = | ||
| [[NSMutableArray alloc] initWithCapacity:self.childrenInHitTestOrder.count]; | ||
| for (NSUInteger i = 0; i < self.childrenInHitTestOrder.count; ++i) { | ||
| [reversedItems | ||
| addObject:self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]]; | ||
| SemanticsObject* child = self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]; | ||
| [reversedItems addObject:child.focusItem]; | ||
| } | ||
| return reversedItems; | ||
| } | ||
|
|
||
| - (id<UICoordinateSpace>)coordinateSpace { | ||
| return self.bridge->view(); | ||
| // A regular SemanticsObject uses the same coordinate space as its parent. | ||
| return self.parent.coordinateSpace ?: self.bridge->view(); | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| /// Scrollable containers interact with the iOS focus engine using the | ||
| /// `UIFocusItemScrollableContainer` protocol. The said protocol (and other focus-related protocols) | ||
| /// does not provide means to inform the focus system of layout changes. In order for the focus | ||
| /// highlight to update properly as the scroll view scrolls, this implementation incorporates a | ||
| /// UIScrollView into the focus hierarchy to workaround the highlight update problem. | ||
| /// | ||
| /// As a result, in the current implementation only scrollable containers and the root node | ||
| /// establish their own `coordinateSpace`s. All other `UIFocusItemContainter`s use the same | ||
| /// `coordinateSpace` as the containing UIScrollView, or the root `FlutterView`, whichever is | ||
| /// closer. | ||
| /// | ||
| /// See also the `frame` method implementation. | ||
| #pragma mark - Scrolling | ||
|
|
||
| @interface FlutterScrollableSemanticsObject (CoordinateSpace) | ||
| @end | ||
|
|
||
| @implementation FlutterScrollableSemanticsObject (CoordinateSpace) | ||
| - (id<UICoordinateSpace>)coordinateSpace { | ||
| // A scrollable SemanticsObject uses the same coordinate space as the scroll view. | ||
| // This may not work very well in nested scroll views. | ||
| return self.scrollView; | ||
| } | ||
|
|
||
| - (id<UIFocusItem>)focusItem { | ||
| return self.scrollView; | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| @interface FlutterSemanticsScrollView (UIFocusItemScrollableContainer) < | ||
| UIFocusItemScrollableContainer> | ||
| @end | ||
|
|
||
| @implementation FlutterSemanticsScrollView (UIFocusItemScrollableContainer) | ||
|
|
||
| #pragma mark - FlutterSemanticsScrollView UIFocusItemScrollableContainer Conformance | ||
|
|
||
| - (CGSize)visibleSize { | ||
| return self.frame.size; | ||
| } | ||
|
|
||
| - (void)setContentOffset:(CGPoint)contentOffset { | ||
| [super setContentOffset:contentOffset]; | ||
| // Do no send flutter::SemanticsAction::kScrollToOffset if it's triggered | ||
| // by a framework update. | ||
| if (![self.semanticsObject isAccessibilityBridgeAlive] || !self.isDoingSystemScrolling) { | ||
| return; | ||
| } | ||
|
|
||
| double offset[2] = {contentOffset.x, contentOffset.y}; | ||
| FlutterStandardTypedData* offsetData = [FlutterStandardTypedData | ||
| typedDataWithFloat64:[NSData dataWithBytes:&offset length:sizeof(offset)]]; | ||
| NSData* encoded = [[FlutterStandardMessageCodec sharedInstance] encode:offsetData]; | ||
| self.semanticsObject.bridge->DispatchSemanticsAction( | ||
| self.semanticsObject.uid, flutter::SemanticsAction::kScrollToOffset, | ||
| fml::MallocMapping::Copy(encoded.bytes, encoded.length)); | ||
| } | ||
|
|
||
| - (BOOL)canBecomeFocused { | ||
| return NO; | ||
| } | ||
|
|
||
| - (id<UIFocusEnvironment>)parentFocusEnvironment { | ||
| return self.semanticsObject.parentFocusEnvironment; | ||
| } | ||
|
|
||
| - (NSArray<id<UIFocusEnvironment>>*)preferredFocusEnvironments { | ||
| return nil; | ||
| } | ||
|
|
||
| - (NSArray<id<UIFocusItem>>*)focusItemsInRect:(CGRect)rect { | ||
| return [self.semanticsObject focusItemsInRect:rect]; | ||
| } | ||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,6 +154,8 @@ - (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)br | |
| _scrollView = [[FlutterSemanticsScrollView alloc] initWithSemanticsObject:self]; | ||
| [_scrollView setShowsHorizontalScrollIndicator:NO]; | ||
| [_scrollView setShowsVerticalScrollIndicator:NO]; | ||
| [_scrollView setContentInset:UIEdgeInsetsZero]; | ||
| [_scrollView setContentInsetAdjustmentBehavior:UIScrollViewContentInsetAdjustmentNever]; | ||
| [self.bridge->view() addSubview:_scrollView]; | ||
| } | ||
| return self; | ||
|
|
@@ -174,7 +176,10 @@ - (void)accessibilityBridgeDidFinishUpdate { | |
| // contentOffset is 0.0, only the scroll down action is available. | ||
| self.scrollView.frame = self.accessibilityFrame; | ||
| self.scrollView.contentSize = [self contentSizeInternal]; | ||
| [self.scrollView setContentOffset:[self contentOffsetInternal] animated:NO]; | ||
| // See the documentation on `isDoingSystemScrolling`. | ||
| if (!self.scrollView.isDoingSystemScrolling) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means if the scroll view is in a FKA scrolling animation, all framework updates will be disregarded. This is not optimal, if the user wants to interrupt the scrolling animation via swipe gestures, the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tried removing this flag, and the offsets echoed back from the framework seem always to be one step behind and I am not sure exactly why: It seems but instead it's always delayed: full log
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, i think this makes sense since FKA set content will change the scrollcontroller, but it will have to wait for one frame to get the layout and semantics update back
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a comment covering this here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it looks like I was looking at an older checkout, and it was changed here. The event ordering becomes the way I expected it to be if we run That seems to be the better option because we don't need the flag and the scroll view will be responsive to user gestures during the FKA scroll animation, but I think I'll make the attempt a separate PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am curious to see if it will work, but I think even if the action is send to framework synchronously, it may still take one frame for flutter to rebuild layout and flush semantics back
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's fine I think. The problem with If the communication is synchronous then the inconsistent state won't be observable as the updates would arrive instantaneously. |
||
| [self.scrollView setContentOffset:self.contentOffsetInternal animated:NO]; | ||
| } | ||
| } | ||
|
|
||
| - (id)nativeAccessibility { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.