Skip to content

Fixes Focus and FocusScope's assignment of canRequestFocus.#46168

Merged
gspencergoog merged 3 commits intoflutter:masterfrom
gspencergoog:fix_scope_canRequestFocus
Dec 5, 2019
Merged

Fixes Focus and FocusScope's assignment of canRequestFocus.#46168
gspencergoog merged 3 commits intoflutter:masterfrom
gspencergoog:fix_scope_canRequestFocus

Conversation

@gspencergoog
Copy link
Copy Markdown
Contributor

Description

This fixes an issue where lines like this:

    focusNode.canRequestFocus = widget.canRequestFocus ?? focusNode.canRequestFocus;

Were causing the canRequestFocus bit to copy the status of the enclosing scope, since canRequestFocus also looks to the enclosing scope to decide if it can focus.

Related Issues

Tests

  • Added a test that tests to make sure that turning on and off canRequestFocus does the right thing for child nodes.

@fluttergithubbot fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Dec 5, 2019
@gspencergoog gspencergoog changed the title Fix scope can request focus Fixes Focus and FocusScope's assignment of canRequestFocus. Dec 5, 2019
@gspencergoog gspencergoog force-pushed the fix_scope_canRequestFocus branch from 58b25da to 9d33d15 Compare December 5, 2019 18:17
Copy link
Copy Markdown
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

expect(key.currentState.focusNode.canRequestFocus, isFalse);
});

testWidgets('canRequestFocus causes descendants of scope to be skipped.', (WidgetTester tester) async {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there also be a new test for skipTraversal since that logic also changes in this PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test. I don't actually need to change the code for skipTraversal: it's getter doesn't depend on the enclosing scope, so the code change doesn't change the logic.

It doesn't hurt to have another test though, and I figured that changing the code to match what I did for canRequestFocus made it more future proof (and didn't actually change anything).

@gspencergoog gspencergoog merged commit e3005e6 into flutter:master Dec 5, 2019
@gspencergoog gspencergoog deleted the fix_scope_canRequestFocus branch March 13, 2020 16:10
@lock
Copy link
Copy Markdown

lock Bot commented Apr 2, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@lock lock Bot locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants