-
Notifications
You must be signed in to change notification settings - Fork 4
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
Aesthetics #83
Aesthetics #83
Conversation
,"default": null | ||
,"type": "string" | ||
} | ||
,"grouping_direction": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll make sure to update the settings-schema after everything is merged. Good idea to do this before each new release. Addressed in #116.
@@ -26,8 +26,5 @@ export default function onPreprocess() { | |||
|
|||
//Insert groupings into data to draw empty rows in which to draw groupings. | |||
if (this.config.y.grouping) defineGroupingData.call(this); | |||
else { | |||
delete this.groupings; | |||
this.config.range_band = this.initialSettings.range_band; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need the range_band reset anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, just needed to move the y-domain definition to onPreprocess rather than onDraw. Or something. Definitely addressed in #114.
@@ -0,0 +1,9 @@ | |||
export default function IEsucks() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this triggers only in IE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeederino
grouping => grouping.value_col === this.config.y.grouping | ||
)[0].label}: ${d.key}` | ||
), | ||
rule = g |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think the rule is needed with the rest of the refactor. I'll take it out in a sec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually - I'm just going to make an issue. I'm getting formatting differences when I rebuild, and don't want to introduce those right now ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #49
Closes #54
Closes #66
Closes #82
Closes #87
Merge after #103 (date reference lines)