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

Alias sortable direction to group #151

Merged
merged 1 commit into from
Oct 19, 2017

Conversation

rmachielse
Copy link
Contributor

@rmachielse rmachielse commented Aug 16, 2017

We are using this addon in an application where group is already a thing in many places.
Therefore we would like to use a different name for that variable.
With this change we only have to override _direction and _tellGroup instead of having to copy complete methods.

Let me know if this is desired or if you know a better way!

@rmachielse rmachielse force-pushed the feature/group branch 2 times, most recently from 47de4fb to 6e1d257 Compare August 16, 2017 12:27
@rmachielse
Copy link
Contributor Author

@jgwhite @acburdine any thoughts about this?

@acburdine
Copy link
Contributor

acburdine commented Aug 29, 2017

So I'm fine with this change for the 1.x branch I think - my idea with a 2.0 release (once I get around to cleaning some stuff up 😬) is to refactor how properties are passed between the two components, so instead of passing the entire group just the needed properties would be transferred.

@rmachielse
Copy link
Contributor Author

Ok, great! What way are you thinking of? Should I change the pr to a different branch?

@rmachielse
Copy link
Contributor Author

@acburdine can this be merged soon?

@rmachielse
Copy link
Contributor Author

@acburdine is there anything that is keeping this from being merged? Please let me know if you need anything from me

@jgwhite
Copy link
Contributor

jgwhite commented Oct 18, 2017

Sorry we haven’t moved forward on this. I hope to get up to speed on it soon and get back to you with the next step.

@jgwhite jgwhite changed the base branch from master to 1.x October 19, 2017 10:57
@jgwhite jgwhite merged commit 119d80b into adopted-ember-addons:1.x Oct 19, 2017
@jgwhite
Copy link
Contributor

jgwhite commented Oct 19, 2017

Thanks so much for the patch! I’ll try to release it later today!

@rmachielse
Copy link
Contributor Author

@jgwhite awesome, thanks a lot!

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