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

feat: transaction list revamp #2744

Merged
merged 50 commits into from
Nov 9, 2023
Merged

feat: transaction list revamp #2744

merged 50 commits into from
Nov 9, 2023

Conversation

reneaaron
Copy link
Contributor

@reneaaron reneaaron commented Sep 8, 2023

Describe the changes you have made in this PR

transaction list revamp

Design

https://www.figma.com/file/IvlUHnC3vkIWnQ14FppZqT/%F0%9F%8E%A8-Extension-UI-revamp?node-id=101%3A62629&mode=dev

@pavanjoshi914 pavanjoshi914 marked this pull request as ready for review September 26, 2023 09:44
@rolznz
Copy link
Contributor

rolznz commented Sep 26, 2023

@pavanjoshi914 I get some NaN sats transactions
image

@rolznz
Copy link
Contributor

rolznz commented Sep 26, 2023

@pavanjoshi914 can you please update the PR description and link to a ticket?

@rolznz
Copy link
Contributor

rolznz commented Sep 26, 2023

Arrow icons do not match the figma (sizing and thickness), can they be updated?

@pavanjoshi914
Copy link
Contributor

@pavanjoshi914 I get some NaN sats transactions image

unable to reproduce this on my side. kindly check in production

@pavanjoshi914
Copy link
Contributor

Arrow icons do not match the figma (sizing and thickness), can they be updated?

bitcoin designs has only those icon, we need custom fat arrow icons if we want the icon similar to design. i didn't put much priority on icons currently

setup callback communication
minor naming corrections
shift publisher link in the transaction modal
correct variable names, show total fees in form of formatted sats
signed-off-by: pavan joshi <pavanj914@gmail.com>
@reneaaron
Copy link
Contributor Author

reneaaron commented Oct 30, 2023

Alby Commando (core-lightning) LNBits LND LNDHub
Incoming
- Empty
- Memo
- LNURLP Comment
- Boostagram
Outgoing
- Empty
- Memo

Other connectors currently have no support for displaying transactions.

Copy link
Contributor Author

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

Looks quite good so far, some feedback to address ⬇️

src/app/components/TransactionsTable/index.tsx Outdated Show resolved Hide resolved
src/app/hooks/useTransactions.ts Show resolved Hide resolved
src/extension/background-script/actions/ln/transactions.ts Outdated Show resolved Hide resolved
src/extension/background-script/actions/ln/transactions.ts Outdated Show resolved Hide resolved
src/i18n/locales/en/translation.json Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
@reneaaron
Copy link
Contributor Author

reneaaron commented Nov 6, 2023

TODO

  • Add decode boostagrams to outgoing transactions
  • LNC: Filter unsettled invoices in memory
  • Invoices API: Remove isSettled from args (not used at all)
  • Display the fees on transaction details (also if they are 0)

Follow-ups

@reneaaron
Copy link
Contributor Author

tACK

@reneaaron reneaaron merged commit 9e24c36 into master Nov 9, 2023
7 of 8 checks passed
@reneaaron reneaaron deleted the feat/transaction-list branch November 9, 2023 22:28
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.

3 participants