Skip to content

Commit e00c5c5

Browse files
authored
Answer a right click the way a long press is answered (#930)
A pointer asks a row for its menu with the secondary button -- a right click, or a two-finger tap -- and nothing answered it. `combinedClickable` reads any button's press as an ordinary tap and finds a long press only by timing a held finger, so a right click did what a left click does. A `View` answers it by itself -- badly: the menu came from the row under the pointer but acted on the field only the long press wrote, opening against whichever row was held last. Compose changed the symptom, not the cause. So the press is handed to the long press's lambda, watched on the initial pass and consumed so the click never lands -- module rows, apps in a scope, contributors on the home screen. Nothing changes for a finger. Fixes #644
1 parent 99c86ed commit e00c5c5

5 files changed

Lines changed: 66 additions & 9 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package org.matrix.vector.ui
2+
3+
import androidx.compose.foundation.combinedClickable
4+
import androidx.compose.ui.Modifier
5+
import androidx.compose.ui.input.pointer.PointerEventPass
6+
import androidx.compose.ui.input.pointer.isSecondaryPressed
7+
import androidx.compose.ui.input.pointer.pointerInput
8+
9+
/**
10+
* A row's whole gesture set: tap it, hold it — and, from a mouse or a touchpad, right-click it.
11+
*
12+
* A tablet with a keyboard case is a pointer device, and the way to ask a row for its menu with a
13+
* pointer is the secondary button: a right click, or a two-finger tap on the touchpad. Compose has
14+
* no notion of that gesture. `combinedClickable` reads a press from any button as the beginning of
15+
* an ordinary tap and reaches [onLongClick] only by timing a held finger, so on a touchpad every
16+
* row here answered a right click by doing whatever a left click does, and whatever the hold was
17+
* for could not be reached at all. The View-based manager never had to say any of this: a `View`
18+
* opens its registered context menu on a secondary press by itself, which is why the gesture worked
19+
* before the manager was written in Compose.
20+
*
21+
* So the secondary press is caught here and handed to [onLongClick] — the same action, because the
22+
* two gestures mean the same thing, one asked with a finger and one asked with a pointer.
23+
*/
24+
fun Modifier.contextClickable(onClick: () -> Unit, onLongClick: (() -> Unit)? = null): Modifier =
25+
combinedClickable(onClick = onClick, onLongClick = onLongClick)
26+
.then(if (onLongClick == null) Modifier else Modifier.onSecondaryPress(onLongClick))
27+
28+
/**
29+
* The secondary button's press, taken before anything underneath can read it as a tap.
30+
*
31+
* On [Initial][PointerEventPass.Initial], the pass that runs before the one `combinedClickable`
32+
* listens on — the same way [org.matrix.vector.ui.navigation.PanelBar] gets ahead of the item it
33+
* wraps — and every event of the press is consumed, which is what keeps the click from landing:
34+
* foundation asks for a press nothing else has taken, and for a release nothing else has taken.
35+
*
36+
* The action runs on the way down rather than on the release, matching the platform: a right click
37+
* on a `View` opens its context menu the moment the button goes down. It runs on the first event
38+
* that reports the button held rather than on the press event alone, because a pointer that
39+
* announces its buttons a moment after it announces the touch would otherwise be missed.
40+
*/
41+
private fun Modifier.onSecondaryPress(action: () -> Unit): Modifier =
42+
pointerInput(action) {
43+
awaitPointerEventScope {
44+
var fired = false
45+
while (true) {
46+
val event = awaitPointerEvent(PointerEventPass.Initial)
47+
if (!event.buttons.isSecondaryPressed) {
48+
fired = false
49+
continue
50+
}
51+
event.changes.forEach { it.consume() }
52+
if (!fired) {
53+
fired = true
54+
action()
55+
}
56+
}
57+
}
58+
}

manager-ui/src/main/kotlin/org/matrix/vector/ui/ModuleRow.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import androidx.compose.foundation.ExperimentalFoundationApi
44
import androidx.compose.foundation.background
55
import androidx.compose.foundation.basicMarquee
66
import androidx.compose.foundation.clickable
7-
import androidx.compose.foundation.combinedClickable
87
import androidx.compose.foundation.layout.Box
98
import androidx.compose.foundation.layout.Column
109
import androidx.compose.foundation.layout.IntrinsicSize
@@ -135,7 +134,7 @@ fun ModuleRow(
135134
Column(
136135
modifier =
137136
if (onIconClick != null)
138-
Modifier.combinedClickable(onClick = onIconClick, onLongClick = onIconLongClick)
137+
Modifier.contextClickable(onClick = onIconClick, onLongClick = onIconLongClick)
139138
else Modifier,
140139
// Against the text, not centred over the badge: the badge below is wider than the icon,
141140
// so centring left a gap between the icon and the edge the names all start from.
@@ -175,7 +174,7 @@ fun ModuleRow(
175174
Modifier.weight(1f)
176175
.then(
177176
if (onClick != null)
178-
Modifier.combinedClickable(onClick = onClick, onLongClick = onLongClick)
177+
Modifier.contextClickable(onClick = onClick, onLongClick = onLongClick)
179178
else Modifier
180179
)
181180
) {

manager/src/main/kotlin/org/matrix/vector/manager/ui/components/CommitTimeline.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import androidx.compose.foundation.background
55
import androidx.compose.foundation.border
66
import androidx.compose.foundation.ExperimentalFoundationApi
77
import androidx.compose.foundation.clickable
8-
import androidx.compose.foundation.combinedClickable
8+
import org.matrix.vector.ui.contextClickable
99
import androidx.compose.foundation.layout.Arrangement
1010
import androidx.compose.foundation.layout.Box
1111
import androidx.compose.foundation.layout.Column
@@ -212,7 +212,7 @@ fun CommitRow(
212212
// means "open this commit", and a long press on a subject line has no obvious
213213
// subject; a long press on a name plainly means *that name*.
214214
modifier =
215-
Modifier.combinedClickable(
215+
Modifier.contextClickable(
216216
onClick = { onOpenCommit(commit) },
217217
onLongClick = {
218218
haptics.performHapticFeedback(HapticFeedbackType.LongPress)

manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/home/HomeScreen.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import androidx.compose.foundation.layout.Arrangement
1010
import androidx.compose.foundation.layout.Box
1111
import androidx.compose.foundation.layout.Column
1212
import androidx.compose.foundation.ExperimentalFoundationApi
13-
import androidx.compose.foundation.combinedClickable
13+
import org.matrix.vector.ui.contextClickable
1414
import androidx.compose.foundation.layout.FlowRow
1515
import androidx.compose.foundation.layout.PaddingValues
1616
import androidx.compose.foundation.layout.size
@@ -922,7 +922,7 @@ private fun ContributorRow(
922922
Column(
923923
horizontalAlignment = Alignment.CenterHorizontally,
924924
modifier =
925-
Modifier.combinedClickable(
925+
Modifier.contextClickable(
926926
onClick = { if (hasProfile) onClick(person) },
927927
onLongClick = {
928928
haptics.performHapticFeedback(HapticFeedbackType.LongPress)

manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeScreen.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import androidx.compose.foundation.lazy.rememberLazyListState
4343
import androidx.compose.foundation.lazy.items
4444
import androidx.compose.foundation.basicMarquee
4545
import androidx.compose.foundation.border
46-
import androidx.compose.foundation.combinedClickable
46+
import org.matrix.vector.ui.contextClickable
4747
import androidx.compose.foundation.shape.CircleShape
4848
import androidx.compose.material.icons.Icons
4949
import androidx.compose.material.icons.automirrored.rounded.ArrowBack
@@ -960,7 +960,7 @@ private fun AppRow(
960960

961961
ListItem(
962962
modifier =
963-
Modifier.combinedClickable(
963+
Modifier.contextClickable(
964964
onClick = { if (enabled) onToggle(!app.isSelectedInScope) },
965965
onLongClick = {
966966
// The long press is where re-optimize lives, and re-optimize is the fix

0 commit comments

Comments
 (0)