HUE-324. Add ability for HueChart.Box charts to determine what data series and value the mouse is over.

Review Request #1245 - Created Nov. 19, 2010 and submitted

Marcus McLaughlin
Add methods and events so that interaction can occur with particular data series within points.

  1. this codebase is increasingly difficult for me to review. it's huge and very math oriented and protovis calls. documentation inline is going to increasingly be more important.
  2. getYRange? Why not getRange(value, ticks)?
  3. can I reiterate that the pattern should be this.scaleTicks.y? (and that options.padding should be an array or integer, not 4 different options)
  4. just call return here. right?
  5. why is this function defined via assignment here instead of in the object you pass to Class? I see your comment but "since it is so closely related to scale" doesn't answer my question.
  6. I hate that this function is only for y range and not just a generic range.
  7. this.scale.y
  8. don't use bind here; all the array iterators take a 2nd argument for binding.
  1. Sigh...hopefully you can still use this.  I needed more context into the lines, so I responded while viewing the diff, forgetting that ReviewBoard creates a new review.
  2. I'm not sure I understand what you're asking here.  Can you explain more ?
    1. why have getXRange, getYRange, getZRange? That's what variables are for. Even if you don't need to get the x value, why not make the interface more generic?
  3. options.padding takes an array, but it also allows you to use named values if you'd like.  the value is yScaleTicks because it's a scale for yTicks, but I can't imagine a situation in which I would ever need a corresponding x object.  I'd change it if I ever did though.
  4. Yep.  you're right.
  5. Hm...I'll change the comment.  This is much more of an internal function than anything else.  It's the method that converts the value from yScale.invert to a value that's appropriately based on a measurement from the bottom of the graph to the top.  It's defined by internal behavior of ProtoVis, not the desire of the user.  They'd never change this function and should never need to.
    1. but you're assigning it to this.yValueReverse. Are you using it elsewhere? if not, make it a stand alone function in a closure (perhaps outside the class - no need to re-define it every time you run this method). If it's something you use elsewhere, why not make it a _private method? Why recreate this function each time?
  6. That's reasonable, but it's because of the way that protovis returns yValues from the mouse.  getRange is much more of a data object question than this.  Perhaps there's a better way to name it.