Skip to content

Conversation

@bleroux
Copy link
Contributor

@bleroux bleroux commented Mar 10, 2023

Description

This PR updates the undo logic to fix two problems:

  • Values with an invalid selection should not be saved in the history (this leads to the cursor disappearing).
  • When an undo is received before the end of the throttling delay, the history should not be undo its last saved value because this will set the text field value to the penultimate value (the last one is not yet in the history).

Steps to Reproduce

Code sample (which creates a TextField with the non empty text, "Flutter").
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      home: Scaffold(
        appBar: AppBar(
          title: const Text("Demo"),
        ),
        body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: <Widget>[
              TextFormField(
                initialValue: "Flutter"
              ),
            ],
          ),
        ),
      ),
    );
  }
}

Problem 1: Tap ‘3’ and undo before the throttling delay ends (500ms).

Before this fix: the text is ‘Flutter’ and the cursor disappeared. Tapping a new key will insert it before ‘Flutter’.
After this fix: the text should be ‘Flutter’ and the cursor at the end.

Problem 2: Tap ‘3’, wait for the throttling delay, tap ‘3’ again and undo before the throttling delay ends.
Before this fix: the text value is ‘Flutter’ (penultimate state).
After this fix: the text value is ‘Flutter3’.

Related Issue

Fixes #120194

Tests

Adds 2 tests.

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Mar 10, 2023
@bleroux bleroux marked this pull request as draft March 10, 2023 15:21
@bleroux bleroux force-pushed the fix_undo_redo_cursor_jump branch from 33895b2 to a5756c3 Compare March 13, 2023 16:38
@bleroux bleroux requested a review from justinmc March 13, 2023 19:38
@bleroux bleroux marked this pull request as ready for review March 13, 2023 19:38
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.

Looks good, just one idea to possibly get rid of _hasPendingChange.

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 be possible to get rid of _hasPendingChange and use _throttleTimer?.isActive instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea. I updated the PR and it works nicely, thanks!

@bleroux bleroux force-pushed the fix_undo_redo_cursor_jump branch from a5756c3 to 1d5e3e3 Compare March 14, 2023 08:58
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 👍 Thank you for fixing this!

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 15, 2023
@bleroux bleroux force-pushed the fix_undo_redo_cursor_jump branch from a68fbb0 to 87e43de Compare March 16, 2023 06:56
@auto-submit auto-submit bot merged commit 09cc1c4 into flutter:master Mar 16, 2023
@bleroux bleroux deleted the fix_undo_redo_cursor_jump branch March 16, 2023 08:32
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TextField/TextFormField cursor jumps to beginning after undo

2 participants