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

Display search results as you type delay #3608

Closed
Mitia99 opened this issue Dec 16, 2023 · 5 comments · Fixed by #3612
Closed

Display search results as you type delay #3608

Mitia99 opened this issue Dec 16, 2023 · 5 comments · Fixed by #3612
Assignees
Milestone

Comments

@Mitia99
Copy link

Mitia99 commented Dec 16, 2023

Describe the bug
Search results aren't instantly displayed while typing in search bar after selecting a zim library

Expected behavior
Display search results as you type

Steps to reproduce the behavior:

  1. Select a zim library
  2. Click on search button
  3. Input text

Environment

  • Version of Kiwix Android : 3.9.0
  • OS version : Android 10
@kelson42
Copy link
Collaborator

@Mitia99 Thank you for your bug report. Here a few questions:

  • What is your smartphone model?
  • What is the ZIM file you test with?
  • How long do you wait to get the suggestion list in average?

@Mitia99
Copy link
Author

Mitia99 commented Dec 16, 2023

@Mitia99 Thank you for your bug report. Here a few questions:

  • What is your smartphone model?
  • What is the ZIM file you test with?
  • How long do you wait to get the suggestion list in average?

Thanks for your reply.

  • Device: Huawei P30
  • Zim: Encyclopédie médicale Wikimed

It's not a long wait but it was instant in 3.8.0 if I remember.
https://github.com/kiwix/kiwix-android/assets/56766778/5fd02710-5cd4-4290-bb43-1bc180b4097b

Thanks for investigating,

@Mitia99
Copy link
Author

Mitia99 commented Dec 16, 2023

@kelson42 kelson42 removed the question label Dec 16, 2023
@kelson42
Copy link
Collaborator

@Mitia99 Thank you for the details. I agree. I have tested myself and there is indeed a regression here in term of usability. We will work to fix this ASAP.
@MohitMaliFtechiz I feel kind of very frustrated here because (1) If I understand everything right I told you 3 weeks ago to not do that (2) Actually - always if I understand right - you seem to have even made the situation worse, moving from 300ms to 500ms (3) All of this when through testing without been noticed and we will probably have to make a new release just because of this. Glad if you explain me I'm wrong.

I made my homework and learned that - on mobile - the average type speed is around 3 characters a second. The current bouncing delay seems to wait 500ms before running a search (which I guess might take a bit as well, lets say 300ms). All of this seems to fit to my user experience: waiting around 1s to get the suggestions.

What will go wrong if we just get rid of this bouncing DEBOUNCE_DELAY?

@MohitMaliFtechiz
Copy link
Collaborator

MohitMaliFtechiz commented Dec 18, 2023

(1) If I understand everything right I told you #3558 (comment) (2) Actually - always if I understand right - you seem to have even made the situation worse, moving from 300ms to 500ms (3) All of this when through testing without been noticed and we will probably have to make a new release just because of this. Glad if you explain to me I'm wrong.

@kelson42 Initially we introduced the debouncing delay with 300L but at that time it was crashing if I frequently/slowly tried them for search results since at that time we were using the estimateMatches method of libkiwix and it was crashing that time, I have deeply tested this functionality at that time with many scenarios and most of the scenarios were crashing with 300L dealy. That's why that time we increased the debouncing with 500L.

However, in #3592 we notice that estimateMatches is redundant for getting the results that was the main reason in most of the crashing scenarios in #3558, and in this PR we also fixed the unnecessary memory allocation and data load on libkiwix.

Since for #3592, I was not able to reproduce the error on my all devices, so I fixed it by improving the coroutines and memory allocation issues, at that time I did not remove the debouncing from code to avoid any other regression.

What will go wrong if we just get rid of this bouncing DEBOUNCE_DELAY?

Now we have built a system where we only get results once from libkiwix, and avoid all the unnecessary memory allocation, and you already tested it without debouncing on the fairphone, on this device it was crashing, so we can get rid of this. Most of the things regarding this you and @gouri-panda already discussed so we can remove this.

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