Skip to content

Conversation

tgucio
Copy link
Contributor

@tgucio tgucio commented Nov 18, 2020

Description

Reset the cursor timer in _handleSelectionChanged() so as to keep it from blinking when it's moved using keyboard.

Related Issues

#69321

Behaviour before and after the change

Before:
before

After:
after

Tests

I added the following tests:

packages/flutter/test/widgets/editable_text_cursor_test.dart:
testWidgets('Cursor animation restarts when it is moved using keys on desktop', (WidgetTester tester)

Relevant tests passing:

% flutter test test/widgets/editable_text_cursor_test.dart
00:15 +24: All tests passed!
% flutter test test/widgets/editable_text_show_on_screen_test.dart
00:05 +9: All tests passed!
% flutter test test/widgets/editable_text_test.dart
00:12 +152: All tests passed!

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.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Nov 18, 2020
@google-cla google-cla bot added the cla: yes label Nov 18, 2020
@goderbauer
Copy link
Member

(PR triage): Looks like the analyzer is unhappy. Can you take a look and fix that?

@goderbauer goderbauer requested a review from justinmc November 18, 2020 22:39
@tgucio
Copy link
Contributor Author

tgucio commented Nov 19, 2020

(PR triage): Looks like the analyzer is unhappy. Can you take a look and fix that?

My bad. I ran the analyzer with another flutter checkout on PATH. Running as "bin/flutter analyze --flutter-repo" helped.

Anyway, the issue was with 2 warnings in the test - I pushed an amended commit and tests are under way.

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this!

This will also reset the timer on mobile and when clicking/tapping instead of using keys, but I think that is desired behavior as well.

I confirmed that the test will fail without the fix.

@justinmc
Copy link
Contributor

justinmc commented Dec 1, 2020

FYI @LongCatIsLooong.

I was just reading your RenderEditable painters design doc and thought this was somewhat related, just giving you a heads up.

@tgucio tgucio deleted the flutter-prs branch December 13, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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