https://kotlinlang.org logo
Join Slack
Powered by
Click handling inside SelectionContainer is still ...
# compose-desktop
o
Click handling inside SelectionContainer is still a big problem for us. This is the issue: https://issuetracker.google.com/u/1/issues/184950231 But it was duplicated to another one from 2019, and got buried under a lot of different worries like internationalization. We don’t use ClickableText, we have our own component for that. It also marked as Feature Request, while I think inability to handle taps inside SelectionContainer is more a bug than a feature request. Can someone from the team clarify if it’s possible to fix this specific issue (selection intercepting all gestures, even if not selection-like), or we are out of luck here? The worst part is that I couldn’t find any workarounds…
s
I think this is probably better addressed to #compose . If I'm not mistaken it's an issue for Compose for Android as well. I'm very interested in seeing a resolution to this because it's a significant bug in one of my apps as well.
j
@Sean McQuillan [G] @Siyamed Any chance you can weigh in here?
s
I am not sure, it has been a while. As far as I remember, there was a missing capability in the gesture/pointer handling that we could not control from the text side. There were many discussions as well. Ilya also mentioned this in their comment (that it is not only the selection or links)
j
cc @matvei @George Mount
s
It looks like Ilya had some ideas "On desktop, it even can be implemented to automatically decide what to do, not sure about Android." I wonder what the solution they have in mind is and if they can send a doc/CL ?
o
I mean, text with links and selection work in browsers (desktop or mobile), so sure it is a solvable problem 🙂 I don’t know enough about all the gestures detection, esp on Android, so can’t suggest anything useful… Maybe special mode for SelectionContainer to activate selection on long tap for mobile, and on mouse-down + drag on desktop?
s
Frankly, I dont like the idea of desktop vs android. Android works with mouse etc.
Of course it is solve-able. ilya what is the request here? Is it solve asap? if so currently it doesnt seem feasible considering resources and the focus.
o
I’m not asking for anything ASAP, for sure. Maybe an idea of a workaround. Maybe just general course of action, like “we have this on the radar for next iteration for text & gestures”. It’s been there for a while (11 Apr 2021), so I wonder if it’s a very low priority, because nobody needs it except few geeks 🙂 Or it’s too big problem with not much use cases.
s
The general Selection has been very low priority compared to text layout, editing, performance and selection in text editing. For this specific case I really dont know how deep the problem is at the moment or what should change but I would suggest to either start a new ticket or star the one linked with a comment. We do take the stars in bugs into account try to prioritize them and mostly do prioritize the top stared tickets (for example there was one with 500+ stars).
I empathize on the frustration on some missing features, not-ideal behaviors or bugs; it would frustrate me as well. On the other hand what we name as text includes many subcomponents/features and it is not possible to tackle more than we tackle now.
o
I’m not an expert in any way in text and selection, but it looks to me the issue is in modifiers’ order. 
TextController
has this:
Copy code
val modifiers: Modifier
        get() = coreModifiers
            .then(semanticsModifier)
            .then(selectionModifiers)
