Fix for incorrect limit with remote #52
Conversation
|
Simple enough change. Thanks @DanielApt. Merge it, merge it! |
|
Merge, please |
|
@sheeley Could this be merged? |
|
@jharding, @vskarich, @timtrueman, I've got a project that's waiting on this fix. Without it, the Limit feature is totally broken. Considering this is "corejavascript" I'm pretty concerned to see this PR languish. Can we get a merge and a release, please? I don't know about other people, but this is a major bug for my organization. |
|
@mlissner: I haven't been keeping up with the releases, but AFAIK the other folks (who actually started this fork) have a couple of outstanding issues they want to solve before creating corejs releases of this fork: #9. Tagging folks like @jharding who aren't involved here won't really help, unfortunately, and that won't update the original typeahead package regardless. @corejavascript/collaborators as I was added after you'd already started this, I don't feel comfortable speaking about the plans/etc. Can you help respond to @mlissner's concerns? |
| rendered += suggestions.length; | ||
|
|
||
| that.async && that.trigger('asyncReceived', query); | ||
| that.cancel = $.noop; |
There was a problem hiding this comment.
btw fixing these whitespace diffs would be appreciated 👍
|
@mlissner There's nothing stopping you from monkey-patching or even committing in your organization's version. Truth be told, at my organization our typeaheadjs library is fairly hacked up (not by me). Sucks but it's life in the open-source world... We don't have end-to-end tests running right now, so it would be helpful if people would identify if they have actually tested the code by running it. If we have 3 people who can say they've done that, it would allay concerns. Another idea is to do a screencast showing the bug and then showing how it operates with the fix. |
Also thanks to @mlissner for his contribution
|
@sheeley I've fixed the whitespace diffs. In the future it might be useful to add an editorconfig file, which should make most code editors use the same whitespace rules. |
|
@jcrben we have been running this PR in production since last week, as well as the test suite in our CI, without any problems |
|
@jcrben We have released this feature to our beta users and they aren't reporting any issues. It's also on Twitter's typeahead.js issue tracker where several people say it's working properly: twitter/typeahead.js#1435 |
Fix for incorrect limit with remote
|
If some of you want to be committers, let us know so we can ping the owner. I'm juggling too many things right now to spend too much time on it... |
|
Thanks @jcrben! |
|
Tracking this for release soon. See #55 |
Hey guys, here's a fix for #43. We've continued @mlissner's great work, and fixed the tests.