Skip to content
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

Scroll scrollable to keep focused control visible. #44965

Merged
merged 4 commits into from Nov 20, 2019

Conversation

@gspencergoog
Copy link
Contributor

gspencergoog commented Nov 15, 2019

Description

Before this change, it was possible to move the focus onto a control that was no longer in the view using focus traversal. This changes that, so that when a control is focused, it makes sure that if it is the child of a scrollable, that the scrollable attempts to keep it in view. If it is already in view, then nothing scrolls.

When asked to move in a direction, the focus traversal code tries to find a control to move to inside the scrollable first, and then looks for things outside of the scrollable only once the scrollable has reached its limit in that direction.

Tests

  • Added tests that make sure that focused items in a scrollable stay in view when traversed.

Breaking Change

  • No, this is not a breaking change.
@googlebot googlebot added the cla: yes label Nov 15, 2019
@gspencergoog gspencergoog force-pushed the gspencergoog:scroll_to_focus branch from bd00709 to 617c083 Nov 15, 2019
@gspencergoog gspencergoog requested a review from darrenaustin Nov 15, 2019
@gspencergoog gspencergoog marked this pull request as ready for review Nov 15, 2019
@gspencergoog gspencergoog force-pushed the gspencergoog:scroll_to_focus branch from aaba3fe to b3afc88 Nov 19, 2019
@Piinks Piinks added the f: scrolling label Nov 19, 2019
Copy link
Contributor

Piinks left a comment

Does the _RequestFocusActionBase class need to be addressed in anyway? It looks like none of its implementors use the that class' invoke method anymore with this change. 🤔

Otherwise, just a few nits!

@gspencergoog gspencergoog removed the request for review from darrenaustin Nov 19, 2019
@gspencergoog gspencergoog force-pushed the gspencergoog:scroll_to_focus branch from b3afc88 to 672d48e Nov 19, 2019
@Piinks
Piinks approved these changes Nov 20, 2019
Copy link
Contributor

Piinks left a comment

LGTM! 🎉

@gspencergoog gspencergoog merged commit 0d27fdc into flutter:master Nov 20, 2019
63 checks passed
63 checks passed
WIP Ready for review
Details
analyze-linux Task Summary
Details
analyze-linux
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
cla/google All necessary CLAs are signed
customer_testing-linux Task Summary
Details
customer_testing-linux
Details
customer_testing-macos Task Summary
Details
customer_testing-macos
Details
customer_testing-windows Task Summary
Details
customer_testing-windows
Details
deploy_gallery-linux Task Summary
Details
deploy_gallery-linux
Details
deploy_gallery-macos Task Summary
Details
deploy_gallery-macos
Details
docs-linux Task Summary
Details
docs-linux
Details
firebase_test_lab_tests-linux Task Summary
Details
firebase_test_lab_tests-linux
Details
flutter-build
Details
framework_tests-libraries-linux Task Summary
Details
framework_tests-libraries-linux
Details
framework_tests-libraries-macos Task Summary
Details
framework_tests-libraries-macos
Details
framework_tests-libraries-windows Task Summary
Details
framework_tests-libraries-windows
Details
framework_tests-misc-linux Task Summary
Details
framework_tests-misc-linux
Details
framework_tests-misc-macos Task Summary
Details
framework_tests-misc-macos
Details
framework_tests-misc-windows Task Summary
Details
framework_tests-misc-windows
Details
framework_tests-widgets-linux Task Summary
Details
framework_tests-widgets-linux
Details
framework_tests-widgets-macos Task Summary
Details
framework_tests-widgets-macos
Details
framework_tests-widgets-windows Task Summary
Details
framework_tests-widgets-windows
Details
hostonly_devicelab_tests-0-linux Task Summary
Details
hostonly_devicelab_tests-0-linux
Details
hostonly_devicelab_tests-1-linux Task Summary
Details
hostonly_devicelab_tests-1-linux
Details
hostonly_devicelab_tests-2-linux Task Summary
Details
hostonly_devicelab_tests-2-linux
Details
hostonly_devicelab_tests-3_last-linux Task Summary
Details
hostonly_devicelab_tests-3_last-linux
Details
web_tests-0-linux Task Summary
Details
web_tests-0-linux
Details
web_tests-1-linux Task Summary
Details
web_tests-1-linux
Details
web_tests-2-linux Task Summary
Details
web_tests-2-linux
Details
web_tests-3-linux Task Summary
Details
web_tests-3-linux
Details
web_tests-4-linux Task Summary
Details
web_tests-4-linux
Details
web_tests-5-linux Task Summary
Details
web_tests-5-linux
Details
web_tests-6-linux Task Summary
Details
web_tests-6-linux
Details
web_tests-7_last-linux Task Summary
Details
web_tests-7_last-linux
Details
@gspencergoog gspencergoog deleted the gspencergoog:scroll_to_focus branch Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.