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

Show current tx fee rate on main screen #4676

Merged
merged 2 commits into from Oct 24, 2020
Merged

Show current tx fee rate on main screen #4676

merged 2 commits into from Oct 24, 2020

Conversation

ghost
Copy link

@ghost ghost commented Oct 21, 2020

Tx fee rate is shown in the footer alongside Synchronized with Bitcoin Mainnet (using Tor) (x sat/vB).

Fixes #4412

Screenshot:

image

Tx fee rate is shown in the footer alongside "Synchronized with
Bitcoin Mainnet (using Tor) (x sat/vB)".

Fixes #4412
@ripcurlx
Copy link
Contributor

I think we might need more context for everyone to understand this number. What do you think about changing it to: Synchronized with Bitcoin Mainnet (using Tor) / Bitcoin fee: x sat/vB? Or something like that.

@cd2357
Copy link
Contributor

cd2357 commented Oct 21, 2020

I think we might need more context for everyone to understand this number. What do you think about changing it to: Synchronized with Bitcoin Mainnet (using Tor) / Bitcoin fee: x sat/vB? Or something like that.

Maybe instead of Bitcoin fee: x sat/vB => Feerate x sat/vB?

Fewer characters, less space taken, with same meaning.

@ghost
Copy link
Author

ghost commented Oct 21, 2020

image

image

IMO both suggestions make it worse! [Just an opinion]. :trollface: ...but I'm OK to be overruled. 😛 ✌️

@cd2357
Copy link
Contributor

cd2357 commented Oct 21, 2020

The screenshots are very persuasive :) Your initial version looks the best.

@chimp1984
Copy link
Contributor

@jmacxx Consider to only show it at final sync state, so the space issues are not a problem as only at sync it gets long. Another option is to hide the "synchroonized with btc network" after a while or shorten it. The "using Tor" is not needed IMO.
I agree with @ripcurlx that we should show some context, as short as possible. @m52go Might have some good idea?
Maybe the whole footer text can be reviewed and shortened? Without context its a bit weird as the other data is about network. Fee rate is another domain.

@m52go
Copy link
Contributor

m52go commented Oct 21, 2020

  • Shorten "synchronized" to "synced"
  • Move Bitcoin network peer count to right with Bisq network peer count

Suggestion:

Left:
Synced with Bitcoin Mainnet (using Tor) / Current fee rate: 86 sat/vB

Right:
Bitcoin network peers: 9 / Bisq network peers: 9

@ghost
Copy link
Author

ghost commented Oct 22, 2020

@wiz Could you weigh in with an opinion?

image

@ghost
Copy link
Author

ghost commented Oct 22, 2020

@chimp1984 There is text added to the footer in the desktop MainViewModel.java which is unknown at the point where the bulk of the synced message is built in WalletAppSetup.java. So not sure how to avoid that. The Synchronizing with DAO pops in to the message briefly at each block received.

@chimp1984
Copy link
Contributor

Not sure as well without trying it out myself, but I think we do not need to show all info all the time. So if there is a dao block sync maybe remove some of the other info text. Or shorten it as well. Update messages are displayed there as a well...

    Shorten "synchronized" to "synced"
    Move Bitcoin network peer count to right with Bisq network peer count

Moved "Synchronizing DAO" to center for clarity/balance.
@ghost
Copy link
Author

ghost commented Oct 23, 2020

Ready!

image

image

image

@wiz
Copy link
Member

wiz commented Oct 23, 2020

adding the rate as a number is good, but also visualizing it with colors or a progress bar meter thing would be better

@chimp1984
Copy link
Contributor

adding the rate as a number is good, but also visualizing it with colors or a progress bar meter thing would be better

I fear that would overload it. UI is already heavy with information...

@wiz
Copy link
Member

wiz commented Oct 23, 2020

good point, but just something simple like <span color=red>100 sat/vB</span> would go a long way, would just need to think about what colors for what ranges

@chimp1984
Copy link
Contributor

Maybe @pedromvpg could add his opinion or make a suggestion? But probably not important enough to waste his precious time...

@ghost
Copy link
Author

ghost commented Oct 23, 2020

I'd prefer to keep it simple and be done with it. Only really wanted (x sat/vB) on display.

@pedromvpg
Copy link
Member

I think it looks great an it would be very helpful.

Thinking that adding color would potentially call too much attention to itself.
Would this be primarily to inform users fee's are particularly high? In relation to what? Some previous span of time? versus a 1 sat/byte fee?

Also, do we need to clarify the fee rate targets 1 block confirmation?

Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

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

utACK

@ghost ghost mentioned this pull request Oct 23, 2020
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx merged commit 55c663a into bisq-network:master Oct 24, 2020
@ripcurlx ripcurlx added this to the v1.5.0 milestone Nov 3, 2020
@ghost ghost mentioned this pull request Nov 26, 2020
@ghost ghost deleted the show_tx_fee branch May 29, 2022 22:45
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.

Show current tx fee rate on main screen
6 participants