Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Show number of confs for incoming onchain txs in activity list #2792

Merged
merged 4 commits into from
Nov 7, 2019

Conversation

korhaliv
Copy link
Member

Description:

Resoles #1685

How Has This Been Tested?

Manually

Screenshots (if appropriate):

image
image
image

Types of changes:

Enhancement/feature

Checklist:

  • My code follows the code style of this project.
  • I have reviewed and updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes where needed.
  • All new and existing tests passed.
  • My commits have been squashed into a concise set of changes.

@korhaliv korhaliv added this to the v0.6.0-beta milestone Aug 22, 2019
@korhaliv korhaliv requested a review from mrfelton August 22, 2019 10:42
@korhaliv korhaliv self-assigned this Aug 22, 2019
@korhaliv korhaliv requested a review from JimmyMow August 22, 2019 10:42
@korhaliv korhaliv force-pushed the feat/unconf-conf-tx branch 2 times, most recently from 16171d5 to db75ced Compare August 22, 2019 11:14
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Tested ACK db75ced

@JimmyMow JimmyMow added the status: needs design issue needs designs label Aug 29, 2019
Copy link
Member

@JimmyMow JimmyMow left a comment

Choose a reason for hiding this comment

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

I've added Needs Design tag to this PR. I'd like to take a shot at improving the UI/UX here

@mrfelton mrfelton added type: enhancement New feature or request and removed type: enhancement labels Sep 4, 2019
@mrfelton mrfelton added the status: needs rebase Issue needs to be rebased on top of the main branch label Oct 14, 2019
@mrfelton mrfelton changed the title feat(ui): show number of confs for incoming onchain txs in activity list Show number of confs for incoming onchain txs in activity list Oct 31, 2019
@JimmyMow
Copy link
Member

JimmyMow commented Nov 5, 2019

Hey @mrfelton @korhaliv.

When discussing with Namson and Ole, we all feel that showing the number of confirmations is an unnecessary addition to the UI, as it is always one click away in the detail modal.

I do like the idea of being able to visually see what transactions are unconfirmed, but I think that is as far as this PR should go personally. What are your thoughts there?

mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
mrfelton added a commit to mrfelton/zap-desktop that referenced this pull request Nov 6, 2019
@korhaliv korhaliv force-pushed the feat/unconf-conf-tx branch from db75ced to ea0b045 Compare November 7, 2019 11:52
@korhaliv korhaliv requested a review from mrfelton November 7, 2019 11:52
@korhaliv korhaliv removed status: needs design issue needs designs status: needs rebase Issue needs to be rebased on top of the main branch labels Nov 7, 2019
@korhaliv korhaliv force-pushed the feat/unconf-conf-tx branch 2 times, most recently from 8f96fb8 to d82f3ab Compare November 7, 2019 12:03
@coveralls
Copy link

coveralls commented Nov 7, 2019

Coverage Status

Coverage decreased (-0.03%) to 22.643% when pulling b8867b9 on korhaliv:feat/unconf-conf-tx into 576dade on LN-Zap:next.

@korhaliv korhaliv force-pushed the feat/unconf-conf-tx branch from d82f3ab to b8867b9 Compare November 7, 2019 13:14
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Tested ACK b8867b9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants