fix(walletkit): keep sign button visible for long dApp messages - #963
Conversation
The SEP-53 message ScrollView had no height bound, so a long message grew the bottom sheet content past the screen — clipping the Cancel/Confirm buttons — and the unbounded ScrollView never scrolled, making the tail of the message unreachable. Cap the message area at 30% of screen height (mirroring the auth-entry sheet's capped XDR view) so long messages scroll inside the box and the action buttons always stay on screen. Fixes stellar#957 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Caps long WalletConnect messages to preserve bottom-sheet actions and adds component tests.
Changes:
- Limits message content to 30% of screen height.
- Adds rendering, JSON formatting, and height-bound tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/components/screens/WalletKit/DappMessageDisplay.tsx |
Adds bounded scrolling for long messages. |
__tests__/components/screens/WalletKit/DappMessageDisplay.test.tsx |
Tests message rendering and height constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <ScrollView | ||
| // Cap the message area so long messages scroll instead of growing the | ||
| // sheet past the screen and pushing the action buttons out of view. | ||
| style={{ maxHeight: Dimensions.get("window").height * 0.3 }} |
There was a problem hiding this comment.
Addressed in 98ae112. Rather than estimating per-state fixed content, the sheet content is now bounded to the usable window height (window minus safe-area insets and sheet chrome) and the message box gets flexShrink with a 56pt minimum scroll window. The layout engine sizes the message from the space actually remaining, so the banner and stacked warning buttons squeeze the (scrollable) message instead of clipping the actions. The 30% cap remains as the upper bound on large screens.
Converting the sheet to scrollable-with-pinned-footer was considered but rejected for this PR: all three request content types (DappSignTransactionBottomSheetContent, DappSignAuthEntryBottomSheetContent, this one) embed DappRequestButtons themselves, so that refactor would restructure the sign-transaction flow as well — out of scope for this bug fix.
The 1KB cap was attributed to SEP-53, but the spec imposes no message size limit and the browser extension enforces none. dApps signing JSON payloads routinely exceed 1KB and were rejected with "Message too long" before the sign sheet even opened. Keep a sanity cap against absurd WalletConnect payloads, but raise it to 10KB so real-world payloads fit. Update the error strings and the inaccurate spec attributions. Part of stellar#957 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d room The 30% screen-height cap on the message area assumed the normal action layout. Flagged requests add a security banner and stacked warning buttons (~150pt extra), which could still push the actions off a small screen. Bound the sheet content to the usable window height and give the message box flexShrink with a minimum scroll window, so the layout engine sizes the message from the space actually remaining — banners and stacked buttons now squeeze the message (which scrolls) instead of clipping the action buttons. Addresses Copilot review on stellar#963. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@CassioMG @piyalbasu could you please review this fix? |
JakeUrban
left a comment
JakeUrban
left a comment
There was a problem hiding this comment.
Hey @SmolinPavel, thanks a bunch for the contribution. I tested it out and it looks great, and I don't have any comments.
Fixes #957
Problem
When a dApp requests a SEP-53 message signature with a long payload (e.g. JSON):
Root causes & fixes
1. Unbounded message view clipped the action buttons
DappMessageDisplayrendered the message in aScrollViewwith no height bound. An unconstrainedScrollViewgrows to its content height, so a long message grew the bottom sheet content past the screen (clipping the action buttons under dynamic sizing), and the ScrollView itself never scrolled — cutting off the rest of the message.Fix: cap the message scroll area at 30% of screen height, mirroring the existing pattern in
DappSignAuthEntryBottomSheetContent(which caps its XDR view at 20% — the sign-message sheet has less fixed content, so it can afford a larger cap; verified the buttons still fit on a 667pt iPhone SE). Long messages now scroll inside the box, the full message is always readable, and the action buttons always stay on screen.2. 1KB size cap misattributed to SEP-53
SIGN_MESSAGE_MAX_BYTESwas 1024 with a comment claiming the limit comes from SEP-53 — but the spec imposes no message size limit, and the browser extension enforces none. dApps signing JSON payloads routinely exceed 1KB.Fix: raise the cap to 10KB (kept as a sanity cap against absurd WalletConnect payloads), correct the spec attributions, and update the en/pt error strings.
Testing
DappMessageDisplay: renders plain messages, pretty-prints JSON, and asserts the scroll area has a bounded max height.walletKitValidationtests: multi-KB JSON payload accepted, boundary tests now relative to the constant, regression test pinning the 10KB limit.tsc --noEmit, ESLint, and Prettier all pass.🤖 Generated with Claude Code