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

Support descending columns in enumerable sortBy #13261

Closed
wants to merge 33 commits into from
Closed

Support descending columns in enumerable sortBy #13261

wants to merge 33 commits into from

Conversation

straydogstudio
Copy link

It would be useful to have intermediate colums be descending without creating computed properties.

Mukund Lakshman and others added 30 commits April 6, 2016 11:55
In browsers that support it, use `(new Error()).stack` instead of
try-catch. Closes #12266
[ci skip]

(cherry picked from commit 5f88a0a)
[ci skip]

(cherry picked from commit 4c5d000)
This adds more tests are custom ids and bindings. These cases seem to be
only tested by the `{{#view}}` helper and that is going away. To ensure
we at least are tracking this functionality I added some tests to the
curly-component integration tests.
`NativeArray.activate` was removed several months ago, but the documentation for the mixin still recommends that method for activating it. This change updates the docs to suggest the same code that is used internally by the `NativeArray` mixin to apply itself to `Array.prototype`.
Only warn when features are enabled that we know about. This prevents a
warning when users enable Ember Data features when running stable
versions of Ember with canary versions of Ember Data.
This is documented as though it is documenting Ember.get/set usage of CPs
but this particular method is very much internal. Also, remove
@constructor from the class description since this is not documenting the
constructor it is documenting the concept of CPs.
This allows for AST plugins and other options to be passed into the
glimmer compiler.
Also removed the old escape test file since it's now covered by the new
tests.
@straydogstudio
Copy link
Author

My rebase didn't quite work. Sigh. Closing and will reopen.

@mmun
Copy link
Member

mmun commented Apr 6, 2016

We want to avoid introduce microsyntaxes into more places (yes, even if it would be consistent with other APIs). We need a strategy to unify all 3 sorting methods with a new, backwards-compatible API. Check out this RFC comment: emberjs/rfcs#87 (comment). There are few syntax proposals in that thread.

BTW, this is a new feature so it would need an RFC & feature flags. You can grep around for isEnabled( to see some examples.

@mmun
Copy link
Member

mmun commented Apr 6, 2016

I was thinking something like

let { desc } = Ember.sort;
array.sortBy(desc('createdAt'), 'name')

@straydogstudio
Copy link
Author

Thanks @mmun. I was definitely trying to be consistent with the computable sortBy implementation, and I didn't notice the RFC. I'll try implementing the new backwards compatible api without microsyntaxes for computable and enumerable.

So the general consensus is to keep computable.sort and enumerable.sortBy, without changing either interface, but providing a better syntax for a descending column? While computable.sort is backwards compatible with the microsyntax?

@mmun
Copy link
Member

mmun commented Apr 6, 2016

@straydogstudio You got it. I think it's important to add a computed.sortBy that mirrors Enumerable#sortBy as part of the effort as well.

@straydogstudio
Copy link
Author

@mmun: What would be the difference between computable.sort and computable.sortBy?

@straydogstudio
Copy link
Author

@mmun: If we want equal interfaces, should enumerable.sortBy accept a function?

Also, this will create common code. Should I create it in enumerable and import that into computed? Create another mixin?

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.