feat: add memo to transaction history#829
feat: add memo to transaction history#829overbit wants to merge 3 commits intosolana-foundation:masterfrom
Conversation
|
@overbit is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Greptile OverviewGreptile SummaryThis PR adds transaction memo display functionality to the transaction history table, improving visibility of transaction metadata. Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TransactionHistoryCard
participant useAccountHistory
participant Connection
participant MemoField
User->>TransactionHistoryCard: Views account page
TransactionHistoryCard->>useAccountHistory: Fetch account history
useAccountHistory->>Connection: getSignaturesForAddress(pubkey)
Connection-->>useAccountHistory: ConfirmedSignatureInfo[] (includes memo field)
useAccountHistory->>TransactionHistoryCard: Returns transaction history
TransactionHistoryCard->>TransactionHistoryCard: Map transactions to rows
TransactionHistoryCard->>TransactionHistoryCard: Extract signatureInfo.memo
alt memo exists
TransactionHistoryCard->>MemoField: Render with memo text
MemoField->>MemoField: Clean memo (remove [n] prefix)
MemoField->>MemoField: Truncate if > 25 chars
MemoField-->>User: Display truncated memo with copy icon
User->>MemoField: Hover over truncated memo
MemoField-->>User: Show full memo in tooltip
else no memo
TransactionHistoryCard-->>User: Display "---"
end
|
|
|
||
| function MemoField({ memo }: { memo: string }) { | ||
| const [showTooltip, setShowTooltip] = React.useState(false); | ||
| const truncateLength = 25; |
There was a problem hiding this comment.
Could we have this at the props with 25 as the default value?
| function MemoField({ memo }: { memo: string }) { | ||
| const [showTooltip, setShowTooltip] = React.useState(false); | ||
| const truncateLength = 25; | ||
| // Remove memo length like "[15] " from the memo for display (handles all occurrences) |
There was a problem hiding this comment.
In the newer master, we have a rule to explicitly state the reason for regexps. That line would fail after merging.
I propose something like:
// eslint-disable-next-line no-restricted-syntax -- Solana RPC prepends `[length] ` to memo strings in getSignaturesForAddress responses; strip for display | const [showTooltip, setShowTooltip] = React.useState(false); | ||
| const truncateLength = 25; | ||
| // Remove memo length like "[15] " from the memo for display (handles all occurrences) | ||
| const cleanMemo = memo.replace(/\[\d+\]\s*/g, '').trim(); |
There was a problem hiding this comment.
Do we really need to replace all the occurrences in the Memo? What if this [N] format is selected by anyone for verification, for example
| onMouseOut={() => setShowTooltip(false)} | ||
| style={{ cursor: isTruncated ? 'pointer' : 'default' }} | ||
| > | ||
| <Copyable text={cleanMemo}> |
There was a problem hiding this comment.
thought: Why not to allow copying the original memo?
| </> | ||
| )} | ||
|
|
||
| <td>{signatureInfo.memo ? <MemoField memo={signatureInfo.memo} /> : '---'}</td> |
There was a problem hiding this comment.
nit: the memo is grey to match the other columns, but --- is white. I suppose it is better to use the same styles
Description
This pull request enhances the transaction history UI by adding support for displaying transaction memos, improving the user experience with memo truncation and tooltips, and updating layout variables for better responsiveness. The most important changes are grouped below:
Transaction History Table Enhancements
TransactionHistoryCard.tsx, displaying memos for each transaction if available.signatureInfoand render memos using the newMemoFieldcomponent. [1] [2]Memo Display Improvements
MemoFieldcomponent, which cleans up memo text, truncates long memos, and shows the full memo in a tooltip on hover. Also allows users to copy the memo easily.Layout and Responsiveness
xxlcontainer width variable in the SCSS layout configuration to support larger screens.Type of change
Screenshots
Testing
No additional RPC calls where implemented so testing was just about simple manual tests on different screen resolutions
Related Issues
Closes: #828
Checklist
build:infoscript to update build informationAdditional Notes