feat: share an order through the system share sheet - #672
Conversation
The app could already open order links but never produced one, so sharing an order meant reading its id aloud and having the other side look it up by hand. The `mostro:` scheme is registered in the manifest, NostrUtils parses `mostro:<order-id>?relays=...&mostro=<pubkey>`, and DeepLinkService resolves the order from those relays, switching Mostro instance when the link points at a different one. Only the encoder was missing. NostrUtils.buildMostroUrl now writes that same format, next to the two functions that read it. It keeps only relays carrying a WebSocket scheme, drops duplicates, caps the list so the link stays pasteable, and emits the instance pubkey only when it passes the check the parser applies, so a link this builds is always one the parser accepts. OrderShareLinkCard shows the link with a copy button, following the shape of the OrderIdCard beside it, and renders nothing when no relay can resolve the order. It appears on the order book's detail screen and, in My Trades, only while the order is still pending: a link to a trade already under way is of no use to whoever opens it. Adds orderLinkLabel and orderLinkCopiedMessage to the six locales.
Copying the order link means copy, leave the app, open the chat, paste. The share sheet takes the same link straight to Telegram, Signal, a note or anything else installed, in one gesture. It also allows something the clipboard cannot: sending the link with a sentence around it. A bare mostro:8927bb1d-...?relays=... lands in a chat as an unexplained string, and most apps only linkify http(s), so the recipient sees neither a tappable link nor a hint of what it is. It now travels as "Take this order on Mostro: <link>". Both actions stay: copying is still faster when the destination is not an app, such as a password manager or a QR generator. The share call follows the pattern the invoice widget already uses, taking everything it needs from the context before the await -- this card is a ConsumerWidget, so there is no mounted to check afterwards -- and reporting a failure through the snackbar rather than letting it escape. It passes sharePositionOrigin, which iPads need to anchor the sheet. Adds orderShareMessage and failedToShareOrder to the six locales. The card now holds two 48dp buttons next to a link long enough to wrap, which is the shape that produced the overflow in MostroP2P#654, so a layout test pins it. Confirmed that test fails against a card whose link is not flexible.
|
Warning Review limit reached
Next review available in: 11 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Request changes
The share-sheet addition itself looks straightforward: the card keeps copy and share, wraps the link in localized explanatory text, anchors the share sheet, and has widget coverage for the share method call and the two-button layout.
I am still requesting changes because this PR explicitly depends on #669 and ships the same generated mostro: link surface. #669 is not ready as reviewed, and the receiver-side path is still known-broken: #670 means a recipient opening one of these shared links while the app is not in the foreground can crash/fail to navigate. That is the primary use case for this PR (send the order to Telegram/Signal/WhatsApp/etc.). Until the link-opening path is fixed/merged, the share sheet would make it easier to distribute links that recipients cannot reliably open.
Please either:
- make this PR wait on a merged fix for #669 and #670, or
- include the receiver-side fix in this stack before closing #671.
Verification performed:
- Reviewed current head
99fc2e62288349b3882e12a2c3ca1f288737059cagainst basec3c2d7a7b318e70b2d7555f43d49ef1bfc009624. - Read the PR body, linked issue #671, PR comments, and current CI state.
- Parsed the touched ARB locale files with duplicate-key and placeholder checks; no new ARB issues found.
- Ran
git diff --checkon the changed files successfully. - Could not run
flutter testlocally because this environment does not haveflutteronPATH; GitHub'sbuildcheck is currently green for this head.
|
Agreed. The body now states both dependencies explicitly: this waits on #669 (where No code change here — the share sheet only makes the same links easier to send, so the fix belongs upstream in #673 rather than in this stack. |
|
same comment #669 (comment) I'm closing this and all other UI PRs, we only are fixing bugs on this apps, not new features, new features in https://github.com/MostroP2P/app please |
2 participants
Depends on #669 and #673
Cut from
feat/order-share-link, which is whereOrderShareLinkCardlives. Until #669 merges this diff carries its commits too — review against that branch, or merge #669 first.#673 fixes the receiving end (#670): until it lands, a recipient opening one of these links with the app closed does not reliably reach the order. Merge order: #673 → #669 → this.
Closes #671
Problem
#669 put the order's
mostro:link on the detail screen with a copy button, which was the whole of what #451 asked for. Copying still means copy → leave the app → open the chat → paste.There is also something the clipboard cannot do: send the link with an explanation. A bare
mostro:8927bb1d-…?relays=…lands in a chat as an unexplained string, and most apps only linkifyhttp(s), so the recipient gets neither a tappable link nor a hint of what they are looking at.Change
A share button beside the copy one in
OrderShareLinkCard, handing the link to the system share sheet wrapped in a sentence: "Take this order on Mostro:<link>".Both actions stay. Copying is still faster when the destination is not an app — a password manager, a terminal, a QR generator.
The share call follows the pattern
pay_lightning_invoice_widget.dartalready uses: everything it needs is read from the context before the await, since this card is aConsumerWidgetand has nomountedto check afterwards, and a failure is reported through the snackbar instead of escaping. It passessharePositionOrigin, which iPads need to anchor the sheet; the three existingShare.sharecalls in the repo omit it, and #80 will eventually want it.Adds
orderShareMessageandfailedToShareOrderto the six locales.share_pluswas already a direct dependency, so nothing new is pulled in.The card now carries both actions, with the link still elided to two lines:
Tapping share opens the system sheet with the link whole — sentence, three relays and the instance pubkey — which is what that elision hides:
Tests
test/shared/widgets/order_cards_test.dart, extending the group #669 added:dev.fluttercommunity.plus/sharewith text containing the full link, and asserts the text does not start withmostro:— the sentence is the point, so its absence should fail the test.RenderFlex overflowed by 152 pixels) against a card whose link is not flexible, so it is a real guard and not one that passes by construction.Test plan
flutter gen-l10n— no untranslated messagesflutter analyzeonlib/sharedandtest/shared— no issuesflutter test test/shared/widgets/order_cards_test.dart— 18 passingflutter test— 901 passing, the same 11 pre-existing failures as onmain(staletest/mocks.mocks.dart, repo issue Dart run build_runner build fails on Flutter 3.44.0, source_gen 3.1.0 incompatible with analyzer 8.x #606)Known limitation
A shared
mostro:link will not be tappable in most chat apps, so the recipient has to copy it and open it. Fixing that properly needs verifiedhttpslinks (App Links / Universal Links) redirecting to the scheme, which is a separate piece of work — worth its own issue if the maintainers want it.Note also #670: opening a
mostro:link while the app is not in the foreground currently crashes on a go_router assertion. Pre-existing and unrelated to this branch, but this feature makes such links common.