Skip to content

Commit

Permalink
Fix missing clicks inside SelectionContainer (#1106)
Browse files Browse the repository at this point in the history
## Proposed Changes

- Do not recreate modifier nodes on focus change - Buttons inside
`DisableSelection` now clickable. Before it receives only second click
- Do not consume UP events if there were no drag before - Buttons inside
`SelectionContainer` now clickable, but text inside them is still
selectable
- `SelectionManager.hasFocus` now `true` if any of it's children is
focusable
- `onClearSelectionRequested` now triggers on consumed click on
focusable, non-selectable child

## Testing

Test: `Selection` page in mpp

## Issues Fixed

Fixes JetBrains/compose-multiplatform#1450
  • Loading branch information
MatkovIvan committed Feb 15, 2024
1 parent adabe82 commit 6b4ea61
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ internal fun SelectionContainer(
DisposableEffect(manager) {
onDispose {
manager.onRelease()
manager.hasFocus = false
manager.focusState = null
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,15 @@ private suspend fun AwaitPointerEventScope.mouseSelection(

val started = observer.onStart(downChange.position, selectionAdjustment)
if (started) {
var dragConsumed = selectionAdjustment != SelectionAdjustment.None
val shouldConsumeUp = drag(downChange.id) {
if (observer.onDrag(it.position, selectionAdjustment)) {
it.consume()
dragConsumed = true
}
}

if (shouldConsumeUp) {
if (shouldConsumeUp && dragConsumed) {
currentEvent.changes.fastForEach {
if (it.changedToUp()) it.consume()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package androidx.compose.foundation.text.selection

import androidx.annotation.VisibleForTesting
import androidx.compose.foundation.focusable
import androidx.compose.foundation.gestures.awaitEachGesture
import androidx.compose.foundation.gestures.waitForUpOrCancellation
import androidx.compose.foundation.text.Handle
import androidx.compose.foundation.text.TextDragObserver
import androidx.compose.foundation.text.selection.Selection.AnchorInfo
Expand All @@ -31,6 +29,7 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.FocusState
import androidx.compose.ui.focus.focusRequester
import androidx.compose.ui.focus.onFocusChanged
import androidx.compose.ui.geometry.Offset
Expand All @@ -41,7 +40,12 @@ import androidx.compose.ui.hapticfeedback.HapticFeedback
import androidx.compose.ui.hapticfeedback.HapticFeedbackType
import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.input.key.onKeyEvent
import androidx.compose.ui.input.pointer.PointerInputScope
import androidx.compose.ui.input.pointer.AwaitPointerEventScope
import androidx.compose.ui.input.pointer.PointerEvent
import androidx.compose.ui.input.pointer.PointerEventPass
import androidx.compose.ui.input.pointer.PointerInputChange
import androidx.compose.ui.input.pointer.changedToUp
import androidx.compose.ui.input.pointer.changedToUpIgnoreConsumed
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.layout.LayoutCoordinates
import androidx.compose.ui.layout.boundsInWindow
Expand All @@ -54,6 +58,7 @@ import androidx.compose.ui.platform.TextToolbarStatus
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.buildAnnotatedString
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.util.fastAll
import androidx.compose.ui.util.fastAny
import androidx.compose.ui.util.fastFold
import androidx.compose.ui.util.fastForEach
Expand Down Expand Up @@ -120,10 +125,25 @@ internal class SelectionManager(private val selectionRegistrar: SelectionRegistr
*/
var focusRequester: FocusRequester = FocusRequester()

/**
* Current focus state.
*/
var focusState: FocusState? by mutableStateOf(null)

/**
* Return true if the corresponding SelectionContainer has a child that is focused.
*/
val hasFocus get() = focusState?.hasFocus ?: false

/**
* Return true if the corresponding SelectionContainer is focused.
*/
var hasFocus: Boolean by mutableStateOf(false)
private val isContainerFocused get() = focusState?.isFocused ?: false

/**
* Return true if dragging gesture is currently in process.
*/
private val isDraggingInProgress get() = draggingHandle != null

/**
* Modifier for selection container.
Expand All @@ -134,10 +154,10 @@ internal class SelectionManager(private val selectionRegistrar: SelectionRegistr
.onGloballyPositioned { containerLayoutCoordinates = it }
.focusRequester(focusRequester)
.onFocusChanged { focusState ->
if (!focusState.isFocused && hasFocus) {
if (!focusState.hasFocus && hasFocus) {
onRelease()
}
hasFocus = focusState.isFocused
this.focusState = focusState
}
.focusable()
.updateSelectionTouchMode { isInTouchMode = it }
Expand Down Expand Up @@ -236,7 +256,7 @@ internal class SelectionManager(private val selectionRegistrar: SelectionRegistr
private set

private val shouldShowMagnifier
get() = draggingHandle != null && isInTouchMode && !isTriviallyCollapsedSelection()
get() = isDraggingInProgress && isInTouchMode && !isTriviallyCollapsedSelection()

@VisibleForTesting
internal var previousSelectionLayout: SelectionLayout? = null
Expand Down Expand Up @@ -664,20 +684,27 @@ internal class SelectionManager(private val selectionRegistrar: SelectionRegistr
override fun onCancel() = done()
}

/**
* Detect tap without consuming the up event.
*/
private suspend fun PointerInputScope.detectNonConsumingTap(onTap: (Offset) -> Unit) {
awaitEachGesture {
waitForUpOrCancellation()?.let {
onTap(it.position)
private fun Modifier.onClearSelectionRequested(block: () -> Unit): Modifier =
pointerInput(Unit) {
awaitPointerEventScope {
while (true) {
awaitPointerEventWhereAllChanges(PointerEventPass.Initial) {
it.changedToUpIgnoreConsumed()
}?.let {
if (!isContainerFocused && !isDraggingInProgress) {
block()
}
}
awaitPointerEventWhereAllChanges(PointerEventPass.Main) {
it.changedToUp()
}?.let {
if (isContainerFocused) {
block()
}
}
}
}
}
}

private fun Modifier.onClearSelectionRequested(block: () -> Unit): Modifier {
return if (hasFocus) pointerInput(Unit) { detectNonConsumingTap { block() } } else this
}

private fun convertToContainerCoordinates(
layoutCoordinates: LayoutCoordinates,
Expand Down Expand Up @@ -992,3 +1019,12 @@ internal fun LayoutCoordinates.visibleBounds(): Rect {

internal fun Rect.containsInclusive(offset: Offset): Boolean =
offset.x in left..right && offset.y in top..bottom

/**
* Suspend until a [PointerEvent] is reported to the specified input [pass].
* Returns it if all changes match the given [predicate].
*/
private suspend fun AwaitPointerEventScope.awaitPointerEventWhereAllChanges(
pass: PointerEventPass = PointerEventPass.Main,
predicate: (PointerInputChange) -> Boolean,
) = awaitPointerEvent(pass).takeIf { it.changes.fastAll(predicate) }
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,50 @@
package androidx.compose.mpp.demo.components

import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.text.selection.DisableSelection
import androidx.compose.foundation.text.selection.SelectionContainer
import androidx.compose.material3.Button
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp

@Composable
fun SelectionExample() {
SelectionContainer(
Modifier.padding(24.dp).fillMaxWidth()
) {
Column {
Text(
"I'm a selection container. Double tap on word to select a word." +
" Triple tap on content to select whole paragraph.\nAnother paragraph for testing.\n" +
"And another one."
)
Text("I'm another Text() block. Let's try to select me!")
Text("I'm yet another Text() with multiparagraph structure block.\nLet's try to select me!")
var count by remember { mutableStateOf(0) }
Column {
Button(onClick = { count++ }) {
Text("Outside Count: $count")
}
SelectionContainer(
Modifier.padding(24.dp).fillMaxWidth()
) {
Column {
Text(
"I'm a selection container. Double tap on word to select a word." +
" Triple tap on content to select whole paragraph.\nAnother paragraph for testing.\n" +
"And another one."
)
Row {
DisableSelection {
Button(onClick = { count++ }) {
Text("DisableSelection Count: $count")
}
}
Button(onClick = { count++ }) {
Text("SelectionContainer Count: $count")
}
}
Text("I'm another Text() block. Let's try to select me!")
Text("I'm yet another Text() with multiparagraph structure block.\nLet's try to select me!")
}
}
}
}

0 comments on commit 6b4ea61

Please sign in to comment.