Skip to content

Conversation

1440000bytes
Copy link

image

@1440000bytes 1440000bytes marked this pull request as ready for review October 17, 2025 20:06
@luke-jr
Copy link
Collaborator

luke-jr commented Oct 17, 2025

Hmm, seems like it would be better to open a transaction-view dialog with more information. Maybe a right-click menu with copy txid to clipboard (I think we have that elsewhere too?)

@1440000bytes
Copy link
Author

Maybe a right-click menu with copy txid to clipboard (I think we have that elsewhere too?)

I thought about this earlier but found it worse in UX.

Copy link

@kwsantiago kwsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been tested on X11 systems where middle-click paste is expected to work?

const auto tx = gi ? gi->data(0).value<CTransactionRef>() : CTransactionRef();
if (tx) {
QString txid = QString::fromStdString(tx->GetHash().ToString());
QApplication::clipboard()->setText(txid);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use GUIUtil::setClipboard() instead of direct clipboard access using the helper in guiutil.cpp:904 since this current implementation doesn't support X11 Selection clipboard (the helper handles both modes)

For reference: transactionview.cpp:467 uses GUIUtil::copyEntryData() which calls setClipboard()

#endif
QToolTip::showText(event_global_pos, tr("Copied: %1").arg(txid), this, {}, -1);

return;
Copy link

@kwsantiago kwsantiago Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return skips QGraphicsView::mousePressEvent(event) (line 107) which could break panning/selection/other interactions when clicking on the transaction bubbles. Consider only returning if you don't want other mouse handling.

return QGraphicsView::viewportEvent(event);
}

void ScalingGraphicsView::mousePressEvent(QMouseEvent * const event)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left-click behavior is inconsistent with rest of the UI

  • For reference, transactionview.cpp:155 uses right-click context menu: "Copy transaction &ID"
  • @luke-jr suggestion about existing UX patterns
    • (right-click for actions, left-click for selection)

Copy link
Author

@1440000bytes 1440000bytes Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not using bubbles anywhere else in the UI. It is a better UX to just click and copy the transaction id.

Copy link

@kwsantiago kwsantiago Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing is whether there is an indicator for the user to know this functionality exists (click and copy the transaction id).

#else
const QPoint event_global_pos = event->globalPos();
#endif
QToolTip::showText(event_global_pos, tr("Copied: %1").arg(txid), this, {}, -1);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the intended behavior for the -1 value? Permanent until mouse moves?

Consider using a timed duration (such as 2000ms) for better tooltip timeout UX.

Copy link
Author

@1440000bytes 1440000bytes Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 is default and I tried other values from 2000 to 100000 but they didn't work as expected.

https://doc.qt.io/archives/qt-5.15/qtooltip.html#showText-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants