Repository navigation
add PageView cacheExtent - #42107
add PageView cacheExtent#42107lukepighetti wants to merge 3 commits into
Conversation
|
Did this PR on master which appears to be a little dirty, but my PR & test coverage is clean. |
chunhtai
left a comment
There was a problem hiding this comment.
The analyzer seems to be unhappy :(
goderbauer
left a comment
There was a problem hiding this comment.
Before this can be merged we need to double-check that it doesn't break the accessibility of the PageView especially on iOS. On iOS the engine uses the cache extent as a signal for accessibility scrolling.
With voiceover turned on, the user should not be able to scroll to the next page just by advancing the accessibility focus by swing left or right.
Technically, we have the hasImplicitScrolling flag [1] to control that, but it looks like that's currently only wired up on Android and not on iOS [2]. We probably need to change that in order to allow PageView to have a cacheExtent.
I am marking this as "request changes" until it has either been verified that this does not break accessibility on iOS, or (if it does) until that has been fixed.
[1] https://master-api.flutter.dev/flutter/dart-ui/SemanticsFlag/hasImplicitScrolling-constant.html
[2] https://github.com/flutter/engine/blob/master/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm
|
@goderbauer does the |
|
flutter_test is for unit tests. This would require an integration test that verifies Flutter's interaction with iOS VoiceOver. |
|
@goderbauer what do you need from me at this point for this to land? |
|
I just verified: Setting a cacheExtend on a pageView breaks accessibility scrolling on iOS. Before we can land this, we need to change the iOS accessibility bridge in the engine [1] to ensure that it properly respects the |
|
@goderbauer Thanks for the follow up. I'll assume there's nothing I can do to assist until I am notified otherwise. |
|
If you want, you could make that change to the accessibility bridge. Other than that there's not much to do. |
|
@goderbauer It's an area I'm not comfortable with, it would need some serious code review. Is this something that is coming down the pipeline or is it only to be implemented with this PR? |
|
@lukepighetti The work for that is currently not planned, but we need to do it at some point. /cc @darrenaustin Maybe you're interested in this? I am going to close this PR for now as we cannot merge it without the engine side changes and nobody is currently working on those. |
|
Why is this closed? We need this pr |
where to add these lines ? kindly tell me. i added inside the page builder as well as in page view but not works, |
|
I just stumbled upon this and it looks like there was a recent pull request that potentially added the required accessibility support on iOS that was blocking this?: flutter-team-archive/engine#26671 |
|
@nocduro The accessibility scroll support in the PR is tangential to the problem @goderbauer mentioned |
|
The flutter team should somehow provide the preload ability for pageview..... can someone tell me when this feature will be planned. |
|
Absolutely need this aswell, as I have a form for data entry spread across several pages. I know that there is a package out there to enable this, but I always prefer to have a "native" solution. With the current PageView, after you scrolled through all the none preloaded pages and then back to the beginning, isn't the situation you are now in exactly the same as if they were preloaded from the beginning? So this simply seems to be a shortcut to get somewhere where you can get already anyway, but also enabling more flexability in how you use the PageView. |
Description
Non-breaking change that allows PageView to specify cacheExtents in whole pages. Ie, setting cacheExtent to 1 will build the current page and the one before and the one after it.
Related Issues
#31191
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?