Skip to content

Conversation

@BrianSipple
Copy link
Collaborator

@BrianSipple BrianSipple commented Oct 14, 2016

Ever since Ember CLI 2.8 was released, I've noticed that our project (currently on 2.7) has been failing in CI for both "beta" (2.9) and "release" configurations. (Side note: ember-try is amazing.)

This can be seen across the builds for the last few PRs (see: the four previous builds from this point).

To remedy this, I updated the project to 2.8, ran ember-init to resolve any discrepancies, and went about fixing things. Outside of some minor syntax changes, the main changes regarding functionality...

So far, so green. I'll update and rebase #111 and #110 if all goes well here.

@BrianSipple BrianSipple force-pushed the update-version-and-fix-tests branch from b228ec2 to 7848583 Compare October 14, 2016 10:54
Copy link
Owner

@cball cball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!


// this currently includes 1 px border from the appearance css. not ideal.
assert.equal(this.$('.fixed-table-columns-wrapper').width(), 151);
assert.equal(parseInt(window.getComputedStyle(columnsElem).width), 151);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on moving to this format. I imagine anywhere we get the width should move to this style, but baby steps. 😄

*/
export default function getNode(context = this) {
return context.$()[0].firstElementChild;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

let rows = $('.table-columns tr').length;
let headers = $('.table-columns th').text().trim();
let rows = Ember.$('.table-columns tr').length;
let headers = Ember.$('.table-columns th').text().trim();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on changing import jQuery from 'jquery' above vs updating this to Ember.$? If you're thinking it's more clear that Ember.$ does not reference the window scope, I'm good with that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All for it! I'll make that change real quick.

@BrianSipple BrianSipple force-pushed the update-version-and-fix-tests branch from 7848583 to dcf282a Compare November 5, 2016 00:51
@BrianSipple
Copy link
Collaborator Author

@cball just updated and rebase around your import jQuery suggestion. Should be good to go!

@cball cball merged commit 745aa8d into master Nov 8, 2016
@cball cball removed the in progress label Nov 8, 2016
@cball cball deleted the update-version-and-fix-tests branch November 8, 2016 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants