[flutter] hacked together global selection example by jonahwilliams · Pull Request #81849 · flutter/flutter

jonahwilliams · 2021-05-04T18:22:22Z

This is a hacked together implementation of #81839 for the purposes of exploring the problem space.

Updated: Updated:

gallery.2021-05-12.10-22-28.mp4

chunhtai

I think this is going in a good direction, although we may need to figure out how this can integrate with our current textediting logic. We are facing an issue that we can't separate out text selection from text editing in the textfield because they are tightly bound together, and here is an on going refactoring to make that easier cc @LongCatIsLooong .

chunhtai · 2021-05-04T18:38:17Z

                    locale: appLocale,
                    delegates: _localizationsDelegates.toList(),
-                    child: title,
+                    child: SelectionArea(child: title),


I wonder whether we should make this to be per modalRoute thing

Ahh, that would probably solve the problem of being able to select text on different routes...

Where would you put this in the router/routing/navigator to essentially wrap each route in its own?

I was imagine we can probably put this in _ModalScope, but I am also worry about whether there is a use case when user want to select across different routes?

To construct one: One bigger desktop screens you may have a two column layout where the left and right side show different Navigator stacks. Do we need to support selecting across both?

Ahh, this works great actually, solves the problem of highlighting old routes. I'm not sure what the use case would be for straddling different modal routes TBH

chunhtai · 2021-05-04T18:41:23Z

