Update QUnit to latest stable version, update tests for jQuery 3, and switch from PhantomJS to node-qunit-puppeteer in CI workflow - #823
Conversation
ElDeveloper
left a comment
ElDeveloper
left a comment
There was a problem hiding this comment.
@actapia This looks great to me. I appreciate all the effort you put into this PR.
- Would you mind expanding on what the
appendToargument means and why is it needed?
| * @param {String[]} uniqueVals Array of unique values for the category | ||
| * @param {Object} data SlickGrid formatted data from setCategory function | ||
| * | ||
| *, |
| // this.ready = undefined; | ||
| this.ready = null; |
There was a problem hiding this comment.
| // this.ready = undefined; | |
| this.ready = null; | |
| this.ready = null; |
| // setup the jquery properties of the button | ||
| scope.$optionsButton.button({text: false, | ||
| icons: {primary: ' ui-icon-gear'}}); | ||
| scope.$optionsButton.button({showLabel: false, |
There was a problem hiding this comment.
Just one note this isn't showing the gear icon anymore:
There was a problem hiding this comment.
The way that button icons are set was changed in the new version of jQuery UI, and it looks like I inadvertently included that change in both this pull request and #822 . I've changed it back to the old version for this pull request, so the icons should show up again.
| text: false, icons: {primary: ' ui-icon-circle-arrow-s'} | ||
| }).attr('title', 'Download Scree Plot'); | ||
| }).button( | ||
| {showLabel: false, icon: 'ui-icon-circle-arrow-s'} |
There was a problem hiding this comment.
Similar to the gear icon, this isn't showing the download icon in the axes tab:
| equal(director.trajectories[1].metadataCategoryName, 'Fast', 'The' + | ||
| assert.equal(director.mappingFileHeaders, | ||
| this.mappingFileHeaders, | ||
| 'The mapping file headers are set correctly'); |
There was a problem hiding this comment.
| 'The mapping file headers are set correctly'); | |
| 'The mapping file headers are set correctly'); |
| this.controllerProcrustes._$otherEdgeColor.spectrum('destroy'); | ||
|
|
||
| this.controller = undefined; | ||
| // if (this.controller !== undefined) { |
There was a problem hiding this comment.
Can you uncomment this block or if it is no longer needed can you delete this?
|
And, also thank you for your patience! |
Thank you. I added the |
|
Only quirk might be sorting out conflicts with the changelog, but I think I should be able to handle that - I'll keep you posted. After that we can get #822 merged as well. |
|
Thank you @actapia |
2 participants
These changes update QUnit to the latest stable version as of this writing (version 2.24) and update the QUnit tests accordingly. Together with #822 , this pull request addresses #820 . These commits also include changes to the QUnit tests to accommodate an upgrade to jQuery 3 (mainly changes to make parts of tests asynchronous).
Since PhantomJS does not work well with ES6 (used in jQuery 3), these changes also replace PhantomJS with node-qunit-puppeteer in the Emperor CI workflow.