search

nytimes / backbone.stickit

Backbone data binding, model binding plugin. The real logic-less templates.
MIT License
1.64k stars 176 forks source link

Performance improvements with documentFragment in addSelectOptions #307

Closed pwolaq closed 8 years ago

pwolaq commented 8 years ago

Added documentFragment so that adding many option elements doesn't downgrade performance by unnecessary rendering.

pwolaq commented 8 years ago

@akre54 do you know why this build is failing?

akre54 commented 8 years ago

Looks like a phantomjs issue? https://travis-ci.org/NYTimes/backbone.stickit/jobs/158697390

Otherwise no clue... sorry.

pwolaq commented 8 years ago

I don't think that my changes are causing this error, it's probably server related

pwolaq commented 8 years ago

@akre54 can you merge without build check?

akre54 commented 8 years ago

Yup. Are you sure this works correctly? Can you run tests locally?

pwolaq commented 8 years ago

it is simple change and it is working in my project

I cannot test it because I get the same error locally - seems like there is install.js missing

akre54 commented 8 years ago

Yeah sorry about that, our test runner was crazy out of date (phantomjs has a ton of problems). I've updated our test harness to use Karma. Can you pull down latest and try now?

Thanks

pwolaq commented 8 years ago

unfortunately, same results

akre54 commented 8 years ago

You might need to run npm install phantomjs one or two times. That worked for me. (Sorry Phantom is such a pain).

akre54 commented 8 years ago

I'll figure out what the deal is with Travis at a later time, but I just ran your branch against the test runner and everything seems to be good. Thanks again.