Skip to content
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

Flutter's text input plugin should report deltas instead of full text blocks #87972

Closed
matthew-carroll opened this issue Aug 10, 2021 · 26 comments · Fixed by flutter/engine#28175, #88477, flutter/engine#28418 or flutter/engine#28527
Assignees
Labels
a: text input Entering text in a text field or keyboard related problems customer: todo framework flutter/packages/flutter repository. See also f: labels. P0 Critical issues such as a build break or regression

Comments

@matthew-carroll
Copy link
Contributor

When a user makes changes to text content, Flutter doesn't report the change in content. Instead, Flutter reports the entire text block with various changes already applied to the text block.

Without the knowledge of text change deltas, it's impossible implement attributed text editing, e.g., styled text, text with links, text separated across document nodes. A delta would inform the text editor how to expand, contract, add, or remove attributions. Without delta information, it's impossible to be sure that attribution changes are appropriate/correct.

How to implement on Android
Currently, Flutter uses SpannableStringBuilder to implement Editable. Instead, Flutter should directly implement Editable and forward the deltas to the framework. Let the framework apply the deltas for the simple use-cases.

How to implement on iOS
I'm not familiar with the iOS side, but a quick search through the docs shows UITextInput and UIKeyInput.

With regard to mobile, I'm sure there is a solution here. It could be taken in two different directions:

  1. Replicate the platform-specific APIs so that Android and iOS developers have the exact same control as their underlying platform.
  2. Create a robust, but common interface that represents deltas on Android and iOS in the same way.

I would recommend doing both 1 and 2, but due to the time pressure that some users have, I would start with 2, which is faster, and then backport to 1.

What about other platforms?
I don't know if web and desktop offer delta information for text changes. I bet they do, because otherwise it's unclear how any rich text editors would be implemented. However, if no similar delta API is available, Flutter still needs to pipe through the mobile deltas. Web and desktop can utilize keyboard listeners, if needed, but mobile only offers a single text editing interface and it's the IME interface that goes through Flutter's text input plugin.

@matthew-carroll matthew-carroll added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Aug 10, 2021
@matthew-carroll
Copy link
Contributor Author

CC @justinmc @Renzo-Olivares

@matthew-carroll
Copy link
Contributor Author

This issue is a blocker for super_editor, and therefore a downstream product blocker for Superlist and Clearful.

@bk-one
Copy link

bk-one commented Aug 10, 2021

To confirm - this is a major blocker for Superlist and quite frankly quite surprising, as it worked fine on Desktop. Given the premise of being a real multi-platform framework, this was rather unpleasant.

@matthew-carroll
Copy link
Contributor Author

@bk-one to avoid any confusion on the Flutter side, I want to point out that this capability doesn't work on desktop. We essentially implemented our own input system in super_editor based on raw keyboard events.

This ticket relates to the Input Method Engine (IME), or similar named construct, on each platform. None of the IME-related behavior in Flutter supports the reporting of deltas. This issue, and associated work, applies to Android, iOS, web, Mac, Windows, and Linux, as far as I'm aware.

@bk-one
Copy link

bk-one commented Aug 10, 2021

Noted, I've updated my comment accordingly. Thanks for the clarification.

@Jethro87
Copy link

I can also confirm that this is a major blocker for Clearful. As a mobile app focused on writing, we have no way to provide a rich text editing experience for our users. This seriously limits the capabilities of our app and forces us to resort to limited workarounds that offer a poor UX.

@Renzo-Olivares
Copy link
Contributor

Renzo-Olivares commented Aug 18, 2021

Hello I have created a design doc you all can view here.

I have also created a created a proof of concept for the Android platform that you can check out here.

Framework PR: #88477
Engine PR: flutter/engine#28175

You can revert ba0af13 in the framework if you want to use deltaManager to listen to the deltas from a TextField. If you do this revert you can use this demo to see the deltas appear over the TextField as they come in. Or you can use this branch from my fork which includes deltaManager https://github.com/Renzo-Olivares/flutter/tree/deltaManager .

