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

app: balances widget #454

Merged
merged 2 commits into from
Jun 12, 2020
Merged

app: balances widget #454

merged 2 commits into from
Jun 12, 2020

Conversation

buck54321
Copy link
Member

With the changes introduced in #390 and #418, we can now display more detailed balance information.

  1. The displayed balance now includes locked and immature balance, and a bunch of new functionality to deal with the various states that the wallet might be in. The balance widget is moved below the order form. Also a number of style tweaks.
  2. If the wallet is not connected, and the balance is more than an hour old, a warning icon is shown with a tooltip on hover. If the wallet is connected though, a fresh balance is requested instead and a little loading icon is shown while it's fetched.
  3. Adds a tooltip. Just add a data-tooltip attribute to any element. Application does the rest.
  4. Generic icons are added for unsupported assets. It just looks better when there is something there. They are letters in colored circle background, and if an unsupported asset is seen, they will be requested instead based on the first letter of the ticker symbol.
  5. Move BalanceSet from client/core to client/db, and store the entire thing. Haven't changed the version yet, so you'll need to delete or cache your dexc.db for testing for now, but if multi: add client db upgrade infrastructure. #448 goes in first, I can update here accordingly. Moved the BalUpdate field from db.Wallet to db.BalanceSet.
  6. For the app test server, there is now an unsupported asset, Komodo, and a market for testing purposes. There are new test configuration variables that can be set to more easily achieve particular hard to reach states. Specifically, if a DEX is added, not all wallets will necessarily be connected, so it's possible for a market to be selected for an existing wallet, but not have any balance information for that particular DEX configuration yet.

And this one is specifically for the situation outlined in 6.

With the changes introduced in decred#390 and decred#418, the balance display can
now display more detailed information. Adds a balance widget to display
the categorized DEX-specific balance and the wallet state.
Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

Code reads well. Great changes to the UI here. Just a couple comments.

client/webserver/api.go Show resolved Hide resolved
client/webserver/site/src/js/doc.js Show resolved Hide resolved
client/webserver/site/src/js/markets.js Outdated Show resolved Hide resolved
@chappjc
Copy link
Member

chappjc commented Jun 9, 2020

5\. Move `BalanceSet` from client/core to client/db, and store the entire thing. Haven't changed the version yet, so you'll need to delete or cache your _dexc.db_ for testing for now, but if #448 goes in first, I can update here accordingly. Moved the `BalUpdate` field from `db.Wallet` to `db.BalanceSet`.

I suppose this would be a good exercise for writing a db upgrade function and all that. TBH I'm not 100% clear on that process, but @dnldd can help with that first 0 -> 1 upgrade.

Are we comfortable beginning db versions now? It has to happen darn soon, but if it's easier to get this in first and then start upgrading, we can do that.

@chappjc
Copy link
Member

chappjc commented Jun 10, 2020

The upcoming DB versioning and upgrade changes are becoming more clear now. This could go in ahead of #460, which generates reference v0 data, which is followed by #448 to do an initial v0 -> v1 upgrade that simply sets the version in the DB.

Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

All good, thanks. Would love a test drive from someone else though.

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

It looks great when I make the accounts!

But if I restart dexc, the ui gets a little wierd, the btc symbol doesn't show up, and there is a spiral progress bar:
bals2

The console error reads:

TypeError: o.balances.xc is null                                                                             markets.js:1317:16
    updateWallet markets.js:1317
    setWallets markets.js:1288
    handleBookRoute markets.js:544
    MarketsPage markets.js:152
    forward ws.js:37
    onmessage ws.js:97

And, could you make the disconnected tooltip have the white background too?
tooltip

@@ -627,11 +627,13 @@ func (c *Core) walletBalances(wallet *xcWallet) (*BalanceSet, error) {
for i, bal := range bals[1:] {
balMap[addrs[i]] = bal
}
coreBals := &BalanceSet{
coreBals := &db.BalanceSet{
Copy link
Member

Choose a reason for hiding this comment

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

core is now anatomically correct

@chappjc
Copy link
Member

chappjc commented Jun 12, 2020

I didn't see any issues in normal use, so I'm assuming the only errors happen when dexc is shutdown. Presumably refreshing the browser once dexc is running again restores normal function? We can address in a follow-up if there's a bigger issue.

@chappjc chappjc merged commit 892f3a3 into decred:master Jun 12, 2020
@JoeGruffins
Copy link
Member

made an issue #473

@chappjc chappjc mentioned this pull request Jun 26, 2020
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