search

Haroenv / holmes

Fast and easy searching inside a page
https://haroen.me/holmes/
Apache License 2.0
1.7k stars 71 forks source link

fix onFound callback to be called when the very first element is visible #100

Closed alexhoma closed 5 years ago

alexhoma commented 5 years ago

Summary: onFound test was checking that the callback was called (but it was being called for every found item), so I've put a number of times that can be called when the input is changed, in order to fix the issue. :tada:

Related issue: https://github.com/Haroenv/holmes/issues/87

alexhoma commented 5 years ago

I've never use it. (Well, not since this weekend).The main reason was to contribute for #hacktoberfest. Since your reply in https://github.com/algolia/algoliasearch-client-javascript/pull/789 I checked your profile and found this pretty cool library :+1: I think it's good to practice open source just getting into the dephts of the libraries en understanding what they're doing and how :)

Haroenv commented 5 years ago

@alexhoma when trying this out in detail i can still see the onFound callback being called when someone does this:

something which hides everything '' (found) t (found) te (found)

Those last two should not call the callback I think

Haroenv commented 5 years ago

Thinking about it more, I fear some people might rely on this (broken) behaviour, because it has been like this for years. Would you mind if I revert your PR? I definitely still appreciate you looking into this

alexhoma commented 5 years ago

Regarding to the first comment, I thought this was the expected behavior :sweat_smile: But sure no problem! Makes sense that some people could see this as a bug instead of a fix. :+1:

Haroenv commented 5 years ago

Definitely still thanks for your contribution