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 Issue #691 - Flash on Price Change #870

Closed
wants to merge 43 commits into from

Conversation

startailcoon
Copy link
Contributor

Proposal for Issue #691

Not sure if this is the most efficient model, but it works when I try it.

@startailcoon
Copy link
Contributor Author

startailcoon commented Dec 14, 2017

Still to do:

  • Consider flash colors
  • Consider if more than %-change should flash

@@ -58,12 +59,40 @@ class MarketGroup extends React.Component {
return (
!utils.are_equal_shallow(nextState, this.state) ||
!utils.are_equal_shallow(nextProps.markets, this.props.markets) ||
nextState.marketsCache !== this.state.marketsCache ||
Copy link
Contributor

Choose a reason for hiding this comment

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

This will always return true, use utils.are_equal_shallow to compare arrays or objects.

- Flash expires correctly
@startailcoon
Copy link
Contributor Author

startailcoon commented Dec 14, 2017

Preview of the function. The delay is most likely due to me debugging and running a lot at the same time. The code adds/removes the desired flash class and is up to the browser/system.

bitshares-marketflashonchange

svk31 and others added 4 commits December 15, 2017 10:16
* added button to create account from accounts page

* Add button to create new account in local wallet mode #800
- Flash only once on market change
- Flash timer 2 seconds

(WIP) Dashboard:
- Initial setup
@startailcoon
Copy link
Contributor Author

When trying to work with the change on the portfolio dashboard I get the following issue, and I have no idea how to fix it.

image

@startailcoon startailcoon changed the title Fix Issue #691 - Flash on Price Change (WIP) Fix Issue #691 - Flash on Price Change Dec 17, 2017
@startailcoon startailcoon changed the title (WIP) Fix Issue #691 - Flash on Price Change Fix Issue #691 - Flash on Price Change Dec 17, 2017
@@ -252,6 +278,9 @@ class AccountOverview extends React.Component {
const canDepositWithdraw = !!this.props.backedCoins.get("OPEN", []).find(a => a.symbol === asset.get("symbol"));
const canWithdraw = canDepositWithdraw && (hasBalance && balanceObject.get("balance") != 0);
const canBuy = !!this.props.bridgeCoins.get(symbol);
const changeClass = !marketsCache[asset.get("id")] ? "" : marketsCache[asset.get("id")][1] == 0 ? "" : marketsCache[asset.get("id")][1] < 0 ? marketsCache[asset.get("id")][2] ? "pulsate-down" : "change-down" : marketsCache[asset.get("id")][2] ? "pulsate-up" : "change-up";
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this line does what you think it does. The syntax is incorrect (too many : parts like here: ? "pulsate-down" : "change-down" : marketsCache[asset.get("id")][2]) and marketsCache[asset.get("id")][1] is a string so you should parse it before comparing it to 0.


case "change":
let change = utils.format_number(stats && stats.change ? stats.change : 0, 2);
let changeClass = change === "0.00" ? "" : change > 0 ? flash ? " pulsate-up" : " change-up" : flash ? " pulsate-down" : " change-down";
Copy link
Contributor

Choose a reason for hiding this comment

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

The syntax is incorrect here as well.

}

let change_on_price = utils.format_number(stats && stats.change ? stats.change : 0, 2);
let changeClass_on_price = change_on_price === "0.00" ? "" : change_on_price > 0 ? flash ? " pulsate-up" : " change-up" : flash ? " pulsate-down" : " change-down";
Copy link
Contributor

Choose a reason for hiding this comment

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

And here

}

marketsCache[id][0] = timestamp;
marketsCache[id][1] = change;
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend using parseFloat here for the change value.

marketsCache[id] = [timestamp, "0.00", false];
}

if(change != marketsCache[id][1] && marketsCache[id][1] != "0.00") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Parsing as float below means you can use !== 0 here. In general you should always use strict equalities.

let marketChangeValue = this.getValue();
let dayChangeClass = parseFloat(marketChangeValue) === 0 ? "" : parseFloat(marketChangeValue) < 0 ? "change-down" : "change-up";
let dayChangeClass = parseFloat(marketChangeValue) === 0 ? "" : parseFloat(marketChangeValue) < 0 ? flash ? "pulsate-down" : "change-down" : flash ? "pulsate-up" : "change-up";
Copy link
Contributor

Choose a reason for hiding this comment

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

This syntax is also incorrect.

@startailcoon
Copy link
Contributor Author

I will rework this in a new branch and make a new PR once completed.

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.

None yet

8 participants