-
Notifications
You must be signed in to change notification settings - Fork 1
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
Query Overview v2.1.1 #148
Conversation
@pburnsdata , in retrospect it was a terrible idea to add unit tests. Took forever, required importing every d3 function explicitly, and was extremely frustrating. You can ignore the explicit import file changes for the most part. |
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.
Looks good! Sorry that the tests got wild - definitely a lot of replaced functions here haha
@@ -25,5 +25,5 @@ export default function onInit() { | |||
removeInvalidControls.call(this); | |||
|
|||
//Initialize listing. | |||
this.listing.init(this.raw_data); | |||
this.listing.init(this.raw_data, this.listing.test); |
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.
@samussiah whats going on with this guy?
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.
webcharts has logic that expects the containing element to have been rendered before it proceeds. but the headless browser we use for testing doesn't actually render the page so there's some logic in init
that avoids the rendered logic when running unit tests.
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.
gotcha gotcha - makes sense
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.
Approved. DESTROY method works.
There actually seems to be an IE issue with destroy. Spencer is looking at it. |
Destroy issue in IE is fixed, all good. |
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.
approved
Issues
Closes #147