import 'package:flutter/material.dart';

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

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(title: 'Flutter Text Delta Demo'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({Key? key, required this.title}) : super(key: key);

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final TextEditingController _controller = TextEditingController();
  final TextEditingDeltaNotifier _deltaNotifier = TextEditingDeltaNotifier();

  @override
  void initState() {
    super.initState();
    _deltaNotifier.addListener(() {
      setState(() {});
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text.rich(
              TextSpan(
                children: <InlineSpan>[
                  const TextSpan(text: 'old text: '),
                  TextSpan(
                    text: _deltaNotifier.value.oldText,
                    style: const TextStyle(
                      fontSize: 40.0,
                      fontWeight: FontWeight.bold,
                    ),
                  ),
                ],
              ),
            ),
            Text.rich(
              TextSpan(
                children: <InlineSpan>[
                  const TextSpan(text: 'delta text: '),
                  TextSpan(
                    text: _deltaNotifier.value.deltaText,
                    style: const TextStyle(
                      fontSize: 40.0,
                      fontWeight: FontWeight.bold,
                    ),
                  ),
                ],
              ),
            ),
            Text('Type: ' + _deltaNotifier.value.deltaType.toString()),
            Text(
              'From ' +
                  _deltaNotifier.value.deltaRange.start.toString() +
                  ' to ' +
                  _deltaNotifier.value.deltaRange.end.toString(),
              style: const TextStyle(
                fontSize: 40.0,
                fontWeight: FontWeight.bold,
              ),
            ),
            TextField(
              autofocus: true,
              controller: _controller,
              deltaManager: _deltaNotifier,
            ),
          ],
        ),
      ),
    );
  }
}

If you would like a more in depth look on what is happening on the engine side, you can check the logcat. I print out the raw SpannableStringBuilder.replace() call with its arguments as well as other SpannableStringBuilder methods.

demo-delta

@Renzo-Olivares
Copy link
Contributor

Renzo-Olivares commented Aug 22, 2021

I have updated the proof of concept a bit, as well as the design doc. You no longer have to revert anything sorry about that. The proof of concept now uses Flutter's TextField along with a DeltaTextEditingController that behaves like a TextEditingController except with an added function to applyDeltas to the controllers value. There are currently no plans to actually implement this DeltaTextEditingController, it exists as purely a proof of concept to show how we can use the deltas as a source of truth for a TextField.

This TextField acts as a normal with the difference being that is uses TextEditingDeltas from the engine to update edits value versus just setting the entire TextEditingValue.

The raw replace() calls are still being logged in the engine as well, as the received deltas on the framework side if you would like to check the logs.

Demo part 1 Demo part 2
demo-part1 demo-part2
import 'package:flutter/material.dart';

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

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(title: 'Flutter Text Delta Demo'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({Key? key, required this.title}) : super(key: key);

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final TextEditingController _controller = DeltaTextEditingController();

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            TextField(
              autofocus: true,
              controller: _controller,
              maxLines: null,
            ),
          ],
        ),
      ),
    );
  }
}

@goderbauer
Copy link
Member

(Triage): @Renzo-Olivares could you post a status update on this issue?

@Renzo-Olivares
Copy link
Contributor

Renzo-Olivares commented Sep 15, 2021

Current status:

Core Framework API is merged.
Android engine PR is merged.
iOS engine PR is merged
Web PR is in review
Desktop platforms: Linux, MacOS, Windows, Glfw which share a common text editing model TBD

Before closing this issue Web PR should be merged as well as a Desktop PR.

@justinmc
Copy link
Contributor

Assigning myself in place of Renzo as I'm going to take a look at implementing the desktop engine pieces soon.

@justinmc
Copy link
Contributor

justinmc commented Oct 6, 2021

Here's the total work required to get this issue merged:

@justinmc
Copy link
Contributor

Still working through the different platforms. The status comment just above this is up to date.

@justinmc
Copy link
Contributor

justinmc commented Feb 2, 2022

The web, Linux, and Windows platforms are all in review and should be ticked off in the status comment soon.

@matthew-carroll
Copy link
Contributor Author

matthew-carroll commented Feb 4, 2022

@justinmc I just tried out IME delta input on Mac desktop. I'm able to enter characters, but backspace and delete don't seem to have any effect. They don't send deltas, and they don't trigger performAction(). Is this a known limitation, or did you think those were working?

EDIT:
Also, the keyboard arrow keys don't seem to report anything. I don't know what the standard communication looks like for the Mac desktop IME, but I would think that arrow keys would be as fundamental to text input on desktop as the backspace and delete keys. I would either expect the keys to cause a non-text change delta, or expect them to be reported via performAction(). Neither seems to happen.

@pulyaevskiy
Copy link
Contributor

