Skip to content

Conversation

@tvolk131
Copy link
Contributor

@tvolk131 tvolk131 commented Sep 6, 2025

Change the type within Message::Send from String to a new SendDestination type, which is an enum containing either a Bolt11Invoice, LnUrl, or bitcoin::Address. This forces the parsing of connection strings to be moved from the message processing code to the message creation code. This has two positive effects:

  • Removes the need to handle un-parseable strings in the message processing code (see the removal of the "Invalid invoice or address" toast)
  • Forces the "Send" button to use .on_press_maybe(), which disables the button unless a valid SendDestination can be parsed

Essentially, rather than allowing for the "Send" button to be pressed at any time, but presenting error toasts if pressed with an unparseable value, we now disable this button from even being pressed unless a parseable value is entered

Copilot AI review requested due to automatic review settings September 6, 2025 20:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the Send UI message to use strong typing by replacing the String parameter with a new SendDestination enum. This moves input validation from message processing to message creation, improving error handling and UX.

  • Introduces SendDestination enum with variants for Bolt11Invoice, LnUrl, and bitcoin::Address
  • Moves destination parsing from message handler to UI layer with parse_send_destination function
  • Changes Send button to use .on_press_maybe() to disable the button when input is invalid

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
harbor-ui/src/routes/send.rs Adds parsing function and updates Send button to use conditional enabling
harbor-ui/src/main.rs Defines SendDestination enum and refactors message handler to use typed destinations
harbor-ui/Cargo.toml Adds lnurl-rs dependency for LnUrl parsing
Comments suppressed due to low confidence (1)

harbor-ui/src/main.rs:1

  • This amount parsing logic is duplicated between the LnUrl and Address cases. Consider extracting it into a helper function to reduce code duplication.
#![warn(clippy::nursery, clippy::pedantic)]

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@tvolk131 tvolk131 force-pushed the strongly_typed_send_message branch from 61acbc7 to 00616fd Compare September 6, 2025 21:31
@benthecarman benthecarman merged commit b4b5705 into HarborWallet:master Sep 6, 2025
1 check passed
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.

2 participants