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

Custom selector and fix for change event #18

Merged
merged 4 commits into from
Nov 3, 2013
Merged

Custom selector and fix for change event #18

merged 4 commits into from
Nov 3, 2013

Conversation

claviska
Copy link
Contributor

859a7ae fixes your todo item at the bottom. It shouldn't affect speed since nothing in the loop changed.

01f76c5 adds a custom selector option, similar to what was suggested in #6.

I've modified the code for clarity and tested it on a list of about 2,100 items. Compared to the original version, there appears to be no noticeable difference when a selector is not specified (i.e. the default, backwards-compatible behavior). As expected, however, there is a nominal performance hit when using a selector. (I've noted this in the readme for prospective users.)

I believe this will be useful for a lot of people. My use case is something like this:

<ul>
    <li>
        <span class="badge">Badge Text</span>
        <span class="description">Description</span>
    </li>
</ul>

By default, the filter will compare against Badge TextDescription, which is undesirable. But when .description is used as a selector, the plugin compares only against Description, which is the desired behavior.

7ad6fcd removes the use of addback(), which was benign but unnecessary and would have made the plugin incompatible with jQuery < 1.8.

@claviska
Copy link
Contributor Author

claviska commented Nov 3, 2013

Is this plugin still being maintained?

@awbush
Copy link
Owner

awbush commented Nov 3, 2013

Sorry, been a bit busy. This change looks fine to me, I'll merge it in.

awbush added a commit that referenced this pull request Nov 3, 2013
Custom selector and fix for change event
@awbush awbush merged commit 5783243 into awbush:master Nov 3, 2013
@claviska
Copy link
Contributor Author

claviska commented Nov 4, 2013

Thanks :)

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.

2 participants