From 070be83e8cf5cee690e7871b7fa6e25b45e52449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Tinkl?= Date: Wed, 26 Aug 2026 15:30:38 +0200 Subject: [PATCH] fix: make the context menu work on desktop+touch - get rid of the MouseArea, replace with Tap/HoverHandlers - suppress synthesized hover events on a tuch screen inside StatusMessage --- storybook/qmlTests/tests/tst_ChatTextView.qml | 58 ++++++ .../tests/tst_ChatTextViewListSelection.qml | 120 +++++++++++ .../qmlTests/tests/tst_StatusMessageHover.qml | 100 +++++++++ .../src/StatusQ/Components/ChatTextView.qml | 192 +++++++++++++++--- .../src/StatusQ/Components/StatusMessage.qml | 23 ++- ui/i18n/qml_uk.ts | 56 ++--- 6 files changed, 479 insertions(+), 70 deletions(-) create mode 100644 storybook/qmlTests/tests/tst_ChatTextViewListSelection.qml create mode 100644 storybook/qmlTests/tests/tst_StatusMessageHover.qml diff --git a/storybook/qmlTests/tests/tst_ChatTextView.qml b/storybook/qmlTests/tests/tst_ChatTextView.qml index 85786bc631..ae993ee4d7 100644 --- a/storybook/qmlTests/tests/tst_ChatTextView.qml +++ b/storybook/qmlTests/tests/tst_ChatTextView.qml @@ -165,6 +165,24 @@ Item { compare(control.selectedText, "") } + // Selection is mouse-only: a touch drag over a selectable view must not select anything + // (touch is handled one level above — tap activates links, long press opens the menu). + function test_touchDragDoesNotSelect() { + control.selectable = true + control.blocks = [ + { type: "text", html: "first line" }, + { type: "text", html: "second line" } + ] + tryVerify(() => control.implicitHeight > 0) + + const touch = touchEvent(control) + touch.press(0, control, 0, 3).commit() + touch.move(0, control, control.width - 4, control.implicitHeight - 4).commit() + touch.release(0, control, control.width - 4, control.implicitHeight - 4).commit() + + compare(control.selectedText, "", "touch must not select text") + } + // With `selectable`, dragging from the first block into the second selects across // both — the combined selectedText contains text from each. function test_crossBlockSelection() { @@ -512,6 +530,46 @@ Item { compare(mentionSpy.signalArguments[0][0], "0xabc") } + // Touch input doesn't select; links are activated via the public activateLinkAt(), + // which the message view calls from its touch TapHandler. + function test_activateLinkAt_link() { + control.selectable = true + control.blocks = [{ type: "text", html: 'https://status.im' }] + tryVerify(() => control.implicitHeight > 0) + + linkSpy.clear() + control.activateLinkAt(Qt.point(10, 5)) + compare(linkSpy.count, 1) + compare(linkSpy.signalArguments[0][0], "https://status.im") + } + + function test_activateLinkAt_mention() { + control.selectable = true + control.blocks = [{ type: "text", html: '@alice' }] + tryVerify(() => control.implicitHeight > 0) + + mentionSpy.clear() + control.activateLinkAt(Qt.point(5, 5)) + compare(mentionSpy.count, 1) + compare(mentionSpy.signalArguments[0][0], "0xabc") + } + + // A real touch tap (not the direct activateLinkAt() API call above) must also activate + // a link. The read-only TextEdit backing each block grabs touch points exclusively by + // default, so this guards against that grab silently swallowing the tap. + function test_realTouchTapActivatesLink() { + control.selectable = true + control.blocks = [{ type: "text", html: 'https://status.im' }] + tryVerify(() => control.implicitHeight > 0) + + linkSpy.clear() + const touch = touchEvent(control) + touch.press(0, control, 10, 5).commit() + touch.release(0, control, 10, 5).commit() + compare(linkSpy.count, 1) + compare(linkSpy.signalArguments[0][0], "https://status.im") + } + // A drag (selection) must NOT be treated as a link click. function test_dragDoesNotClickLink() { control.selectable = true diff --git a/storybook/qmlTests/tests/tst_ChatTextViewListSelection.qml b/storybook/qmlTests/tests/tst_ChatTextViewListSelection.qml new file mode 100644 index 0000000000..687fae4c5e --- /dev/null +++ b/storybook/qmlTests/tests/tst_ChatTextViewListSelection.qml @@ -0,0 +1,120 @@ +import QtQuick +import QtTest + +import StatusQ.Core +import StatusQ.Components + +// A mouse text-selection drag inside a scrollable list (the chat's `StatusListView`) must +// extend the selection without the list scrolling underneath: a `Flickable` starts dragging +// at its own drag threshold and jumps the content before a pointer handler in the delegate +// can take the grab, which used to make selecting multiple lines nearly impossible. +Item { + id: root + width: 600 + height: 500 + + Component { + id: componentUnderTest + + StatusListView { + id: listView + + width: 400 + height: 300 + + // Mirrors the chat message list. + verticalLayoutDirection: ListView.BottomToTop + + property var views: ({}) + + model: 20 + + delegate: Item { + width: listView.width + implicitHeight: textView.implicitHeight + 10 + + ChatTextView { + id: textView + + width: parent.width + selectable: true + font.pixelSize: 15 + blocks: [ + { type: "text", html: "Msg " + index + " line one here" }, + { type: "text", html: "Msg " + index + " line two here" }, + { type: "text", html: "Msg " + index + " line three here" } + ] + + Component.onCompleted: listView.views[index] = textView + } + } + } + } + + TestCase { + name: "ChatTextViewListSelection" + when: windowShown + + // Drives a drag as a sequence of moves, synchronising on rendering instead of + // sleeping for a fixed amount of time. + function dragBy(listView, fromX, fromY, toX, toY, step) { + mousePress(listView, fromX, fromY) + const dx = (toX - fromX) / step + const dy = (toY - fromY) / step + for (let i = 1; i <= step; ++i) { + mouseMove(listView, fromX + dx * i, fromY + dy * i) + waitForRendering(listView) + } + } + + function selectedTexts(listView) { + return Object.keys(listView.views) + .map(key => listView.views[key] ? listView.views[key].selectedText : "") + .filter(text => !!text) + } + + function test_selectionDragDoesNotScrollTheList() { + const listView = createTemporaryObject(componentUnderTest, root) + verify(listView) + tryVerify(() => !!listView.views[0] && listView.views[0].implicitHeight > 0) + // Let the list settle at its initial position before sampling it. + tryVerify(() => !listView.moving) + waitForRendering(listView) + + const contentYBefore = listView.contentY + + dragBy(listView, 5, 5, 65, 60, 11) + + compare(listView.contentY, contentYBefore, + "the list must not scroll while a selection drag is in progress") + + const selected = selectedTexts(listView) + compare(selected.length, 1, "exactly one message should hold the selection") + verify(selected[0].length > 0, "the drag should have selected text") + + mouseRelease(listView, 65, 60) + } + + // Suspending the list while selecting must not leave it permanently unscrollable. + function test_listStaysScrollableAfterSelectionDrag() { + const listView = createTemporaryObject(componentUnderTest, root) + verify(listView) + tryVerify(() => !!listView.views[0] && listView.views[0].implicitHeight > 0) + tryVerify(() => !listView.moving) + waitForRendering(listView) + + dragBy(listView, 5, 5, 65, 60, 11) + mouseRelease(listView, 65, 60) + + tryVerify(() => listView.interactive, + 5000, "the list must be interactive again after the drag") + + const contentYBefore = listView.contentY + dragBy(listView, listView.width - 10, 20, listView.width - 10, 120, 10) + mouseRelease(listView, listView.width - 10, 120) + + tryVerify(() => listView.contentY !== contentYBefore, + 5000, "the list should scroll again") + } + } +} diff --git a/storybook/qmlTests/tests/tst_StatusMessageHover.qml b/storybook/qmlTests/tests/tst_StatusMessageHover.qml new file mode 100644 index 0000000000..5f7fa3349d --- /dev/null +++ b/storybook/qmlTests/tests/tst_StatusMessageHover.qml @@ -0,0 +1,100 @@ +import QtQuick +import QtTest + +import StatusQ.Components + +// Hover reporting must be exclusive to real pointing devices. A touch tap makes Qt synthesize +// a mouse move ("core pointer"), which used to set `hovered` and pop up the hover-driven +// quick-actions context menu on top of the long-press one. +Item { + id: root + width: 600 + height: 400 + + Component { + id: componentUnderTest + + StatusMessage { + width: 500 + messageId: "m1" + messageDetails: StatusMessageDetails { + messageText: "hello" + unparsedText: "hello" + } + } + } + + SignalSpy { + id: hoverSpy + signalName: "hoverChanged" + } + + TestCase { + id: testCase + name: "StatusMessageHover" + when: windowShown + + // Touch taps are expected NOT to produce a hover. Waiting for a fixed amount of time + // would only make the test slow and flaky, so instead a subsequent real mouse hover is + // used as a synchronisation point: once it has been reported, any (synthesized) hover + // from the preceding touch tap would certainly have been delivered as well. + function syncOnMouseHover(control) { + mouseMove(control, 50, 10) + tryVerify(() => control.hovered, 5000, "the mouse hover was never delivered") + } + + function createControl() { + const control = createTemporaryObject(componentUnderTest, root) + verify(control) + hoverSpy.target = control + hoverSpy.clear() + return control + } + + // A mouse move over the message reports hovered=true. + function test_mouseHoverEmitsHoverChanged() { + const control = createControl() + + mouseMove(control, 50, 10) + tryVerify(() => hoverSpy.count > 0, 2000, "mouse hover must emit hoverChanged") + compare(hoverSpy.signalArguments[hoverSpy.count - 1][0], "m1") + compare(hoverSpy.signalArguments[hoverSpy.count - 1][1], true) + + // Move away again: hover ends. + mouseMove(control, 50, root.height - 1) + tryCompare(control, "effectiveHovered", false) + } + + // A touch tap must not report hover at all (the synthesized mouse move is suppressed). + // Note the synthesized hover latches on and does not clear by itself, so this checks that + // hover never pulses true at any point, not merely that it settled back to false. + function test_touchTapDoesNotEmitHoverChanged() { + const control = createControl() + + const touch = touchEvent(control) + touch.press(0, control, 50, 10).commit() + touch.release(0, control, 50, 10).commit() + syncOnMouseHover(control) + + compare(hoverSpy.count, 0, "touch tap must not emit hoverChanged") + compare(control.effectiveHovered, false) + } + + // The suppression must not be sticky: a real mouse hover after a touch tap still works. + function test_mouseHoverAfterTouchTapStillWorks() { + const control = createControl() + + const touch = touchEvent(control) + touch.press(0, control, 50, 10).commit() + touch.release(0, control, 50, 10).commit() + + // Leave the message, then come back with the mouse. (That the tap itself reports no + // hover is covered by `test_touchTapDoesNotEmitHoverChanged`.) + mouseMove(control, 50, root.height - 1) + tryVerify(() => !control.hovered, 5000, "the mouse never left the message") + mouseMove(control, 50, 10) + tryVerify(() => hoverSpy.count > 0, 2000, "mouse hover after a touch tap must work") + compare(hoverSpy.signalArguments[hoverSpy.count - 1][1], true) + } + } +} diff --git a/ui/StatusQ/src/StatusQ/Components/ChatTextView.qml b/ui/StatusQ/src/StatusQ/Components/ChatTextView.qml index 2aa90b716d..8d06af6a64 100644 --- a/ui/StatusQ/src/StatusQ/Components/ChatTextView.qml +++ b/ui/StatusQ/src/StatusQ/Components/ChatTextView.qml @@ -73,6 +73,15 @@ Control { ClipboardUtils.setText(root.selectedText) } + // Activates the link (mention or url) at `pos`, given in this item's coordinates; a no-op + // when there is no link there. Text selection is a mouse-only interaction (see the overlay + // below), so touch input is routed here from one level above. + function activateLinkAt(pos) { + const p = root.contentItem.mapFromItem(root, pos.x, pos.y) + d.collectEditors() + d.activateLinkAt(p.x, p.y) + } + onSelectableChanged: d.clearSelection() onBlocksChanged: d.clearSelection() onEditedChanged: d.clearSelection() @@ -152,6 +161,23 @@ Control { text: effectiveStyle + d.wrapContent(content) onHoveredLinkChanged: d.hoveredLink = hoveredLink + + // A plain TextEdit grabs touch points exclusively (even with selectByMouse: + // false), so a TapHandler placed anywhere above it in the hierarchy (e.g. in + // StatusTextMessage, one level up) never sees touch taps. Handling the touch tap + // here, as a direct child of the TextEdit, is the only place the grab can be + // taken over from. + TapHandler { + acceptedDevices: PointerDevice.TouchScreen + gesturePolicy: TapHandler.ReleaseWithinBounds + grabPermissions: PointerHandler.CanTakeOverFromItems + | PointerHandler.CanTakeOverFromHandlersOfDifferentType + onSingleTapped: eventPoint => { + const link = textEdit.linkAt(eventPoint.position.x, eventPoint.position.y) + if (link) + d.activateLink(link) + } + } } } @@ -370,19 +396,31 @@ Control { root.mentionClicked(href) } - // Selectable mode: the overlay swallows clicks, so resolve the link ourselves by finding - // the editor under (x, y) (in contentItem coords) and asking it for the link there. - function activateLinkAt(x, y) { + // Selectable mode: the overlay swallows hover, so resolve the link under (x, y) (in + // contentItem coords) ourselves and publish it as the hovered link. + function updateHoveredLinkAt(x, y) { + if (editors.length === 0) + collectEditors() + hoveredLink = linkAt(x, y) + } + + // Returns the link href at (x, y) (in contentItem coords), or "" when there is none. + function linkAt(x, y) { for (let i = 0; i < editors.length; ++i) { const editor = editors[i] const point = editor.mapFromItem(root.contentItem, x, y) - if (editor.contains(point)) { - const link = editor.linkAt(point.x, point.y) - if (link) - activateLink(link) - return - } + if (editor.contains(point)) + return editor.linkAt(point.x, point.y) || "" } + return "" + } + + // Selectable mode: the overlay swallows clicks, so resolve the link ourselves by finding + // the editor under (x, y) (in contentItem coords) and asking it for the link there. + function activateLinkAt(x, y) { + const link = linkAt(x, y) + if (link) + activateLink(link) } function applySelection(a, b) { @@ -538,16 +576,25 @@ Control { // Cross-block selection: one overlay (over the whole content) drives the per-editor // selection from a single press+drag. It is a child of the Control (not the Column), so // it can fill the content area. - MouseArea { + // + // Selection is a mouse-only interaction: the handlers below accept pointing devices only, + // so touch events pass through untouched and are handled one level above (tap to activate + // a link, long press to open the message context menu). + Item { + id: selectionOverlay + anchors.fill: root.contentItem z: 100 enabled: root.selectable visible: root.selectable - cursorShape: !!root.hoveredLink ? Qt.PointingHandCursor : Qt.IBeamCursor - preventStealing: true + + readonly property int pointingDevices: PointerDevice.Mouse | PointerDevice.TouchPad + | PointerDevice.Stylus property real pressX: 0 property real pressY: 0 + property real lastX: 0 + property real lastY: 0 property bool moved: false // Multi-click tracking. Clicks in place cycle: 1 click, 2 word, 3 line, 4 deselect, @@ -556,18 +603,19 @@ Control { property real lastPressTime: 0 readonly property int clickSlop: 4 - onPressed: (mouse) => { + function handlePress(x, y) { root.forceActiveFocus() // Grab focus (deselects other views) const now = Date.now() - const near = Math.abs(mouse.x - pressX) <= clickSlop - && Math.abs(mouse.y - pressY) <= clickSlop + const near = Math.abs(x - pressX) <= clickSlop && Math.abs(y - pressY) <= clickSlop clickCount = (now - lastPressTime <= Qt.styleHints.mouseDoubleClickInterval && near) ? clickCount + 1 : 1 lastPressTime = now - pressX = mouse.x - pressY = mouse.y + pressX = x + pressY = y + lastX = x + lastY = y moved = false d.collectEditors() @@ -575,30 +623,120 @@ Control { const mode = (clickCount - 1) % 4 if (mode === 1) { d.anchor = null // discrete word selection, no drag-extend - d.selectWordAt(mouse.x, mouse.y) + d.selectWordAt(x, y) moved = true // suppress link activation on release } else if (mode === 2) { d.anchor = null - d.selectLineAt(mouse.x, mouse.y) + d.selectLineAt(x, y) moved = true } else if (mode === 3) { d.clearSelection() moved = true } else { - d.anchor = d.hitTest(mouse.x, mouse.y) + d.anchor = d.hitTest(x, y) d.applySelection(d.anchor, d.anchor) } } - onPositionChanged: (mouse) => { - if (Math.abs(mouse.x - pressX) > 3 || Math.abs(mouse.y - pressY) > 3) + + function handleMove(x, y) { + lastX = x + lastY = y + if (Math.abs(x - pressX) > 3 || Math.abs(y - pressY) > 3) moved = true if (d.anchor) - d.applySelection(d.anchor, d.hitTest(mouse.x, mouse.y)) + d.applySelection(d.anchor, d.hitTest(x, y)) } - // A click (no drag) on a link activates it; a drag selects text instead. - onReleased: (mouse) => { - if (!moved) - d.activateLinkAt(mouse.x, mouse.y) + + // The overlay covers the editors, so they never see hover events in selectable mode; + // resolve the link under the pointer here instead (drives the link hover highlight + // and the cursor shape). + HoverHandler { + acceptedDevices: selectionOverlay.pointingDevices + cursorShape: !!root.hoveredLink ? Qt.PointingHandCursor : Qt.IBeamCursor + + onPointChanged: if (hovered) d.updateHoveredLinkAt(point.position.x, point.position.y) + onHoveredChanged: if (!hovered) d.hoveredLink = "" + } + + TapHandler { + acceptedButtons: Qt.LeftButton + acceptedDevices: selectionOverlay.pointingDevices + // Keep the exclusive grab for the whole press, so a drag keeps extending the + // selection even when it leaves the message bounds (mimics `preventStealing`). + gesturePolicy: TapHandler.ReleaseWithinBounds + grabPermissions: PointerHandler.CanTakeOverFromItems + | PointerHandler.CanTakeOverFromHandlersOfDifferentType + | PointerHandler.ApprovesTakeOverByHandlersOfSameType + + onPressedChanged: { + if (pressed) { + selectionOverlay.suspendFlicking() + selectionOverlay.handlePress(point.position.x, point.position.y) + } else { + selectionOverlay.resumeFlicking() + } + } + onPointChanged: if (pressed) + selectionOverlay.handleMove(point.position.x, point.position.y) + // A click (no drag) on a link activates it; a drag selects text instead. + onTapped: if (!selectionOverlay.moved) + d.activateLinkAt(selectionOverlay.lastX, selectionOverlay.lastY) + } + + // Ancestor `Flickable` (a `StatusListView` in the chat) whose interactivity is + // suspended while a selection drag is in progress. Pointer handler grab permissions + // alone aren't enough: a `Flickable` starts dragging at its own drag threshold and + // applies a one-shot content jump before any handler here can take the grab, which + // makes precise text selection (especially across lines) impossible. + property Flickable ancestorFlickable: null + property bool flickableWasInteractive: false + + function findAncestorFlickable() { + let item = root.parent + while (item) { + if (item instanceof Flickable) + return item + item = item.parent + } + return null + } + + function suspendFlicking() { + ancestorFlickable = findAncestorFlickable() + if (!ancestorFlickable) + return + flickableWasInteractive = ancestorFlickable.interactive + ancestorFlickable.interactive = false + } + + function resumeFlicking() { + if (!ancestorFlickable) + return + ancestorFlickable.interactive = flickableWasInteractive + ancestorFlickable = null + } + + // A `TapHandler` alone (even with an exclusive grabbing `gesturePolicy`) is treated as + // a passive grab by ancestor `Flickable`s once the pointer moves past their drag + // threshold, so a `StatusListView`/`ListView` ancestor steals the gesture and scrolls + // instead of extending the text selection. Only a `DragHandler` takes the kind of + // exclusive (active) grab a `Flickable` respects, so one is added here to hold the + // grab for the drag part of the gesture; it moves nothing (`target: null`). + // + // Because it takes the grab away from the `TapHandler`, the `TapHandler` stops getting + // point updates once dragging starts, so the selection has to be extended from here + // for the rest of the gesture (the `TapHandler` still handles the press and the + // no-drag tap that activates links). + DragHandler { + target: null + acceptedButtons: Qt.LeftButton + acceptedDevices: selectionOverlay.pointingDevices + grabPermissions: PointerHandler.CanTakeOverFromItems + | PointerHandler.CanTakeOverFromHandlersOfDifferentType + + onCentroidChanged: if (active) + selectionOverlay.handleMove(centroid.position.x, + centroid.position.y) } } diff --git a/ui/StatusQ/src/StatusQ/Components/StatusMessage.qml b/ui/StatusQ/src/StatusQ/Components/StatusMessage.qml index 84e4bdc092..741bf8273c 100644 --- a/ui/StatusQ/src/StatusQ/Components/StatusMessage.qml +++ b/ui/StatusQ/src/StatusQ/Components/StatusMessage.qml @@ -123,8 +123,14 @@ Control { }) } + HoverHandler { + acceptedDevices: PointerDevice.TouchScreen + onHoveredChanged: if (hovered) d.touchHoverSuppressed = true + } + hoverEnabled: (!root.isActiveMessage && !root.disableHover) - onHoveredChanged: root.hoverChanged(root.messageId, root.hovered) + onHoveredChanged: if (!hovered) d.touchHoverSuppressed = false + readonly property alias effectiveHovered: d.effectiveHovered background: Rectangle { color: { @@ -401,6 +407,21 @@ Control { QtObject { id: d property string selectedText + + // Hover must be reported for real pointing devices only. A touch tap makes Qt synthesize a + // mouse move (a "core pointer" event, indistinguishable from a real mouse by + // `acceptedDevices` on a HoverHandler), which would otherwise set `hovered` and trigger + // hover-driven UI such as the quick-actions context menu. On touch that menu is the + // long-press gesture instead, so a touch-originated hover is latched as suppressed until + // the hover genuinely ends. A latch rather than a timer is required because the synthesized + // hover stays active for as long as the pointer "rests" on the message after the tap. + // + // A HoverHandler (passive, never takes a grab) is used deliberately here: the read-only + // TextEdits rendering the message text take the exclusive grab on any touch point, which + // would keep a PointHandler at this level from ever becoming active. + property bool touchHoverSuppressed: false + readonly property bool effectiveHovered: root.hovered && !d.touchHoverSuppressed + onEffectiveHoveredChanged: root.hoverChanged(root.messageId, d.effectiveHovered) } component StatusTextMessageCommon: StatusTextMessage { diff --git a/ui/i18n/qml_uk.ts b/ui/i18n/qml_uk.ts index 924ba42569..a639c29895 100644 --- a/ui/i18n/qml_uk.ts +++ b/ui/i18n/qml_uk.ts @@ -11425,25 +11425,21 @@ to load MessageContextMenuView - - Reply to - Відповісти - - - Edit message - Редагувати - - - Copy - Копіювати - Copy message Копіювати - Copy Message Id - Копіювати ID + Reply + Відповісти + + + Edit + + + + Copy selected + Copy link to message @@ -11458,12 +11454,12 @@ to load Закріпити - Mark as unread - Непрочитане + Delete + Видалити - Delete message - Видалити + Mark as unread + Непрочитане @@ -11531,30 +11527,6 @@ to load GIF GIF - - Reply - Відповісти - - - Edit - Редагувати - - - Unpin - Відкріпити - - - Pin - Закріпити - - - Mark as unread - Непрочитане - - - Delete - Видалити - Unknown message. Trying to recover it Невідоме повідомлення. Спроба відновлення