@chunhtai Let's see if we like this one

MultiSelectableSelectionContainerDelegate is abstract and requiring users to create their own delegate class for that behavior seems overly complicated. Maybe we need a new intermediate widget to handle it?

Something like this maybe?

Removed for readability

Well that code didn't work and these delegates seem overly complicated to design. Can we just reuse the _SelectableRegionContainerDelegate?

class SelectionInterceptor extends StatelessWidget {
  const SelectionInterceptor({
    super.key,
    required this.copyInterceptor,
    required this.child,
  });

  final CopyInterceptor copyInterceptor;
  final Widget child;

  @override
  Widget build(BuildContext context) {
    return SelectionContainer(
      registrar: SelectionContainer.maybeOf(context),
      delegate: _SelectableRegionContainerDelegate(copyInterceptor: copyInterceptor),
      child: child
    );
  }
}

@Rexios80 I agree, I think having to create a delegate right now is complicated. We can make it easier for users to create a SelectionContainerDelegate if we re-used _SelectableRegionContainerDelegate and made it public.

Then they can do the following, without an intermediate widget:

SelectionContainer(
  delegate: MySpacesCopyInterceptorSelectionContainerDelegate(),
  child: Row(
    children: <Widget>[
      const Text(
        'Text widget 1',
      ),
      const Text(
        'Text widget 2',
      ),
      const Text(
        'Text widget 3',
      ),
      const Text(
        'Text widget 4',
      ),
    ],
  ),
),

class MySpacesCopyInterceptorSelectionContainerDelegate extends _SelectableRegionContainerDelegate {
    @override
    CopyIterator get copyIterator => CopyIterator.spaces;
}

What do you think?

I think having users create any class at all is too much work to achieve this behavior. If the SelectableRegionContainerDelegate is public couldn't they just use it directly instead of subclassing?

I think having users create any class at all is too much work to achieve this behavior. If the SelectableRegionContainerDelegate is public couldn't they just use it directly instead of subclassing?

I think your code in this post works #146625 (comment) , but I don't think we need an intermediate widget if we are making SelectableRegionContainerDelegate public.

My other concern is similar to https://github.com/flutter/flutter/pull/146625/files#r1563001791 . For Text, exposing copyIterator may sometimes produce unexpected behaviors because a Text widgets RenderParagraph will internally split its text into multiple Selectables in the case when WidgetSpans are involved. In that case i'm not sure how useful exposing copyIterator is here.

I think the expected behavior of copyIterator on SelectionArea/SelectableRegion can also be unclear when you have a complex layout with many widgets that may be selectable. This is because a selectable may be either a single Selectable, or a SelectionContainer that may manage multiple Selectables, so it's not clear what widgets copyIterator is actually affecting when declared under a SelectionArea/SelectableRegion. This has me leaning towards keeping this functionality configurable through wrapping a desired subtree with a SelectionContainer and a configured delegate.

If that's the case then we can just create a delegate such as this

class CopyInterceptorContainerDelegate extends _SelectableRegionContainerDelegate {
  CopyInterceptorContainerDelegate({
    required this.interceptor,
  });

  final CopyInterceptor interceptor;

  @override
  SelectedContent? getSelectedContent() {
    final List<SelectedContent> selections = <SelectedContent>[];
    for (final Selectable selectable in selectables) {
      final SelectedContent? data = selectable.getSelectedContent();
      if (data != null) {
        selections.add(data);
      }
    }
    if (selections.isEmpty) {
      return null;
    }
    return SelectedContent(plainText: interceptor.intercept(selections));
  }
}

and revert the rest of the changes in this PR. My concern with doing it this way is user discoverability of this feature. With a copyInterceptor field discoverability is easy and we can even provide default behavior.

@Rexios80 I understand your concern about discoverability but I'm a little hesitant to add copyIterator to widgets were the behavior might be ambiguous. I think we should still keep the defaults you have defined in this PR as they are useful. The defaults for Text and Scrollable are reasonable, though I am unsure about having a default for SelectionArea/SelectableRegion other than none, what do you think?

We should also make it easier for users to create a SelectionContainerDelegate so they can easily wrap subtrees with a SelectionContainer with custom handling. If someone wanted to change the "global" copyIterator like this PR does in SelectionArea.copyIterator, one would just need to wrap the child given to SelectionArea with a SelectionContainer with their custom delegate.

I think we could always add a copyIterator field to these widgets later if we see a need for it.

@Renzo-Olivares I agree that having the default separator as none for SelectionArea/SelectableRegion makes more sense, however I'm not sure I agree with removing the copyInterceptor constructor parameter entirely. Removing that parameter would require users to always use 2 widgets to create a SelectionArea with proper copy support unless the child widget is one of the ones we are adding copy interceptors to in this PR.

I also agree that it makes more sense to use SelectionContainers for nested interceptors instead of nesting SelectionAreas, but I'm still considering having a separate widget so it's easier to discover/use.

I'm not sure we would need to "penetrate" SelectionContainers. If we were to add a SelectionContainer to a widget that would need a copyInterceptor, its delegate would extend the MultiSelectableSelectionContainerDelegate which forces you to implement the copyInterceptor parameter.

I'm not sure we would need to "penetrate" SelectionContainers. If we were to add a SelectionContainer to a widget that would need a copyInterceptor, its delegate would extend the MultiSelectableSelectionContainerDelegate which forces you to implement the copyInterceptor parameter.

What I meant is if we add a SelectionContainer for a inside a widget, it will override all copyinterceptor above.

My Idea is we can do it through inheritedwidget, so that customer would have full control over the copy. One Idea is to add it to DefaultSelectionStyle

@chunhtai I made a proof of concept for using a widget for this. Please let me know what you think.

Hey @Rexios80 thanks for the updates. It looks like there are some failing checks, can you take a look?

@Piinks I've already rewritten this PR 4 times. I don't want to put effort into fixing CI if we don't like the idea.

@chunhtai do you have any thoughts?

(Triage) Hi @Rexios80 what is the status of this PR?

@Piinks to move on I need to know what method we want to use to accomplish this. If someone could point me in the right direction I can rewrite this (hopefully) one last time.

Going along with what @chunhtai and @Renzo-Olivares suggested, I'm probably going to rewrite this PR using DefaultSelectionStyle to hold the copyInterceptor.

I just haven't had time yet. If anyone has an objection to this approach or would like to implement it themselves please comment.

I will make a new PR when I have a chance to make the new implementation