-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Implicit a11y scrolling for iOS (and caching in Viewports) #17021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| /// is different from the rect returned by [describeApproximatePaintClip]. | ||
| /// If the semantics clip rect and the paint clip rect are the same, this | ||
| /// method returns null. | ||
| Rect describeSemanticsClip(covariant RenderObject child) => null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth adding a paragraph about the motivation and typical use
| /// | ||
| /// The cache area is also used to implement implicit accessibility scrolling | ||
| /// on iOS: When the accessibility focus moves from an item in the visible | ||
| /// viewport to an invisible item in the cache area, th framework will bring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
th->the
| break; | ||
| } | ||
|
|
||
| return parentData.layoutOffset < renderObject.constraints.scrollOffset + renderObject.constraints.remainingPaintExtent && parentData.layoutOffset + itemExtent > renderObject.constraints.scrollOffset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line could benefit from some wrapping
|
|
||
| @override | ||
| Rect describeSemanticsClip(RenderObject child) { | ||
| switch(axis) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: space after switch
| semanticBounds.bottom, | ||
| ); | ||
| } | ||
| assert(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the assert (!= null) should be before the switch
| /// | ||
| /// The cache area is also used to implement implicit accessibility scrolling | ||
| /// on iOS: When the accessibility focus moves from an item in the visible | ||
| /// viewport to an invisible item in the cache area, th framework will bring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
th->the here too
you might want to use a template rather than duplicating this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL DartDoc templates. So cool.
| /// | ||
| /// The cache area is also used to implement implicit accessibility scrolling | ||
| /// on iOS: When the accessibility focus moves from an item in the visible | ||
| /// viewport to an invisible item in the cache area, th framework will bring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
th->the
| /// The scroll view has an area before and after the visible area to cache | ||
| /// items that are about to become visible when the user scrolls. | ||
| /// | ||
| /// Items that fall in this cache area are rendered even though they are not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(here and everywhere else this text is included):
"rendered" means "shown on screen".
Maybe use the term "laid out"? Or "instantiated"?
|
|
||
| @override | ||
| Rect describeSemanticsClip(RenderSliver child) { | ||
| switch(axis) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same nits as before
maybe factor out this method into something where it can be reused directly?
| case Axis.vertical: | ||
| return new Rect.fromLTRB( | ||
| semanticBounds.left, | ||
| semanticBounds.top - cacheExtent, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(same comment in the other instance of this method):
Why a fixed extend out in the scrolling axis, instead of near-infinity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the semantics tree compact.
In the context of the SingleChildViewport: Imagine a thousands of pixels long column inside the viewport with many child SemanticsNodes (one per cell). Only the semantics nodes that fall inside the visible range plus a handful of objects before and after the viewport are relevant. Everything else is overhead. This ensures we cut those other child semantic nodes out.
Same is true for the other Sliver-based viewport: Imagine a loooong sliver with many child semantics nodes. Again, only some of these are relevant.
|
|
||
| // we made it without a correction, whee! | ||
| return 0.0; | ||
| return 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unexpected extra space
|
Great addition! |
Viewport Cache
Adds a
cacheExtentto viewports, which defines an area before and after the visible part of the viewport in axis direction. Elements, that fall into this cache area, will be layouted and included in the render tree even though they are not visible (yet). They are pre-cached because they will appear in the viewport next when the user starts scrolling.Slivers are informed about the
cacheExtentin their incoming SliverConstraints via the newcacheOriginandremainingCacheExtentproperties and they should compute a value for the newcacheExtentproperty in their SliverGeometry (see the corresponding doc comments for details).Fixes #170.
Implicit a11y scrolling on iOS
The elements falling into the cache area are also included in the semantics tree, where they are marked as
hidden. These are used on iOS to implement implicit a11y scrolling: When the user moves the focus from the last visible element in the viewport to one of these hidden elements via linear navigation (right/left swipes) the framework is asked to bring this hidden element into view and an implicit scroll action is performed (see flutter/engine#5052).To figure out which SemanticsNodes should be marked as hidden in the tree the viewport defines a new
semanticsClipRectin addition to the already presentpaintClipRect. While thepaintClipRecthas the size of the viewport (the visible part), thesemanticsClipRectis extended by the cache areas at the beginning and end of the viewport. If a SemanticsNode falls outside of thesemanticsClipRectit is not included in the semantics tree at all because it's currently unreachable by the user. If it is insidesemanticsClipRect, but outside ofpaintClipRectit is included in the tree and marked as hidden to enable implicit a11y scrolling. If it is insidepaintClipRectit is included like a regular SemanticsNode.Fixes #11983.
Fixes #17023.
Also in this PR
RenderObjects that are currently not visible in the viewport are now considered offstage in tests. In order to find them with the Finders provided by the Flutter test framework you'll have to set
skipOffstage: falseon the Finder.