Long-press popups: one owner, always on screen
- A second long-press finishes the first (typing its choice) before opening its own popup, instead of sharing one - Popups move down over their key when the room above is too small - No popup over an unknown key: an empty rect made every cell zero wide - A repeated touch id cancels the stale pointer instead of leaking it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -50,6 +50,7 @@ Window {
|
||||
visible: !root.settingsOpen
|
||||
model: root.layoutModel
|
||||
controller: root.controller
|
||||
popupHeadroom: root.popupMargin + anchors.margins
|
||||
}
|
||||
|
||||
SettingsPanel {
|
||||
|
||||
@@ -2,14 +2,16 @@
|
||||
|
||||
#include <QtMath>
|
||||
|
||||
void AlternatesPicker::open(const QRectF &keyRect, const QStringList &choices, qreal boundsWidth, qreal gap)
|
||||
void AlternatesPicker::open(const QRectF &keyRect, const QStringList &choices, qreal boundsWidth, qreal minTop,
|
||||
qreal gap)
|
||||
{
|
||||
if (choices.isEmpty())
|
||||
return;
|
||||
if (choices.isEmpty() || keyRect.isEmpty())
|
||||
return; // an empty rect would make every cell zero wide
|
||||
|
||||
const qreal width = keyRect.width() * choices.size();
|
||||
const qreal left = qBound(0.0, keyRect.left(), qMax(0.0, boundsWidth - width));
|
||||
m_rect = QRectF(left, keyRect.top() - keyRect.height() - gap, width, keyRect.height());
|
||||
const qreal top = qMax(minTop, keyRect.top() - keyRect.height() - gap);
|
||||
m_rect = QRectF(left, top, width, keyRect.height());
|
||||
m_choices = choices;
|
||||
m_selected = -1;
|
||||
selectAt(keyRect.center().x()); // emits changed()
|
||||
|
||||
@@ -28,8 +28,9 @@ public:
|
||||
|
||||
// Places one key-sized cell per choice just above the key, starting over it
|
||||
// and extending right (shifted left near the edge to stay within
|
||||
// [0, boundsWidth]). The cell above the finger starts selected.
|
||||
void open(const QRectF &keyRect, const QStringList &choices, qreal boundsWidth, qreal gap);
|
||||
// [0, boundsWidth]; lowered, over the key, to stay below minTop). The cell
|
||||
// above the finger starts selected.
|
||||
void open(const QRectF &keyRect, const QStringList &choices, qreal boundsWidth, qreal minTop, qreal gap);
|
||||
void selectAt(qreal x);
|
||||
void close();
|
||||
|
||||
|
||||
@@ -97,8 +97,15 @@ void KeyboardSurface::geometryChange(const QRectF &newGeometry, const QRectF &ol
|
||||
|
||||
void KeyboardSurface::pointerDown(int id, QPointF pos, qint64 timeMs)
|
||||
{
|
||||
m_lastEventMs = timeMs;
|
||||
if (!m_model || !m_controller)
|
||||
return;
|
||||
// A press for an id still down means its release was lost.
|
||||
if (const auto stale = m_pointers.constFind(id); stale != m_pointers.cend()) {
|
||||
const Pointer pointer = *stale;
|
||||
m_pointers.erase(stale);
|
||||
cancel(pointer);
|
||||
}
|
||||
// Rollover would let go of keys still held, which hold-keys mode must not.
|
||||
if (!m_controller->holdsKeys())
|
||||
commitPendingTaps(timeMs);
|
||||
@@ -134,7 +141,8 @@ void KeyboardSurface::pointerMove(int id, QPointF pos)
|
||||
m_model->setFlickProgress(pointer.index, pointer.tracker.flickProgress());
|
||||
break;
|
||||
case Gesture::LongPress:
|
||||
m_alternates.selectAt(pos.x());
|
||||
if (ownsAlternates(pointer))
|
||||
m_alternates.selectAt(pos.x());
|
||||
break;
|
||||
case Gesture::Trackpad:
|
||||
m_controller->moveCursor(pointer.tracker.takeTrackpadSteps());
|
||||
@@ -149,6 +157,7 @@ void KeyboardSurface::pointerMove(int id, QPointF pos)
|
||||
|
||||
void KeyboardSurface::pointerUp(int id, qint64 timeMs)
|
||||
{
|
||||
m_lastEventMs = timeMs;
|
||||
const auto it = m_pointers.constFind(id);
|
||||
if (it == m_pointers.cend())
|
||||
return;
|
||||
@@ -166,7 +175,7 @@ void KeyboardSurface::longPressElapsed(int id, quint64 serial)
|
||||
if (it->tracker.gesture() == Gesture::Trackpad)
|
||||
setTrackpadActive(true);
|
||||
else if (it->tracker.gesture() == Gesture::LongPress)
|
||||
openAlternates(*it);
|
||||
openAlternates(Pointer(*it), m_lastEventMs); // a copy: opening may erase from m_pointers
|
||||
}
|
||||
|
||||
// --- Gesture outcomes
|
||||
@@ -184,10 +193,14 @@ void KeyboardSurface::finish(const Pointer &pointer, qint64 timeMs)
|
||||
case Gesture::Flick:
|
||||
m_controller->keyUp(key, tracker.flickCommitted() ? KeyOutput::Flick : KeyOutput::Primary, {}, timeMs);
|
||||
break;
|
||||
case Gesture::LongPress:
|
||||
m_controller->keyUp(key, KeyOutput::Alternate, m_alternates.selectedText(), timeMs);
|
||||
m_alternates.close();
|
||||
case Gesture::LongPress: {
|
||||
// Without the popup (it never opened), the key types its primary output.
|
||||
const QString alternate = ownsAlternates(pointer) ? m_alternates.selectedText() : QString();
|
||||
if (ownsAlternates(pointer))
|
||||
closeAlternates();
|
||||
m_controller->keyUp(key, KeyOutput::Alternate, alternate, timeMs);
|
||||
break;
|
||||
}
|
||||
case Gesture::Trackpad:
|
||||
m_controller->keyCancelled(key);
|
||||
setTrackpadActive(false);
|
||||
@@ -201,6 +214,18 @@ void KeyboardSurface::finish(const Pointer &pointer, qint64 timeMs)
|
||||
}
|
||||
}
|
||||
|
||||
void KeyboardSurface::cancel(const Pointer &pointer)
|
||||
{
|
||||
if (m_model)
|
||||
release(pointer);
|
||||
if (ownsAlternates(pointer))
|
||||
closeAlternates();
|
||||
if (pointer.tracker.gesture() == Gesture::Trackpad)
|
||||
setTrackpadActive(false);
|
||||
if (m_controller)
|
||||
m_controller->keyCancelled(pointer.tracker.key());
|
||||
}
|
||||
|
||||
// Clears the visual state a pointer left on the model.
|
||||
void KeyboardSurface::release(const Pointer &pointer)
|
||||
{
|
||||
@@ -232,14 +257,38 @@ void KeyboardSurface::updateSlideHover(Pointer &pointer)
|
||||
pointer.hoverIndex = hover;
|
||||
}
|
||||
|
||||
void KeyboardSurface::openAlternates(const Pointer &pointer)
|
||||
// One popup: a second long-press first finishes the one showing it, as a new
|
||||
// press commits pending taps.
|
||||
void KeyboardSurface::openAlternates(const Pointer &pointer, qint64 timeMs)
|
||||
{
|
||||
if (pointer.index < 0)
|
||||
return; // the layout changed under the finger: no key to place it over
|
||||
for (auto it = m_pointers.begin(); it != m_pointers.end(); ++it) {
|
||||
if (it->serial != pointer.serial && ownsAlternates(*it)) {
|
||||
const Pointer owner = *it;
|
||||
m_pointers.erase(it);
|
||||
finish(owner, timeMs);
|
||||
break;
|
||||
}
|
||||
}
|
||||
QStringList choices = pointer.tracker.key().alternates;
|
||||
if (m_model->isUppercase()) {
|
||||
for (QString &choice : choices)
|
||||
choice = choice.toUpper();
|
||||
}
|
||||
m_alternates.open(m_model->keyRect(pointer.index), choices, width(), PopupGap);
|
||||
m_alternates.open(m_model->keyRect(pointer.index), choices, width(), -m_popupHeadroom, PopupGap);
|
||||
m_alternatesSerial = pointer.serial;
|
||||
}
|
||||
|
||||
bool KeyboardSurface::ownsAlternates(const Pointer &pointer) const
|
||||
{
|
||||
return pointer.serial == m_alternatesSerial && m_alternates.isOpen();
|
||||
}
|
||||
|
||||
void KeyboardSurface::closeAlternates()
|
||||
{
|
||||
m_alternates.close();
|
||||
m_alternatesSerial = 0;
|
||||
}
|
||||
|
||||
void KeyboardSurface::setTrackpadActive(bool active)
|
||||
@@ -253,13 +302,9 @@ void KeyboardSurface::setTrackpadActive(bool active)
|
||||
void KeyboardSurface::cancelAll()
|
||||
{
|
||||
const auto pointers = std::exchange(m_pointers, {});
|
||||
for (const Pointer &pointer : pointers) {
|
||||
if (m_model)
|
||||
release(pointer);
|
||||
if (m_controller)
|
||||
m_controller->keyCancelled(pointer.tracker.key());
|
||||
}
|
||||
m_alternates.close();
|
||||
for (const Pointer &pointer : pointers)
|
||||
cancel(pointer);
|
||||
closeAlternates();
|
||||
setTrackpadActive(false);
|
||||
}
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
#include <QHash>
|
||||
#include <QPointer>
|
||||
#include <QQuickItem>
|
||||
#include <limits>
|
||||
|
||||
// Receives all touch points (and the mouse) over the keyboard, runs one
|
||||
// PointerTracker per finger, and turns finished gestures into controller
|
||||
@@ -21,6 +22,9 @@ class KeyboardSurface : public QQuickItem
|
||||
Q_PROPERTY(KeyboardController *controller READ controller WRITE setController NOTIFY controllerChanged)
|
||||
Q_PROPERTY(AlternatesPicker *alternates READ alternates CONSTANT)
|
||||
Q_PROPERTY(bool trackpadActive READ isTrackpadActive NOTIFY trackpadActiveChanged)
|
||||
// Room above the surface that long-press popups may use; taller keys put
|
||||
// top-row popups lower, over their key, instead of cutting them off.
|
||||
Q_PROPERTY(qreal popupHeadroom MEMBER m_popupHeadroom NOTIFY popupHeadroomChanged)
|
||||
|
||||
public:
|
||||
explicit KeyboardSurface(QQuickItem *parent = nullptr);
|
||||
@@ -36,6 +40,7 @@ Q_SIGNALS:
|
||||
void modelChanged();
|
||||
void controllerChanged();
|
||||
void trackpadActiveChanged();
|
||||
void popupHeadroomChanged();
|
||||
|
||||
protected:
|
||||
void touchEvent(QTouchEvent *event) override;
|
||||
@@ -60,10 +65,13 @@ private:
|
||||
void longPressElapsed(int id, quint64 serial);
|
||||
|
||||
void finish(const Pointer &pointer, qint64 timeMs);
|
||||
void cancel(const Pointer &pointer);
|
||||
void release(const Pointer &pointer);
|
||||
void commitPendingTaps(qint64 timeMs);
|
||||
void updateSlideHover(Pointer &pointer);
|
||||
void openAlternates(const Pointer &pointer);
|
||||
void openAlternates(const Pointer &pointer, qint64 timeMs);
|
||||
bool ownsAlternates(const Pointer &pointer) const;
|
||||
void closeAlternates();
|
||||
void setTrackpadActive(bool active);
|
||||
void cancelAll();
|
||||
void forgetIndices();
|
||||
@@ -71,7 +79,10 @@ private:
|
||||
QPointer<LayoutModel> m_model;
|
||||
QPointer<KeyboardController> m_controller;
|
||||
AlternatesPicker m_alternates;
|
||||
quint64 m_alternatesSerial = 0; // the pointer the popup belongs to
|
||||
QHash<int, Pointer> m_pointers;
|
||||
quint64 m_nextSerial = 1;
|
||||
qint64 m_lastEventMs = 0; // timers have no event time; they use this
|
||||
bool m_trackpadActive = false;
|
||||
qreal m_popupHeadroom = std::numeric_limits<qreal>::max();
|
||||
};
|
||||
|
||||
@@ -12,7 +12,7 @@ private Q_SLOTS:
|
||||
void opensAboveKeyExtendingRight()
|
||||
{
|
||||
AlternatesPicker picker;
|
||||
picker.open(QRectF(100, 100, 50, 60), {u"é"_s, u"è"_s, u"ê"_s}, 1000, 8);
|
||||
picker.open(QRectF(100, 100, 50, 60), {u"é"_s, u"è"_s, u"ê"_s}, 1000, -1000, 8);
|
||||
QVERIFY(picker.isOpen());
|
||||
QCOMPARE(picker.rect(), QRectF(100, 32, 150, 60));
|
||||
QCOMPARE(picker.selectedText(), u"é");
|
||||
@@ -21,7 +21,7 @@ private Q_SLOTS:
|
||||
void staysInsideRightEdgeAndSelectsUnderFinger()
|
||||
{
|
||||
AlternatesPicker picker;
|
||||
picker.open(QRectF(900, 100, 50, 60), {u"a"_s, u"b"_s, u"c"_s}, 1000, 8);
|
||||
picker.open(QRectF(900, 100, 50, 60), {u"a"_s, u"b"_s, u"c"_s}, 1000, -1000, 8);
|
||||
QCOMPARE(picker.rect().left(), 850.0);
|
||||
QCOMPARE(picker.selectedText(), u"b"); // key center x=925 lies over the second cell
|
||||
}
|
||||
@@ -29,7 +29,7 @@ private Q_SLOTS:
|
||||
void selectionFollowsFingerAndClamps()
|
||||
{
|
||||
AlternatesPicker picker;
|
||||
picker.open(QRectF(100, 100, 50, 60), {u"a"_s, u"b"_s, u"c"_s}, 1000, 8);
|
||||
picker.open(QRectF(100, 100, 50, 60), {u"a"_s, u"b"_s, u"c"_s}, 1000, -1000, 8);
|
||||
picker.selectAt(210);
|
||||
QCOMPARE(picker.selected(), 2);
|
||||
picker.selectAt(-50);
|
||||
@@ -41,11 +41,25 @@ private Q_SLOTS:
|
||||
void closeClears()
|
||||
{
|
||||
AlternatesPicker picker;
|
||||
picker.open(QRectF(0, 0, 10, 10), {u"a"_s}, 100, 0);
|
||||
picker.open(QRectF(0, 0, 10, 10), {u"a"_s}, 100, -1000, 0);
|
||||
picker.close();
|
||||
QVERIFY(!picker.isOpen());
|
||||
QVERIFY(picker.selectedText().isEmpty());
|
||||
}
|
||||
|
||||
void emptyKeyRectDoesNotOpen()
|
||||
{
|
||||
AlternatesPicker picker;
|
||||
picker.open(QRectF(), {u"a"_s, u"b"_s}, 1000, -1000, 8);
|
||||
QVERIFY(!picker.isOpen());
|
||||
}
|
||||
|
||||
void lowersToStayBelowMinTop()
|
||||
{
|
||||
AlternatesPicker picker;
|
||||
picker.open(QRectF(100, 10, 50, 60), {u"a"_s}, 1000, -20, 8);
|
||||
QCOMPARE(picker.rect().top(), -20.0);
|
||||
}
|
||||
};
|
||||
|
||||
QTEST_GUILESS_MAIN(TestAlternatesPicker)
|
||||
|
||||
@@ -124,6 +124,24 @@ private Q_SLOTS:
|
||||
QVERIFY(!surface->alternates()->isOpen());
|
||||
}
|
||||
|
||||
void secondLongPressFinishesFirst()
|
||||
{
|
||||
const QPoint e = keyCenter(u"e"_s), u = keyCenter(u"u"_s);
|
||||
auto *surface = qobject_cast<KeyboardSurface *>(keyboard);
|
||||
QTest::touchEvent(&window, touchscreen).press(0, e);
|
||||
QTest::qWait(LongPressWaitMs);
|
||||
QTest::touchEvent(&window, touchscreen).stationary(0).press(1, u);
|
||||
QTest::qWait(LongPressWaitMs); // u's popup replaces e's, which types its choice
|
||||
QCOMPARE(engine.output, u"é");
|
||||
QVERIFY(surface->alternates()->isOpen());
|
||||
QTest::touchEvent(&window, touchscreen).release(0, e).stationary(1);
|
||||
QCOMPARE(engine.output, u"é"); // the first finger's lift does nothing more
|
||||
QVERIFY(surface->alternates()->isOpen());
|
||||
QTest::touchEvent(&window, touchscreen).release(1, u);
|
||||
QCOMPARE(engine.output.size(), 2);
|
||||
QVERIFY(!surface->alternates()->isOpen());
|
||||
}
|
||||
|
||||
void secondFingerCommitsFirst()
|
||||
{
|
||||
const QPoint a = keyCenter(u"a"_s), s = keyCenter(u"s"_s);
|
||||
|
||||
Reference in New Issue
Block a user