@matthew-carroll I believe on desktop non-character keys (arrows, etc) are not handled by the TextInputPlugin but by the Intent/Action framework (which binds to the keyboard service). See DefaultTextEditingShortcuts and co. This is actually relevant to the discussion I started in #90684

Not sure about backspace and delete though.

@matthew-carroll
Copy link
Contributor Author

I'd have to hear a lot more about the basis for that division. The IME holds a lot of responsibility. It understands every character (lowercase, as well as shift + character). It offers auto-completion. It offers spelling corrections. I'd be quite surprised if the Mac OS IME lacks a response to the user pressing backspace.

If the TextInputPlugin doesn't report it because Mac OS doesn't respond to it, then I guess I can work around that. But if Mac OS reports it and then the TextInputPlugin ignores it, then that's a bug.

@justinmc
Copy link
Contributor

justinmc commented Feb 4, 2022

Indeed backspace and delete are handled as shortcuts in the framework (code). Currently you'll have to reimplement those shortcuts yourself (a quick and dirty example of that is here). I know that's pretty unrealistic, I'm working on it. I'll continue the discussion on re-exposing all of the shortcut logic over in #90684.

@matthew-carroll
Copy link
Contributor Author

@justinmc is it like that because the framework chose not to forward the OS signal, or is it because the OS doesn't provide any IME signal for that? It seems strange that iOS would send deletions but Mac wouldn't.

@justinmc
Copy link
Contributor

@matthew-carroll I looked into this and it's a choice. I think the idea was to attempt to keep most character boundary logic in the framework when possible (like if you delete a complex emoji).

@matthew-carroll
Copy link
Contributor Author

@justinmc I'd like to better understand the philosophy that's driving such API decisions. Can you describe why it's a good idea for Flutter to continue to block information from the platform at this unavoidable bottleneck?

You mentioned a complex emoji - I assume that the Mac OS IME understands complex emojis far better than I do, so if the user deletes an emoji, I'd much rather let the IME tell me what to delete, instead of calculating that, myself. Is there something about emojis that Mac OS (or other desktops) doesn't know how to handle?

I'm also confused by this approach given that iOS and Android IMEs already send a deletion signal (including for emojis), and the app has to know how to handle that. So hiding this signal isn't reducing the burden on text editing implementations. In fact, it's strictly increasing the burden.

More broadly, I'd like to repeat a concern that I've mentioned a number of times. This text input surface isn't just for the Flutter project and material text input widgets - this surface is the sole interaction that apps have with the platform's text input system. If I want information from the OS, and Flutter chooses to hide it, I'm hosed. I have been prevented from accessing what I need. I'd really like to see this fact carefully and thoughtfully addressed in a way that those of us on the outside can depend upon.

In general, why is it acceptable for Flutter to forcefully prevent app developers from responding to OS signals? And, in general, if Flutter wants to create a simplified API surface for the average use-case, why can't Flutter ignore the signals after they cross the framework boundary, instead of before?

If you'd like to discuss the nuances of this offline, let me know.

@justinmc
Copy link
Contributor

justinmc commented Feb 23, 2022

@matthew-carroll Sorry for the slow response, happy to discuss more offline.

The OS signal of "user pressed the backspace key" shouldn't be hidden from Flutter developers. You should be able to use Shortcuts or RawKeyboardListener to receive that.

Flutter's implementation of backspace is hidden from users (temporarily while we try to find a nice way to expose it). I think exposing those shortcut handlers is what you need right?

Edit: Here is where a keypress is handled in the engine for reference: https://github.com/flutter/engine/blob/d5d7526b117dc76ac1cf851ca370be9284ece351/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm#L448

@justinmc
Copy link
Contributor

Status update: All platforms should be working except for Windows, which is just held up by my messed up dev environment. Comment with full status.

@justinmc
Copy link
Contributor

justinmc commented Mar 7, 2022

I think we can consider this fixed. TextEditingDeltas is now available on all platforms except Fuchsia, which will come later. We are continuing work to make this easier to use and will track that work elsewhere.

@justinmc
Copy link
Contributor

I've opened a new issue to track the lack of reusable keyboard shortcuts that was discussed above: #100260

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2022
@flutter-triage-bot flutter-triage-bot bot added P0 Critical issues such as a build break or regression and removed P2 labels Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: text input Entering text in a text field or keyboard related problems customer: todo framework flutter/packages/flutter repository. See also f: labels. P0 Critical issues such as a build break or regression
Projects
None yet
9 participants