Skip to content
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

Enable custom footer event #468

Merged
merged 2 commits into from
Dec 19, 2017
Merged

Enable custom footer event #468

merged 2 commits into from
Dec 19, 2017

Conversation

billy-addepar
Copy link
Contributor

No description provided.

@billy-addepar billy-addepar force-pushed the billy/send-footer-event branch from b608b9a to d957c44 Compare December 18, 2017 20:03
Copy link
Contributor

@pzuraq pzuraq left a comment

Choose a reason for hiding this comment

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

@billy-addepar We should fix things upstream if we can.

Copy link
Contributor

@pzuraq pzuraq left a comment

Choose a reason for hiding this comment

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

Generally seems good, but let's only use the on style for tests only. Also, can you squash and make sure the commit message is descriptive?

</div>
`);
} else {
this.on('onTableFooterEvent', callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

This style should work all the way through modern Ember versions, we use this in vertical-collection

this.set('rows', generateRows(10, columnCount));
this.set('footerRows', footerRows);

if (Ember.VERSION >= '1.13') { // eslint-disable-line no-restricted-globals, no-undef
Copy link
Contributor

Choose a reason for hiding this comment

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

Ember.VERSION is a string. This comparison works right now because 1.13 was the last of the 1.* cycle, so all versions in 2.* are bigger, string-wise. It would fail when 2.13 >= 2.8 for instance. So, we should always use a semver library, or ember-compatibility-helpers

@billy-addepar billy-addepar force-pushed the billy/send-footer-event branch from 9f0d1f0 to d410e26 Compare December 19, 2017 00:57
Copy link
Contributor

@pzuraq pzuraq left a comment

Choose a reason for hiding this comment

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

Looks good!

@billy-addepar billy-addepar merged commit cfe0d3a into master Dec 19, 2017
@bantic bantic deleted the billy/send-footer-event branch July 11, 2019 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants