flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
166.32k stars 27.53k forks source link

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

Closed matthew-carroll closed 2 years ago

matthew-carroll commented 3 years ago

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 commented 3 years ago

CC @justinmc @Renzo-Olivares

matthew-carroll commented 3 years ago

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

bk-one commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

Jethro87 commented 3 years ago

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 commented 3 years ago

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: https://github.com/flutter/flutter/pull/88477 Engine PR: https://github.com/flutter/engine/pull/28175

You can revert https://github.com/flutter/flutter/pull/88477/commits/ba0af13fdea43e66fa6cfee61d51a3a15a70bf63 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 commented 3 years ago

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 commented 3 years ago

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

Renzo-Olivares commented 3 years ago

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 commented 3 years ago

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

justinmc commented 3 years ago

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

justinmc commented 3 years ago

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

justinmc commented 2 years ago

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

matthew-carroll commented 2 years ago

@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 commented 2 years ago

@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 https://github.com/flutter/flutter/pull/90684

Not sure about backspace and delete though.

matthew-carroll commented 2 years ago

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 commented 2 years ago

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 https://github.com/flutter/flutter/pull/90684.

matthew-carroll commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

I've opened a new issue to track the lack of reusable keyboard shortcuts that was discussed above: https://github.com/flutter/flutter/issues/100260

github-actions[bot] commented 2 years ago

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.