EditableText Cursor can be set to not blink for testing #20004
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
goderbauer
reviewed
Jul 30, 2018
| @@ -399,6 +399,15 @@ class EditableText extends StatefulWidget { | |||
| /// Defaults to EdgeInserts.all(20.0). | |||
| final EdgeInsets scrollPadding; | |||
|
|
|||
| static bool get debugDeterministicCursor => _debugDeterministicCursor; | |||
There was a problem hiding this comment.
Can you please add a doc comment explaining this setting?
| @@ -399,6 +399,15 @@ class EditableText extends StatefulWidget { | |||
| /// Defaults to EdgeInserts.all(20.0). | |||
| final EdgeInsets scrollPadding; | |||
|
|
|||
| static bool get debugDeterministicCursor => _debugDeterministicCursor; | |||
| static bool _debugDeterministicCursor = false; | |||
| static set debugDeterministicCursor(bool value) { | |||
There was a problem hiding this comment.
Looks like this setter doesn't really do anything? Could we just use a regular property here?
gspencergoog
approved these changes
Aug 6, 2018
|
|
gspencergoog
previously requested changes
Aug 6, 2018
| @@ -399,6 +399,8 @@ class EditableText extends StatefulWidget { | |||
| /// Defaults to EdgeInserts.all(20.0). | |||
| final EdgeInsets scrollPadding; | |||
|
|
|||
| static bool debugDeterministicCursor = false; | |||
There was a problem hiding this comment.
Please add a comment here describing what it does and when it is used (only in tests).
sandrasandeep
dismissed
gspencergoog’s stale review
Added documentation as requested.
|
LGTM (still) :-) |
xster
reviewed
Aug 6, 2018
| @@ -1030,4 +1036,4 @@ class _Editable extends LeafRenderObjectWidget { | |||
| ..cursorWidth = cursorWidth | |||
| ..cursorRadius = cursorRadius; | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
I took it out because it causes the tests to fail.
Hixie
added a commit
that referenced
this pull request
Aug 6, 2018
* Revert "increase size of user account drawer headers to 48 by 48 (#20266)" This reverts commit 4a7b4a4. * Revert "EditableText Cursor can be set to not blink for testing (#20004)" This reverts commit d041b31. * Revert "Refactor analysis benchmark and collect more data (#20169)" This reverts commit 5ea0a13.
Hixie
reviewed
Aug 14, 2018
| @@ -881,7 +887,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien | |||
| textSpan: buildTextSpan(), | |||
| value: _value, | |||
| cursorColor: widget.cursorColor, | |||
| showCursor: _showCursor, | |||
| showCursor: EditableText.debugDeterministicCursor ? ValueNotifier<bool>(true) : _showCursor, | |||
There was a problem hiding this comment.
we should probably have a constant here rather than creating a new value notifier each frame
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses issue #14656. Adds static field to EditableText to enable developers to make the cursor continually appear on the screen (for testing purposes).