Text Editing Actions - SingleTapUp - #79414

Gold has detected about 2 untriaged digest(s) on patchset 6.
View them at https://flutter-gold.skia.org/cl/github/79414

@justinmc to confirm the flow:

• The TextSelectionGestureDetectorBuilder will handle double click from the gesture detector and invoke intents that map to the gesture
• The top-level DefaultTextEditingActions widget maps double click to _SelectLastTapDownPositionOrEdgeTextAction
• This action is a thin wrapper that calls out to the RenderEditable

Related to this I've got some questions:

• What is the separation of responsibility between an Action and RenderEditable? How do you decide what part of the logic lives in the action itself vs the RenderEditable?
• What is the motivation for the case statements of the pointer kind to live in the actions?

Gold has detected about 2 untriaged digest(s) on patchset 7.
View them at https://flutter-gold.skia.org/cl/github/79414

@venkatd That looks right to me, except that the double clicking action would have a different name like _SelectLastTapDownWordTextAction or something like that, assuming that's what it does.

My vision for RenderEditable is that has a low-level text editing API that is concerned only about text editing itself, and explicitly not concerned with:

• Business logic (like how Flutter behaves when you click a word)
• Gestures and keyboard keys
• Different platforms

Not all of that is true yet, but I'm trying to move in that direction.

So in the original PR #75032, I chose methods like moveSelectionLeft instead of handleLeftArrow for RenderEditable. I'd like the business logic to live in the DefaultTextEditingActions and the platform mapping. So that's why I kept the pointer kind logic in the Action.

Hi @justinmc thanks for the explanation. I'll have time to dive into all this code starting next week. I might try to refactor the double-click to use actions/intents so I get a feel for how triple click might work.