+
+  @override
+  void update(RenderParagraph renderBox, Rect rect) {
+    var tl = renderBox.localToGlobal(rect.topLeft);


Maybe there can be an API to let SelectionRegistrant to decide selection rect given the pointer start and end position?

chunhtai · 2021-05-04T18:45:32Z

+  @override
+  void paint(PaintingContext context, Offset offset) {
+    super.paint(context, offset);
+    for (var entry in _state._selectionStates.entries) {


I wonder whether we need to worry about something been draw on top of the selection, we can also make this an overlay entry that either been part of the modalRouter overlayEntries or a CompositedTransformFollower.

Yes, definitely took some short cuts with this one 😅

jonahwilliams · 2021-05-04T18:54:48Z

I think this is going in a good direction, although we may need to figure out how this can integrate with our current textediting logic.

Definitely - one way to think about unification would be for each editable text to introduce its own SelectableArea. That could allow us to bind the selection region to that specific field if you're editing.

goderbauer

Wondering: Instead of drawing the selection in a global place high up the tree could we turn this upside down, i.e. interested Widgets can register their interest in being selectable as part of global select. When the SelectionArea then detects a global select gestures, it tells all interested widgets that fall in the user-selected area that they have been selected (giving them a rect to indicate the area inside their own bonds that's considered "selected"). The individual widgets can then decide how they want to render this selection state. This would help with drawing the selections in the right order and non-text widgets could also participate with custom "selected" states/

goderbauer · 2021-05-04T19:12:25Z

+  _SelectionAreaState createState() => _SelectionAreaState();
+
+  static SelectionRegistrant? of(BuildContext context) {
+    return context.findAncestorStateOfType<_SelectionAreaState>();


Could be an InheritedWidget for faster lookups?

goderbauer · 2021-05-04T19:18:31Z

+      gestures: _gestureRecognizers,
+      behavior: HitTestBehavior.translucent,
+      excludeFromSemantics: true,
+      child: RenderHackyTextSelectionWidget(this, widget.child),


Could maybe be replaced by a CustomPaint?

goderbauer · 2021-05-04T19:19:34Z

+  }
+}
+
+class _SelectionState {


nit: *State is a not an ideal name for this given that State objects usually belong to StatefulWidgets.

goderbauer · 2021-05-04T19:30:35Z

Presumably, we also need a way to opt out? For example, on the web it seems that buttons are not participating in global selection. In Flutter, they'd be using Text widgets.

jonahwilliams · 2021-05-04T19:32:13Z

Yeah - for the opt out we could provide some sort of flag / setting that ends up with text not registering itself with the selection area. That would cause it to be otherwise ignored. We would need to update the framework defaults

jonahwilliams · 2021-05-04T19:37:37Z

Wondering: Instead of drawing the selection in a global place high up the tree could we turn this upside down

I can see that working - with the caveat that I don't think we necessarily want to expand Text to include all of the parameters that would be necessary to theme that selection. Unless of course that was only able to be configured via some inherited widget.

It would also be odd if different text selection runs had, say, different colors for example

skia-gold · 2021-05-04T19:38:54Z

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

goderbauer · 2021-05-04T19:45:54Z

It would also be odd if different text selection runs had, say, different colors for example

I think that's allowed, though: https://www.w3schools.com/howto/howto_css_text_selection.asp (select across the table under "Select the following text:")

goderbauer · 2021-05-04T19:49:14Z

(Just saw the video added to the PR description): The current implementation selects too much too eagerly right? "Animation" and "2D Transformation" shouldn't be selected until the mouse has moved below it, I think?

jonahwilliams · 2021-05-04T19:52:05Z

(Just saw the video added to the PR description): The current implementation selects too much too eagerly right? "Animation" and "2D Transformation" shouldn't be selected until the mouse has moved below it, I think?

Yes the logic on what to select is totally bonkers right now, but getting there ...

jonahwilliams · 2021-05-04T19:52:59Z

I will update the example to push the selection painting down to the paragraph layer.

jonahwilliams · 2021-05-04T20:14:28Z

Ahh, and of course drawing the selection overlays "on top" does not work when you scroll

jonahwilliams · 2021-05-04T20:49:14Z

Moving selection box drawing into the paragraph objects themselves is much simpler. Still need to handle the correct order for assembling the text elements into a reasonable copy paste output. You could sort of concatenate it all together based on top to bottom, left to right? Need to think about it some more.

goderbauer · 2021-05-05T17:03:31Z

+  void _onCopy() {
+    if (_selectionStates.isEmpty)
+      return;
+    // The order in which these should be concatenated is ???. For now this


I actually came here to ask the same question. How do we determine the order the content of the "selected" widgets should appear in in the copy buffer?

Yes, this one is a real quandary! Right now I am sorting by the topLeft offset of the render objects bounding rect. Something we can talk about today - but there is probably room for gradually adjusting this algorithm based on feedback

I tried on html, it seems like it uses div order https://www.w3schools.com/code/tryit.asp?filename=GQ94272JBPA3
Maybe we can just use widget order and web developers may already be used to it?

Definitely worth trying - though what is interesting in that example is how the selection boxes are calculated. Maybe this is actually more of a hit test special case

Each widget (widget's renderer) could report their selection value and the rectangle within the overall selection rectangle that their selection value came from. The overall selection widget could assemble the contributions top to bottom and sorted horizontally according to the text direction.

jonahwilliams · 2021-05-05T17:07:19Z

+  bool update(Offset start, Offset end) {
+    final Offset localStart = globalToLocal(start);
+    final Offset localEnd = globalToLocal(end);
+    final TextPosition startText = getPositionForOffset(localStart);


This doesn't quite do what I want if start/end don't really intersect with the text bounding box

skia-gold · 2021-05-05T17:07:28Z

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

skia-gold · 2021-05-05T17:46:28Z

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

jonahwilliams · 2021-05-05T18:21:43Z

+  const ClipboardData({ this.text }) : dataType = ClipboardDataType.text, image = null;
+
+  /// Creates image data for the system clipboard.
+  const ClipboardData.image(this.image) : dataType = ClipboardDataType.image, text = null;


I don't plan to fully implement this right now, adding to help design selection for multiple data types

The support for this will be platform dependent, android doesn't support multimedia clipboard while windows/macOS/ios supports some multimedia I believe.

skia-gold · 2021-05-05T19:46:27Z

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

LongCatIsLooong · 2021-05-05T22:42:59Z

+
+  @override
+  bool update(Offset start, Offset end) {
+    final Rect localRect = Rect.fromPoints(globalToLocal(start), globalToLocal(end));


I think in theory we need to compute the intersection of the visible (interactable rather) part of the render object of interest, and the select rectangle right? So a full traversal from the root is needed to figure out whether/which part of the render object was painted and visible in case:

the render object isn't visible at all

the render object is being clipped and only part of the copyable content is visible (in which case I think we should only highlight and extract content from the visible part that's selected right?). For RenderEditable it knows to some degree how it was clipped but might not be the case for other render objects that can contribute content.

I think extending the RenderObject protocol to add the capability of extracting content from within a given rect (and maybe order its children's contents in their logical order instead of visual order so we can assemble the contents) will help us solve similar problems (e.g., #75472). A full traversal may not be that bad performance-wise:

we only care about contents on screen so we can short-circuit early during the traversal for many render objects,

for gesture-based selection, the pointer event for the current drag gesture can update at most once per frame I think (unless we want to implement the multi-selection/caret in google docs)?

This would definitely be useful, but i would be concerned about being too top down? I'm not quite sure. if we could basically hit test in a rect area that would solve this problem exactly

LongCatIsLooong · 2021-05-05T23:08:41Z

+    }
+    _textSelection = TextSelection(baseOffset: startText.offset, extentOffset: endText.offset);
+    markNeedsPaint();
+    return true;


nit: I tried selecting something close to the edge of the bottom of the screen on a web page and the page scrolled to reveal the content below. So maybe call RenderObject.showOnScreen.

skia-gold · 2021-05-05T23:49:27Z

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

jonahwilliams · 2021-05-05T23:55:33Z

I did walk things back to the inherited widget (disclaimer, I know I just used the state lookup method) style. Currently have some selection bugs with it but I'll see how far I can take it.

jonahwilliams · 2021-05-05T23:59:29Z

Is there a good way to observe navigation changes without connecting to the app level navigator? I'd like to make sure selection is disabled for non-visible routes (I guess that problem is also solved with the hit test method)

…tion

skia-gold · 2021-05-11T00:05:20Z

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

skia-gold · 2021-05-12T06:08:20Z

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

jonahwilliams · 2021-05-12T17:25:09Z

So I managed to get @HansMuller 's original suggestion of delegating entirely to the render objects for selection to work. I'm currently looking at how I could expand this support to work well with render editable and possibly share some of the existing logic.

I looked more into the hit testing change and I think it would be too drastic at least for now.

skia-gold · 2021-05-12T22:26:21Z

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

jonahwilliams · 2021-05-19T16:59:03Z

The status of this is that it works up until you hit a page with a text field, and then you hit #32433 which makes Ctrl+C and Ctrl+A no longer work. I wonder if shortcuts is the wrong approach?

…tion

skia-gold · 2021-05-20T02:45:12Z

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

chunhtai · 2021-05-26T21:46:06Z

Blocked on #82575

goderbauer · 2021-07-14T21:50:22Z

(triage): What is the status of this PR? The blocking bug appears to be closed?

jonahwilliams · 2021-07-15T18:23:09Z

@goderbauer I would appreciate if your triage meeting could skip drafts. Opening one was important to gather initial feedback and I'd rather not close it again and lose the context while I keep working on that

[flutter] hacked together global selection example

8f0c06d

flutter-dashboard Bot added framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels May 4, 2021

google-cla Bot added the cla: yes label May 4, 2021

chunhtai reviewed May 4, 2021

View reviewed changes

revert cache change

b205277

goderbauer reviewed May 4, 2021

View reviewed changes

put in modal route

2d32239

move selection boxes to paragraph

af62178

jonahwilliams added 2 commits May 4, 2021 14:22

handle copy out

07c4a1f

move selection logic into widgets

326d837

goderbauer reviewed May 5, 2021

View reviewed changes

jonahwilliams commented May 5, 2021

View reviewed changes

Draft of how to do image selection

a28a6b9

jonahwilliams commented May 5, 2021

View reviewed changes

jonahwilliams added 2 commits May 5, 2021 12:39

make selection based on hit testing

40b8073

oops

f4d4459

LongCatIsLooong reviewed May 5, 2021

View reviewed changes

back to inherited something

4e5afa1

jonahwilliams added 2 commits May 10, 2021 14:06

Merge branch 'master' of github.com:flutter/flutter into global_selec…

c823700

…tion

use shortcuts

fe75621

cleanup and make it work

d9f1e6d

make this work for render editable too

273251b

jonahwilliams added 2 commits May 19, 2021 19:19

Merge branch 'master' of github.com:flutter/flutter into global_selec…

deae282

…tion

use existing focus node

0b652db

Piinks added a: text input Entering text in a text field or keyboard related problems a: typography Text rendering, possibly libtxt a: quality A truly polished experience labels Jun 10, 2021

jonahwilliams closed this Jul 15, 2021

jonahwilliams deleted the global_selection branch July 15, 2021 18:35

jonahwilliams mentioned this pull request Jul 19, 2021

text selection for the web #86664

Closed

justinmc mentioned this pull request Aug 17, 2021

Proposal: Simple custom text selection toolbars #73574

Closed

justinmc mentioned this pull request Aug 25, 2021

text selection reacts only to SelectableText bounds #78909

Closed

Conversation

jonahwilliams commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented May 4, 2021

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer commented May 4, 2021

Uh oh!

jonahwilliams commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams commented May 4, 2021

Uh oh!

skia-gold commented May 4, 2021

Uh oh!

goderbauer commented May 4, 2021

Uh oh!

goderbauer commented May 4, 2021

Uh oh!

jonahwilliams commented May 4, 2021

Uh oh!

jonahwilliams commented May 4, 2021

Uh oh!

jonahwilliams commented May 4, 2021

Uh oh!

jonahwilliams commented May 4, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HansMuller May 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skia-gold commented May 5, 2021

Uh oh!

skia-gold commented May 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skia-gold commented May 5, 2021

jonahwilliams commented May 4, 2021 •

edited

Loading

jonahwilliams commented May 4, 2021 •

edited

Loading

HansMuller May 5, 2021 •

edited

Loading