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

Fix intermittent blank price cells in offer book view #4420

Merged
merged 2 commits into from
Aug 31, 2020

Conversation

stejbac
Copy link
Contributor

@stejbac stejbac commented Aug 20, 2020

Replace faulty cell update logic, which uses a ChangeListener<Scene>, added in July 2017 (#73f21399) to keep the price column in the offer book table up to date, as it appears to occasionally result in blank cells. Also it seems only the prices, not the volumes, were being kept in sync with the market price feed.

Make the price and volume cells stateless and keep them in sync with the market feed by adding it as a dependency of each OfferBookListItem Observable generated by the cell value factory, instead of directly attaching listeners to it. In this way, TableCell::updateItem will be called by the framework whenever the price/volume needs updating.

(This does have the disadvantage that if the price feed is unavailable, causing Offer::getPrice to return null, then the cells will reflect that immediately instead of showing any old, stale values, but that is necessary for the UI to behave consistently anyway.)

--

This is to fix issues like the following, that I have occasionally observed in the Sell Bitcoin offer book, filtered for BSQ (though never in the Buy Bitcoin offer book or for other counter currencies for some reason):

s/isAnyPricePresent/isAnyPriceAbsent/g

Also use a lambda to shorten 'currenciesUpdatedListener' slightly.
Replace faulty cell update logic, which uses a ChangeListener<Scene>,
added in July 2017 (#73f21399) to keep the price column in the offer
book table up to date, as it appears to occasionally result in blank
cells. Also it seems only the prices, not the volumes, were being kept
in sync with the market price feed.

Make the price and volume cells stateless and keep them in sync with the
market feed by adding it as a dependency of each OfferBookListItem
Observable generated by the cell value factory, instead of directly
attaching listeners to it. In this way, TableCell::updateItem will be
called by the framework whenever the price/volume needs updating.

(This does have the disadvantage that if the price feed is unavailable,
causing Offer::getPrice to return null, then the cells will reflect that
immediately instead of showing any old, stale values, but that is
necessary for the UI to behave consistently anyway.)
@stejbac
Copy link
Contributor Author

stejbac commented Aug 20, 2020

(I seem to be having some difficulty attaching any PNGs or editing the above description in my current browser for some reason - perhaps a bug in the GitHub Javascript?)

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

ACK

I think showing nothing when there is no feed is better than showing an old price, so that's also an improvement in my opinion.

@sqrrm sqrrm added this to the v1.3.8 milestone Aug 31, 2020
@sqrrm sqrrm merged commit 4d32da7 into bisq-network:master Aug 31, 2020
@stejbac
Copy link
Contributor Author

stejbac commented Sep 22, 2020

Here is a screenshot of the problem I meant to attach earlier (but couldn't as I was using an old unsupported browser):

Screenshot from 2020-07-24 15-30-49

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