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

Complete the Sort plugin #1454

Merged
merged 1 commit into from Feb 19, 2015
Merged

Complete the Sort plugin #1454

merged 1 commit into from Feb 19, 2015

Conversation

akagomez
Copy link
Contributor

As far as I could tell the Sort plugin was largely incomplete.

After a lot of work it now...

  • Two-way binds a can.List's sort order to the DOM
  • Makes the fewest amount of DOM manipulations possible when rendered with {{#each}}
  • Ensures that operations like push, unshift, and splice add items at the correct index via can.List._getInsertIndex() (for efficiency there's no need to append then move an item)
  • Operations that result in an item being moved from one index to another result in a move event (this used to result in a remove and add event)
  • DOM manipulations made on behalf of the {{#each}} helper move the underlying DOM nodes rather than removing and re-creating them (this is much faster).
  • Resolves index changes to individual items with can.List._getInsertIndex as opposed to a complete can.List.sort operation
  • Changes to items in the can.List that have a batchNum result in a complete can.List.sort operation rather than (potentially) lots of far less efficient can.List._getInsertIndex operations
  • Breaks from can.List.sort operations when a partial sort is all that's needed to resolve the entire can.List's sort order

Future goals:

  • Rough out documentation
  • Choose a more efficient sorting strategy when there are no {{#each}} dependencies
  • Benchmark this implementation of the Sort plugin against https://github.com/Matt-Esch/virtual-dom

Closes #1404, #1114, #1265, #828

@akagomez akagomez changed the title Complete the Sort Plugin Complete the Sort plugin Feb 17, 2015
@asavoy
Copy link
Contributor

asavoy commented Feb 19, 2015

+1 This looks awesome

@imjoshdean
Copy link
Contributor

👍

@matthewp
Copy link
Contributor

This might need to be redirect to minor. @daffl is that right?

@daffl
Copy link
Contributor

daffl commented Feb 19, 2015

No. Master is up to date and ready for 2.2. Ideally things go against master now but it doesn't really matter since I need to merge minor in again anyway.

@marshallswain
Copy link
Member

If you can sneak it into master, that would be awesome.

@daffl daffl added this to the 2.2.0 milestone Feb 19, 2015
daffl added a commit that referenced this pull request Feb 19, 2015
@daffl daffl merged commit 801bfb5 into master Feb 19, 2015
@daffl daffl deleted the sort-plugin branch February 19, 2015 22:01
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.

can.List.prototype.sortIndexes is defined but not referenced
6 participants