and it is then applied to BasicText’s Layout as 
modifier.then(controller.modifiers)
, which (if I understand it right for 
Main
pass) means 
pointerInput
of 
selectionModifiers
is handled first and consumes point up and down events. Whatever pointer input handling 
modifier
passed to BasicText has – it doesn’t have a chance to handle things. So, the workaround could be handling tap gestures on the 
Initial
pass on the BasicText’s modifiers, but it looks like pretty complex thing to do with all the logic in tap gesture detection, drags and so on. And also not sure it is a “good enough” way to work it around, or even would work. Or maybe 
selectionModifiers
need to handle pointer events on the 
Final
pass. Just thinking out loud looking into debugger :)
s
@Zach Klippenstein (he/him) [MOD] FYI
c
I'm no text expert, so take this with a grain of salt, but it looks like the 
SelectionManger
consumes the event (the 
SelectionContainer
) uses 
PointerInputScope.detectDragGesturesAfterLongPressWithObserver()
(in 
TextDragObserver.kt
) which uses the 
PointerInputScope.detectDragGesturesAfterLongPress()
and consumes the event there... which I assume is why the children Composables aren't getting the events?
I assume there is some magic going on in there for 
SelectionManager
to consume that before child composable, so I will let Zach comment on what that is.
o
There is no child composable. Selection container sets some local, and core text itself appends some modifiers for tracking selection interactions.
The first click (when mouse is active) sets zero-range selection and consumes events. It is needed for shift-click to extend selection. It also counts clicks for double and triple clicks to select word or paragraph. It works the same in browser, so logic is correct. However, in browser links (and I assume other custom handlers) takes precedence. That’s why links work, and you can’t start selection over links in browsers – drag for example starts link dragging instead of selection expansion.
Note, that we can’t just swap modifiers, because layout modifiers should go before point input modifiers (correct me, if I’m wrong). So quick and dirty fix could be to pass additional “overrideModifier” to BasicText/CoreText, which would go after 
controller.modifiers
. This way input modifiers can get a chance to handle clicks. But it would look terrible in the API 😞
z
layout modifiers should go before point input modifiers
I don’t think it should matter. I’ll try swapping the order and see if that fixes it
I couldn’t reproduce this on Android. Will try on desktop next.
o
As far as can see from code, touch device will await long tap to start selection. So if one wants to use long tap on eg link in text to show some context action (share, open in browser, etc) - they wouldn’t be able to. But I don’t have a device or setup Android environment to test it.
Btw, on mouse device I’ve found a case when selection code doesn’t consume one of the events. Click outside of the link (or whatever region you want to override click on), and then shift-click inside the link. Suddenly click is registered on the user-provided basic text pointer input modifier.
z
I’m trying to use the desktop demo app from the androidx repo on macOS, it’s almost unusable – hangs as soon as i try interacting with it. Known issue?
o
Idk, never tried it. Cc @Igor Demin
z
looks like it might be making some sort of blocking a11y-related call on the UI thread. anyway, will just avoid it
j
@Zach Klippenstein (he/him) [MOD] When you say "the desktop demo app", which target specifically are you referring to? Also, what is your goal with launching the app? The reason I ask is that most people use one of the templates from https://github.com/JetBrains/compose-jb/tree/master/templates or one of the examples from https://github.com/JetBrains/compose-jb/tree/master/examples - curious if one of those would work better for you?
z
./gradlew :compose:desktop:desktop:desktop-samples:run1
from the androidx repo, not the github one
It looks like selection dragging is also completely broken on desktop – are yall seeing that as well?
i
The desktop target in AOSP repo is outdated, we haven't upstreamed changes for a while. The last changes are in https://github.com/JetBrains/androidx, but setup of it is different. I can check. Should we just swap modifiers and see what is happening?
z
Oof, that would have been good to know before I spent a day debugging in the old code
So you can try swapping modifiers, but I think a more robust approach is that the selection gestures should use the 
PointerInputPass.Final
pass, not 
Main
. That way any “normal” pointer events will get to work as normal. I tried this in aosp desktop and it did fix this particular issue. However, it broke double- and triple-clicking, which I did not have time yesterday to dig into. But i think that probably using a later pass to start the selection gestures is the right approach, and less brittle than relying on modifier order.
o
Frankly, I dont like the idea of desktop vs android. Android works with mouse etc.
As @Siyamed noted, mouse is not just desktop, isn’t it? Unfortunately, I’m not much into Android, what are cases when Android mouse support is essential? TV with BT mouse? Windows Subsystem for Android? Also, some desktop devices have touch support, and Compose for Desktop as far as I can see hardcodes “there is no touch, just mouse”. I understand that proper multitouch support in AWT is hard, but still.
z
I believe there’s an 
isTouchMode
flag that is currently hard-coded by platform, but @Grant Toepfer is working on fixing that
o
Gentle nudge here, was there any progress? 🙂
g
None on this specific issue. Handling touch, mouse/trackpad selection is WIP, currently fixing many, many bugs uncovered in adding tests for selection. I assigned the clicking in selection container bug to myself so I come back around to it after the above WIP. If JB hasn’t upstreamed in a while, should I be concerned about merge conflicts with my WIP? Should I be concerned that the bugs would regress when JB does push up their changes?
o
I’m not on compose team at JetBrains, so tagging @Igor Demin just in case
Thanks for an update! Waiting patiently 🙂
i
should I be concerned about merge conflicts with my WIP? Should I be concerned that the bugs would regress when JB does push up their changes?
Merge conflicts and regressions in desktop currently fixed by us. If desktop will break, we just fix it in desktopMain in our fork, or make a fix in commonMain in AOSP.
133 Views
Open in Slack
PreviousNext
Powered by