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

SCP-2545 - Marlowe Run Client - Implement transaction summary #3656

Merged
merged 4 commits into from
Jul 31, 2021

Conversation

hrajchert
Copy link
Contributor

This PR adds a tooltip to the past actions card with the timezone information and includes an initial version of the transaction/payment summary.

time-tooltip

payment_summary

While I was implementing this feature, I discovered a bug in the balance tab of the simple escrow example. When the buyer deposits and chooses to report a problem, there is a payment from the Sellers account to the Buyers account (which is shown by the new payment summary). But in the balances tab the money is removed from the Sellers account but is not added to the Buyers.

Pre-submit checklist:

  • Branch
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@hrajchert hrajchert requested review from merivale and palas July 31, 2021 00:10
Copy link
Contributor

@palas palas left a comment

Choose a reason for hiding this comment

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

Nice! I am a bit troubled by to being to the left of from, not gonna lie, but other than that :)

if min' `mod` 60 == zero then
show hours
else
show hours <> ":" <> formatMinutes (min' - hours * 60)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, it even considers minute offsets, cool :)

@palas
Copy link
Contributor

palas commented Jul 31, 2021

Oh, I see now IDeposit is that way around too. Anyway, I have changed that as I was fixing purty

@hrajchert
Copy link
Contributor Author

Yes, i dont like the order either, but i tried to be consistent with Deposit

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