Refactor macOS text editing shortcuts

[chunhtai]

This pr enables widgets other than editable text to handle macos text editing shortcut.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[chunhtai]

@justinmc @Renzo-Olivares This is ready for review

[Renzo-Olivares]

Should line 352/353 go under an _iOSDisablingShorcuts or is that not within the scope of the PR?

[Renzo-Olivares]

Nit: Web -> Platform

[chunhtai]

Should line 352/353 go under an _iOSDisablingShorcuts or is that not within the scope of the PR?

I am not sure if i understand, can you explain a bit more?

[Renzo-Olivares]

I want to make sure I understand what is happening in this PR.

1. macOS default shortcuts now uses iOS shortcuts.
2. Previously there was only a disabling shortcuts widget, defined under the default shortcuts widget that applied for disabled web shortcuts. Now the common disabled shortcuts from macOS shortcuts and web shortcuts have been moved to a common disabled shortcuts map, and macOS has its own disabled shortcuts map as well with specific disabled shortcuts for macOS.

I'm not fully understanding how these changes, allows a widget other than editable text to handle the macOS text editing shortcuts. Do you mind clarifying for my sake?

[Renzo-Olivares]

Should line 352/353 go under an _iOSDisablingShorcuts or is that not within the scope of the PR?

I am not sure if i understand, can you explain a bit more?

I considered those lines also as disabled shortcuts.

    const SingleActivator(LogicalKeyboardKey.space): const DoNothingAndStopPropagationTextIntent(),
    const SingleActivator(LogicalKeyboardKey.enter): const DoNothingAndStopPropagationTextIntent(),

So what I was suggesting was something like

  static final Map<ShortcutActivator, Intent> _iOSDisablingTextShortcuts = <ShortcutActivator, Intent>{
    const SingleActivator(LogicalKeyboardKey.space): const DoNothingAndStopPropagationTextIntent(),
    const SingleActivator(LogicalKeyboardKey.enter): const DoNothingAndStopPropagationTextIntent(),
  };

  Map<ShortcutActivator, Intent>? _getDisablingShortcut() {
    if (kIsWeb) {
      return _webDisablingTextShortcuts;
    }
    switch (defaultTargetPlatform) {

      case TargetPlatform.android:
      case TargetPlatform.fuchsia:
      case TargetPlatform.iOS:
		return _iOSDisablingTextShortcuts;
      case TargetPlatform.linux:
      case TargetPlatform.windows:
        return null;
      case TargetPlatform.macOS:
        return _macDisablingTextShortcuts;
    }
  }

Edit: looks like those two lines are common to windows/linux/android/fuschia/and iOS

and remove the two shortcuts from the default iOS mapping. But this PR only targets macOS, so I understand if you weren't planning on doing that.

[chunhtai]

The goal I am trying to achieve is that. on macOS or web

1. If the shortcut is fired under EditableText, no one can handle it, because EditableText need the key event to be sent back to the platform.
2. If the shortcut is fired above EditableText, the otherwidget above it can still handle it if they want.
3. For none web or macos platform, widget above EditableText can still override these shortcut action if they want.

There are still a lot of hidden requirement, but those are use cases based

to achieve this. I created two shortcuts, inner one(disabling) and outer one(real shortcut)

Let's consider two examples:

1. if shortcut fired when focus is under EditableText
   the key event will be matched by the inner shortcut which translate to DoNothingAndStopPropagationTextIntent.
   The intent will bubble up from under EditableText and EditableText will catches that and do DoNothingAction which will stop intent bubbling and nothing will happen, so no one above the EditableText can handle this shortcut. This is needed because EditableText need to stop the shortcut system so that the keyevent can be sent back to platform to do its own thing
2. if shortcut fired when focus is above EditableText
   the key event will still be matched by the inner shortcut which translate to DoNothingAndStopPropagationTextIntent.
   but since there is no EditableText above the current focus, so it just get ignored. The key event will then be matched by the outer shortcut and translate to the correct intent. Other widget can then handle it.

[Renzo-Olivares]

The goal I am trying to achieve is that. on macOS or web

1. If the shortcut is fired under EditableText, no one can handle it, because EditableText need the key event to be sent back to the platform.
2. If the shortcut is fired above EditableText, the otherwidget above it can still handle it if they want.
3. For none web or macos platform, widget above EditableText can still override these shortcut action if they want.

There are still a lot of hidden requirement, but those are use cases based

to achieve this. I created two shortcuts, inner one(disabling) and outer one(real shortcut)

Let's consider two examples:

1. if shortcut fired when focus is under EditableText
   the key event will be matched by the inner shortcut which translate to DoNothingAndStopPropagationTextIntent.
   The intent will bubble up from under EditableText and EditableText will catches that and do DoNothingAction which will stop intent bubbling and nothing will happen, so no one above the EditableText can handle this shortcut. This is needed because EditableText need to stop the shortcut system so that the keyevent can be sent back to platform to do its own thing
2. if shortcut fired when focus is above EditableText
   the key event will still be matched by the inner shortcut which translate to DoNothingAndStopPropagationTextIntent.
   but since there is no EditableText above the current focus, so it just get ignored. The key event will then be matched by the outer shortcut and translate to the correct intent. Other widget can then handle it.

Thank you for the explanation. I think I understand both examples. I would have thought example 2 was already possible before this change if EditableText did not have focus, is this not true?

I thought the widget that receives the KeyEvent is the one focused, if that focused widget does not have a Shortcuts -> Intent mapping that handles the KeyEvent, then that KeyEvent is bubbled up. So if something above EditableText has focus it will receive the KeyEvent, and that will bubble up to the nearest Shortcut widget. Who will call Actions.maybeFind(primaryFocus?.context, matchedIntent) to find the appropriate Action in the Intent -> Actions mapping for the widget with focus.

[chunhtai]

before this change macos doesn't have a shortcut for arrow key at all, so no intent will be fired.

[Renzo-Olivares]

LGTM. Thank you for the explanations!