Skip to content

Verify EditableText fires update on replaced controller #12276

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

Merged
merged 2 commits into from
Sep 28, 2017

Conversation

cbracken
Copy link
Member

Adds a test that verifies that EditableText sends a
TextInput.setEditingState message to the engine when the associated
TextEditingController is replaced.

@cbracken
Copy link
Member Author

Adds test to cover #12154.

cursorColor: Colors.blue,
selectionControls: materialTextSelectionControls,
keyboardType: TextInputType.text,
onChanged: (String value) {},
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial nit: style guide says to use {} for empty maps and { } for empty code blocks

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


final MethodCall setStateCall = log.firstWhere((MethodCall methodCall) {
return methodCall.method == 'TextInput.setEditingState';
});
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to be more precise about what should be in the log? e.g. that it only has one entry, or whatnot? What if there were two calls, one saying Wobble and one saying Wibble?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Was worried about testing some assumptions we don't care about here but the result is under-testing, and if someone's going to tweak this code, we should probably force them to check these tests anyway :-)

@Hixie
Copy link
Contributor

Hixie commented Sep 27, 2017

LGTM

FWIW what I usually do with these logs is have the log be a List and have whatever adds stuff to the logs add a description of what it cares about (e.g. method name, argument contents), and then in the expects call i just match the entire list with exactly what all the strings should be.

Adds a test that verifies that EditableText sends a
TextInput.setEditingState message to the engine when the associated
TextEditingController is replaced.
@cbracken cbracken force-pushed the controller-update-test branch from d13c670 to 780522f Compare September 28, 2017 18:14
@cbracken cbracken merged commit f06ef52 into flutter:master Sep 28, 2017
@cbracken cbracken deleted the controller-update-test branch September 28, 2017 19:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants