Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADP-3479] Add deposit wallet payment page #4837

Merged
merged 15 commits into from
Dec 10, 2024

Conversation

paolino
Copy link
Collaborator

@paolino paolino commented Nov 8, 2024

  • Remove mock funding
  • Add payments page
  • Add a general purpose modal component
  • Restyle the boxes to get a better readability

ADP-3479

@paolino paolino self-assigned this Nov 8, 2024
@paolino paolino added Deposit UI UI related changes labels Nov 8, 2024
@paolino paolino force-pushed the paolino/ADP-3479/add-payment-page branch from 608ad20 to fb9289a Compare November 9, 2024 19:03
Base automatically changed from paolino/ADP-3479/add-address-codec-to-deposit-wallet to master November 10, 2024 08:57
@paolino paolino force-pushed the paolino/ADP-3479/add-payment-page branch 14 times, most recently from 6b4010d to 28ab6b0 Compare November 14, 2024 12:09
@paolino paolino changed the base branch from master to paolino/ADP-3479/split-pure-module November 14, 2024 12:10
@paolino paolino force-pushed the paolino/ADP-3479/add-payment-page branch 2 times, most recently from 5e4340b to bb3d325 Compare November 14, 2024 12:14
Base automatically changed from paolino/ADP-3479/split-pure-module to master November 14, 2024 14:33
@paolino paolino force-pushed the paolino/ADP-3479/add-payment-page branch 7 times, most recently from 599efb3 to 69c34dc Compare November 21, 2024 09:58
@paolino paolino force-pushed the paolino/ADP-3479/fix-private-key-creation branch 2 times, most recently from 4ef48d6 to 0200b5a Compare December 5, 2024 14:22
@paolino paolino force-pushed the paolino/ADP-3479/add-payment-page branch 2 times, most recently from 6a21e81 to bf9a4a7 Compare December 6, 2024 08:36
@paolino paolino force-pushed the paolino/ADP-3479/fix-private-key-creation branch 2 times, most recently from 2876118 to 925cd15 Compare December 6, 2024 08:54
@paolino paolino force-pushed the paolino/ADP-3479/add-payment-page branch from bf9a4a7 to bb8e1d7 Compare December 6, 2024 08:55
Base automatically changed from paolino/ADP-3479/fix-private-key-creation to master December 6, 2024 10:37

showLovelaceAsAda :: Integral a => a -> String
showLovelaceAsAda c =
showFFloatAlt @Double (Just 2) (fromIntegral c / 1_000_000) ""
Copy link
Member

Choose a reason for hiding this comment

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

It seems odd that I can enter 10.000001 as payment and not see the result:

Suggested change
showFFloatAlt @Double (Just 2) (fromIntegral c / 1_000_000) ""
showFFloatAlt @Double (Just 6) (fromIntegral c / 1_000_000) ""

Copy link
Member

Choose a reason for hiding this comment

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

Also, I wonder if the Double precision could be problematic...

Copy link
Collaborator

Choose a reason for hiding this comment

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

The maximum amount of Lovelace one can define is bounded to be a Word64 if I am not mistaken, which should be representable as Double with some loss of precision. Do we want an exact representation? If so then this code is incorrect. If we don't really care which I think we don't because this UI will probably not go into production as is, then this is fine.

Copy link
Collaborator

@abailly abailly left a comment

Choose a reason for hiding this comment

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

I am tempted to approve this PR even though I did not went through it in details (it's adding 2000 lines which is way too much IMO), for the sake of moving forward and given the fact we've actually demoed this feature last week.

@abailly abailly force-pushed the paolino/ADP-3479/add-payment-page branch from 5718b41 to 8f6fd06 Compare December 9, 2024 17:09
@abailly abailly added this pull request to the merge queue Dec 10, 2024
Merged via the queue into master with commit 5cf6c6c Dec 10, 2024
24 checks passed
@abailly abailly deleted the paolino/ADP-3479/add-payment-page branch December 10, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deposit UI UI related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants