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

Conversation

Projects
None yet
6 participants
@akagomez
Contributor

akagomez commented Feb 16, 2015

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 from Complete the Sort Plugin to Complete the Sort plugin Feb 17, 2015

Chris Gomez
@asavoy

This comment has been minimized.

Show comment
Hide comment
@asavoy

asavoy Feb 19, 2015

Contributor

+1 This looks awesome

Contributor

asavoy commented Feb 19, 2015

+1 This looks awesome

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Feb 19, 2015

Contributor

👍

Contributor

imjoshdean commented Feb 19, 2015

👍

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Feb 19, 2015

Contributor

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

Contributor

matthewp commented Feb 19, 2015

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

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 19, 2015

Contributor

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.

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

This comment has been minimized.

Show comment
Hide comment
@marshallswain

marshallswain Feb 19, 2015

Member

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

Member

marshallswain commented Feb 19, 2015

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

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@daffl daffl deleted the sort-plugin branch Feb 19, 2015

@daffl daffl removed the fixed in branch label Feb 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment