Fixing ohlc showing wrong color when opening equals closing

[noway]

Haven't found how to contact maintainers (no irc/slack, apart from community.plot.ly which doesn't seem to be the appropriate place?) so posting the PR here.

Fixed the case when opening is equal to closing and it always showing "increasing" color. It should instead determine the color based on the relation of previous close price to close price of the new one, or, if that's equal too, on the color of the previous candle.

So that it renders correctly in the following case:

[etpinard]

Haven't found how to contact maintainers

Usually, contributors first open up an issue about the problem and discuss it there. Then contributors are encouraged to make a PR to their forked repo ask for a review there. Community PRs that don't reference an issue ticket can sometimes feel a little rude to FOSS maintainers.

No need to close your PR and redo the process though. Take it as an FYI.

---

About

Fixed the case when opening is equal to closing and it always showing "increasing" color. It should instead determine the color based on the relation of previous close price to close price of the new one, or, if that's equal too, on the color of the previous candle.

That sounds fair. But, can you confirm that this is the official way of handling this? How does this compare to what other graphing libraries are doing?

[noway]

bitcoinwisdom which is a really reputable example deals with it this way.

(in current version all of those dashes would be green)

[etpinard]

The algo looks good 👍

It might worth the time to make it a little DRYer by wrapping isPrevThisDirestion, oprev and cprev in a closure. That is, something like:

exports.getFilterFn = function(direction) {
  return new _getFilterFn(direction);
};

function _getFilterFn(directon) {
  var isPrevThisDirection = null;
  var oprev = null;
  var cprev = null;
  
  var fn;
  switch(direction) {
    case 'increasing':
      fn = function(o, c) { /* */ };
      break;
    case 'decreasing':
      fn = function(o, c) { /* */ };
      break;
  }

  return function(o, c) {
    var out = fn(o, c);
    isPrevThisDirection = !!out;
    oprev = o;
    cprev = c;
    return out;
  };
};

that way we wouldn't have to patch candlestick/transform.js and ohlc/transform.js.

[noway]

That's a good idea. Fixed, although haven't had a chance to test it.

[etpinard]

@noway thanks. If you're up for a challenge you can try adding a few test cases in this suite.

[noway]

Unfortunately I'd have to pass, can't allocate enough of a chunk of time. Otherwise I'm happy to help :-)

[etpinard]

Superseded by #1655