ticks.display: 'above' #6226
ticks.display: 'above' #6226
Conversation
etimberg
requested review from
simonbrunel,
benmccann and
nagix
and removed request for
benmccann
src/core/core.scale.js
Outdated
|
|
||
| if (!me._isVisible()) { | ||
| if (!me._isVisible() || (top && optionTicks.display !== 'top')) { |
There was a problem hiding this comment.
I'd probably have done this check in core.controller instead
There was a problem hiding this comment.
Not sure its worth digging in to box to see if its a scale and if its ticks should be drawn above in core.controller.
OTOH custom scales that don't implement this would be drawn twice now.
I'll look into this.
above instead of top makes more sense to me too.
|
Changed the option to 'above' instead of 'top'. Demo changed. It gets cluttery and ugly fast checking |
There was a problem hiding this comment.
I don't think display is the correct option to control that behavior because everywhere else, display means visibility (not order). Also, scale.display: 'auto' && scale.display: 'top' would not work while it's a valid use case if we want to control the order at the scale level. The option name/values combination is also confusing to me because it can be understood as the drawing position (x, y) instead of the drawing order (z).
| }, me); | ||
|
|
||
| me.drawDatasets(easingValue); | ||
|
|
||
| helpers.each(me.boxes, function(box) { |
There was a problem hiding this comment.
I'm really not fan of this extra loop calling draw() a second time for all scales and as you said, it would draw existing custom scales 2 times. I don't think scales should be aware of the drawing order and check internally this extra argument "to draw or not to draw". IMO, draw() should be called only one time before or after datasets.
There was a problem hiding this comment.
Talked with @kurkle and we will investigate another approach to implement this feature (using layers). Let's keep that PR open but mark it as "Request changes" to prevent merging for now.
|
Closed via #6241 |
Allow drawing tick labels after datasets.
Fixes: #2808
Fixes: #3724
Fixes: #3738
Fixes: #3722
Demo