@justinmc I tried out this idea (for #79796). So far I'm having 2 problems creating an overridable and "global"-ish gesture handler (in the form of an Actions widget):

1. Gesture handlers generally have states, and since the Actions widget may be used by multiple text fields, we need to store/maintain the states separately for each text field to prevent text fields from corrupting each other's gesture states . Potential solution:
   developers can create their own inherit widget and wrap each TextField in it, so it can be accessed from the BuildContext associated with the Intent, and modified by the gesture event's corresponding action. But they have to make sure that the inherited widget isn't being shared between text fields and it has the same lifespan as the text field.
2. You can't tell an Action to not handle an Intent by not having that Intent in the actions map (because it would always delegate to its ancestor Actions), so that the corresponding gesture doesn't compete in the gesture arena. Instead we need to introduce a DoNothing action and make it special. This is a bit suboptimal. Or alternatively we can add a static method to Actions which does not delegate the action lookup to its ancestor if an Intent is not mapped to anything in the actions map.

Do you have any suggestions? I'm ok with "canonicalize" the DoNothingAction for 2, but my current solution to problem 1 is complicated and error-prone I feel.

Examples for more context:

1. If the default action for handling "long-press start" sets an internal flag, and the default action for handling "long-press move" depends on the flag, the developer would have to override both actions even if they only wanted to override the functionality of one, and they probably won't find out what are the actions they need to override in order to achieve the desired behavior, until they discovered there's a bug and read through our implementation to figure out what are the internal states they need to re-create in their implementation.
   From what I'm seeing, currently it seems possible to create a default implementation that's internal-state free (basically we have enough public states such as selection. E.g., when we drag to extend a selection we can keep the baseOffset and extend the extendOffset to the current drag location, instead of using a private flag to track the drag start location). But it might not always be the case.

2. Say on desktop we don't want to handle the "long press" gesture family. We can't map the long-press intents to actions that do nothing, because that way the long press gesture recongizer will still be able to participate in the gesture arena to compete with, say, tap gestures, and win (and do nothing). We need something that allows us to determine at build time whether a gesture should be handled. We can extend the current TextEditingGestureBuilderDelegate to answer such questions but I think it will be nice if we can handle that in the global default text actions (since it's most likely going to be platform-dependent and one of the goals for text editing actions is to provide sensible defaults on different platforms).

In response to #79414 (comment):
1 . I think the state needed by an Action should be passed along via the Intent.
2. I agree that making it easy to defeat an Action, for example by mapping an Intent to null or a DoNothingAction, needs to be easy.

I think we need an inherited widget like ShortcutManager that records the mapping from Intents to Actions for mouse and touch input gestures. I suggest GestureShortcutManager.

Recognizing mouse/touch input gestures is complex so I'd suggest leaving that to widgets (and their renderers). So GestureShortcutManager would not have a method that's comparable to handleKeyPress.

Here's the data flow. It's similar to what's been proposed before.

• Widget detects a gesture that it cares about, like right-button-down
• The widget (or its renderer) creates a TextFieldRightButtonDown intent with whatever info an Action might need
• The widget looks up the Action for TextFieldRightButtonDown with the GestureShortcutsManager inherited widget
• In this case that would be a ShowTextfieldMenu action by default
• It invokes the action using ActionDispatcher.invokeAction.

One notable difference is the meaning and naming of intents and actions. The intent identifies the input gesture and its source: it's a right button down event that occurred in a TextField. The action identifies the type of the widget it's to be applied to and what it actually does. The action's name does not include the input gesture. I would hope that we would try and create actions that can be reused in other contexts, or customized by subclassing.

An app could replace or defeat this binding by creating a shadow GestureShortcutsManager that mapped TextfieldRightButtonDown to a custom action or a DoNothingAction. Using null for "do nothing" would be convenient, since it obviates creating a DoNothingAction of the right type.

Some applications might want to change the mapping from input gestures to intents, for example to pass along additional information to the action. It doesn't seem important to deal with this case immediately however we could make it possible to specify a delegate class that did all of the mappings:

TextFieldIntentDelegate {
   TextFieldRightButtonDown? forRightButtonDown(...)
   TextFieldTap? forTap(...)
   TextFieldDoubleTap? forDoubleTap(...)
   // ...
}

To handle gestures that correspond to a series of events, notably press-drag-release gestures, the widget could invoke the same Action repeatedly, with differently configured intents. For example, to handle a press-drag-release gesture that changes the selection:

• Widget detects left-button-down or tap-down
• It creates a TextfieldPressDragRelease intent with whatever info an Action might need
• It looks up the Action TextfieldPressDragRelease in a GestureShortcutsManager inherited widget
• In this case that would be a TextfieldDragSelect action
• It invokes the action repeatedly on TextfieldPressDragRelease intents beginning with an intent that represents the press, then as many as necessary for the drag, then a final one for the release.

I realize this is all pretty sketchy and I may not have addressed all of the issues raised in #79414 (comment). Hopefully I've propelled the discussion forward.

Gold has detected about 2 untriaged digest(s) on patchset 8.
View them at https://flutter-gold.skia.org/cl/github/79414

I'm currently trying to put a RawGestureDetector in an EditableText, and get its <Type, GestureRecognizerFactory> map from a new InheritedWidget TextEditingGestures, so you get to customize/override gesture recognizers in an EditableText (different platforms support different gestures, e.g., iOS supports triple click and force touch but android does not):

class TextEditingGestures extends InheritedWidget {
  final Map<Type, GestureRecognizerFactoryWithBuildContext> gestures;
  // If true `maybeOf` merges `gestures` with the gestures from its ancestor `TextEditingGestures`
  final bool inherit;
  static Map<Type, GestureRecognizerFactory>? maybeOf(BuildContext context);
}

The gesture recognizers in the default map are set up to fire intents to invoke pre-defined actions (retrieved from the EditableTextState's BuildContext).
I'm leaning towards naming the text editing intents and actions after what they do instead of the user interaction that triggered them, and they should not have secret/private states that may affect other actions in an unexpected way (So we'll have DeleteIntent - DeleteAction, ShowToolBarIntent - ShowToolBarAction).
that way we'll actually have an API for changing a text field's content (instead of doing that in RenderEditable), and they're overridable to some extent.

But I'm not exactly sure how best to implement an Action or an Intent that has secret/private states:

• store states in an ActionDispatcher: doesn't not seem suitable for most private states. E.g. the base text position of a drag gesture
• store states in an Intent: ok but something needs to hold the intents. Letting gesture recognizers hold those intents are suboptimal since they may have different lifespans.
• store states in an Action: sounds ok if we have a private action for each text field. But if someone wants to have an Action that covers multiple text fields they would have to store states for each text field in a map and when one of the descendant text fields is removed from the tree the action needs to clean up the states.

@LongCatIsLooong would this design help you? It would allow definition of actions that could be invoked be something that didn't have focus, just using the intent.

I've also been musing with the idea of a similar registry for shortcuts, which should allow definition of the shortcut binding to an intent at a lower level where state is available instead of requiring it to be hoisted high enough for the definition.

Then both actions and shortcuts could be defined privately, and be registered and effective at the level of the widget containing the registry and below.

@gspencergoog for now I don't think I'm relying on the focus system for implementing gestures (just using CallbackActions and invoking it using Actions.maybeInvoke). I looked at the API docs and the only thing that depends on the focus system seems to be ContextAction?

Yes, that's right. If you aren't using the focus system, then the new design probably won't matter to you.

With a DefaultTextEditingActions sitting at the root of the widget tree we'll be able to override the default Intent-Action mapping in descendant widgets, but

1. we'll have a hard time finding the right place to store states, since the Actions in the map need to be stored in a globally visible position (it's not impossible just requires a lot of boilerplate code and is error-prone). Maybe we should use an Intent-ActionBuilder map instead, to build and store the Action?
2. The Actions in the map need to rely on the current primaryFocus to get the BuildContext of the text field. These Actions don't know what the BuildContext is capable of - currently it needs to do a bunch of downcasting to turn that BuildContext to a TextEditingActionTarget to access the underlying RenderEditable.

As I understand it Actions provides a way to dispatch an Intent from a BuildContext to its ancestors. IOW it can give a BuildContext the ability to invoke a function when given a supported type of Intent.
So I'm thinking maybe we should make the actions local to each text field. E.g., the EditableTextState class can embed its own Actions widget, to respond to DeleteTextIntent and UpdateTextEditingStateIntent as it knows how to delete and update. And TextField can embed an Actions widget to respond to ShowTextEditingTooltipIntent.

Pros:

1. The actions will be able to store states, they have the exact lifespan as the text editing component that owns them, since they're embedded in these components as part of the tree. These actions are basically public methods that can be invoked using an Intent (or private if you make the Intent subtype private). To enable more complex logic we can try letting text editing intents encode the return type, e.g.: ShowTextEditingTooltipIntent<Bool> would allow the caller to know if the "TextEditingTooltip" action is performed successfully.
2. These actions don't have to downcast their BuildContexts as they're mostly just going to be CallbackActions and can just capture the EditableTextState or TextFieldState as free variables.

Cons:

1. You won't be able to override/disable the pre-defined Intent-Action map (but do we really want to override what DeleteTextIntent does?), since the text editing widgets are all leaf widgets. What you can do is to change the shortcut-intent mapping for keyboard shortcuts and the gesture-intent mapping for gestures.
2. You won't be able to access the EditableTextState in your Intent or the gesture recognizer, most of the logic has to live in the defined Actions.
3. Supported Intents need to be documented and do not have great discoverability and no autocomplete for them as public methods would (basically all the dynamic dispatch woes).

An example, to add force press support:

1. Add a TextEditingGestures with a ForcePressGestureRecognizer . The gesture recognizer fires an StartForcePressIntent on onStart, etc, from the BuildContext that corresponds to EditableTextState (where the RawGestureDetector was built).
2. Add an Actions widget with a map { StartForcePressIntent: CallbackAction(...), MoveForcePressIntent: CallbackAction(...), EndForcePressIntent: CallbackAction(...) } where the private force touch states are maintained in EditableTextState (or even a state/element class provided by a 3rd party widget). It's actually a good thing here that these Actions can't be overriden, so that people don't map the StartForcePressIntent to a different Action, and breaks MoveForcePressIntent EndForcePressIntent.

** Update **:
The implementation seems to be quite straightforward: master...LongCatIsLooong:text-editing-gestures . For instance, to add secondary tap support, one just has to configure their own gesture recognizer:

  static final ContextGestureRecognizerFactory<TapGestureRecognizer> secondaryTapRecognizer =  ContextGestureRecognizerFactory<TapGestureRecognizer>.withFunctions(
    constructor: (BuildContext context) => TapGestureRecognizer(debugOwner: context),
    initializer: (TapGestureRecognizer recognizer, BuildContext context) {
      recognizer.onSecondaryTapUp = (TapUpDetails details) {
        Actions.invoke(
          context,
          SelectTextAtPositionIntent(
            fromPosition: details.globalPosition,
            textBoundaryType: TextBoundary.word,
            cause: SelectionChangedCause.tap,
          ),
        );
        Actions.maybeInvoke(context, SelectionToolbarControlIntent.show);
      };
    }
  );

and add a TextEditingGestures widget.

The current behavior was relatively easy to re-implement. 2 things I found may not be as easy to implement:

1. on iOS, most gesture recognizers are disabled if multistage input is in progress (i.e. there's a non-empty composing region), except (let's say) single tap, which changes the location of the caret. So a long-press should be recognized as a tap. To do this we need the ability to configure the gesture recognizers based on the text field's state. Since we have access to a BuildContext this is possible (but can be a bit annoying since to expose TextEditingValue on the BuildContext you'll need a new InheritedWidget).
2. on iOS, double-tap-drag extends the selection by words. This requires the tap gesture recognizer passing states to a drag gesture recognizer. But this is more of a gesture recognizer problem I think (we need a double tap ).

Current progress: https://github.com/flutter/flutter/pull/84232/files

Closing this PR as most of its functionality should be superseded by #84232 and related work.