PlotLayer add axes titles #671
Conversation
|
|
There was a problem hiding this comment.
@Firenze11 Very cool PR, I like it!
A possible concern: how does this work with longer labels? However, since this PR is inside an example and not a production layer, it should be OK if doesn't handle all cases.
@Pessimistress you wrote the original example.
examples/plot/app.js
Outdated
| resolution={200} | ||
| showAxis={true} /> | ||
| showAxis={true} | ||
| onHover={this._onHover.bind(this)} /> |
There was a problem hiding this comment.
Better to pre-bind this in constructor as Function.bind() generates a new function every render and can thus defeat shouldComponentUpdate.
| "build": "NODE_ENV=production webpack --env.prod=true" | ||
| }, | ||
| "dependencies": { | ||
| "d3-scale": "^1.0.6", |
There was a problem hiding this comment.
Is this used in the PR? Or was it a missing dependency?
There was a problem hiding this comment.
same question... is d3-scale used for scaling each axis?
There was a problem hiding this comment.
This was a missing dependency before the PR was added. Yes it is used to scale the axes.
|
|
||
| return [xTicks, yTicks, zTicks]; | ||
| return [ | ||
| [...xTicks, (bounds[0][0] + bounds[0][1]) / 2], |
There was a problem hiding this comment.
Does this PR handles the case that different axis has different ranges? You once mentioned that you'd like to scale each axis differently.
There was a problem hiding this comment.
Not yet. The solution to axes scaling was implemented by @Pessimistress probably on some other branch.
|
Also.. github says you have not signed CLA |
|
|
||
| - Default: `['x', 'z', 'y']` | ||
|
|
||
| Strings to draw next to each axis as their titles, note that the second element is the axis that points upwards. |
showed tooltip in plotlayer example PlotLayer add axes titles PlotLayer add axes titles - small modification binding methods in constructor updating readme for axesTitles
5 participants
Allow passing in X, Y, Z axes title strings, displaying them along the axes.
