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

Search: Typing lag when entering queries in the search toolbar #1995

Closed
agramner opened this issue Feb 1, 2022 · 29 comments
Closed

Search: Typing lag when entering queries in the search toolbar #1995

agramner opened this issue Feb 1, 2022 · 29 comments
Assignees
Labels
bug Something isn't working easy Easy issue for beginners frontend Requires experience with HTML/JS/CSS released Available in the stable release ux Impacts User Experience

Comments

@agramner
Copy link

agramner commented Feb 1, 2022

What does not work as described in the documentation?
This is not a major problem, just a bit annoying.

On the People-page, when there are many people in the list, typing in the search box is slow. It's fast when there are few people, for example after searching on a first name to narrow down the result. Tried both Firefox and Chrome on OSX, Windows and Android.

I currently have 320 people in the list and all the people are loaded when opening the People-page

How can we reproduce it?
Steps to reproduce the behavior:

  1. Go to 'People' page
  2. Click in the search input box
  3. Start typing

What behavior do you expect?
I think typing should be super fast no matter how many people there are in the list because "nothing" is done until the user presses Enter key

What could be the cause of your problem?
I don't know Vue but can it have something to do with the how the "v-model" on the search box works? Something seems to cause a lot of computation on text input.

Can you provide us with example files for testing, error logs, or screenshots?
Nothing is written to the Console log of the browser

Which software versions do you use?

  • PhotoPrism Architecture & Build Number (AMD64, ARM64, ARMv7,...): AMD64, 220121-2b4c8e1f-Linux-x86_64
  • Database Type & Version (MariaDB, MySQL, SQLite,...): MySQL
  • Operating System Types & Versions (Linux, Windows, Android,...): OSX
  • Browser Types & Versions (Firefox, Chrome, Safari on iPhone,...): Firefox, Chrome, Safari...

On what kind of device is PhotoPrism installed?

  • Device / Processor Type (Raspberry Pi 4, Intel Core i7-3770, AMD Ryzen 7 3800X,...): Intel Core i5-8400T
  • Physical Memory & Swap Space (in GB): 8 GB, Swap 1 GB
  • Storage Type (HDD, SSD, RAID, USB, Network Storage,...): NVME SSD
@agramner agramner added the bug Something isn't working label Feb 1, 2022
@lastzero
Copy link
Member

lastzero commented Feb 1, 2022

Yeah, so 320 people is definitely much more than this was designed for. The list of people is searched via JavaScript and updated while you type. I would assume that facial recognition is also getting slow with that many people/faces.

@agramner
Copy link
Author

agramner commented Feb 1, 2022

I understand. So it's suppose to update while typing? It doesn't, it only updates when pressing enter for me

@lastzero
Copy link
Member

lastzero commented Feb 1, 2022

Yes, it's supposed to work like that - of course it could be optimized, but we don't have a lot of resources. Pull request welcome!

@agramner
Copy link
Author

agramner commented Feb 2, 2022

Ok, maybe I'll look into it when I have some time over

@agramner
Copy link
Author

agramner commented Feb 3, 2022

Investigated this and seems like the 2-way-binding (v-model) seem to cause a lot of components to be re-rendered each time you type a letter in the search box. Tried to change v-model to use only @input and that fixed to problem. Don't know Vue (I work with React), how do you avoid "everything" to be re-rendered each time some data change?

@lastzero lastzero self-assigned this Mar 6, 2022
@lastzero lastzero added the ux Impacts User Experience label Mar 6, 2022
@lastzero lastzero removed their assignment Mar 6, 2022
@lastzero lastzero added the help wanted Well suited for external contributors! label Mar 6, 2022
@ppprism
Copy link
Contributor

ppprism commented Mar 12, 2022

Just an addition: same holds true for searching in the 'Albums' section (and probably others like 'Labels').

@lastzero lastzero self-assigned this Mar 23, 2022
@lastzero lastzero added the please-test Ready for acceptance test label Mar 23, 2022
lastzero added a commit that referenced this issue Mar 23, 2022
@lastzero
Copy link
Member

We'll probably rework the forms and search when we upgrade to Vuetify 3, so I've only fixed the specific UX issues. Started a new Development Preview build for testing! 👍

@lastzero lastzero removed the help wanted Well suited for external contributors! label Mar 23, 2022
@lastzero lastzero changed the title People: Typing in the search box on the people page is slow UX: Faster and more intuitive handling of search query changes Mar 23, 2022
@agramner
Copy link
Author

Sounds great, nice work! Will try in out later tonight

@agramner
Copy link
Author

agramner commented Mar 23, 2022

Sorry to say, but it didn't seem to fix the issue 😩. Just tried the preview image.

@lastzero
Copy link
Member

What version / build specifically? See footer of Settings page.

@agramner
Copy link
Author

It's Build 220323-45b56313-Linux-AMD64

@lastzero
Copy link
Member

From the VueJS docs at https://v2.vuejs.org/v2/guide/forms.html?redirect=true#lazy)html?redirect=true#lazy:

By default, v-model syncs the input with the data after each input event (with the exception of IME composition, as stated above). You can add the lazy modifier to instead sync after change events:

<!-- synced after "change" instead of "input" -->
<input v-model.lazy="msg">

@lastzero
Copy link
Member

As it turns out, "lazy" is not so lazy.... every time the filter object changes, the entire page is re-rendered. A new property has been added that is not directly referenced in the DOM, so rendering can at least be delayed until the user is done typing. Should consider developing critical UI components with vanilla JS in the future.

@lastzero
Copy link
Member

A new Development Preview build has been pushed to Docker Hub. Albums, Labels, and the Library > Errors page have been reworked along with People > Recognized.... hope this works for you!

@agramner
Copy link
Author

This sure fixed it, nice! :)

@agramner
Copy link
Author

The behavior has changed a bit though. The search is "executed" when the search box loses focus. Not on submit (enter-key) as before.

@lastzero
Copy link
Member

Yes, that seemed more intuitive when I tested it. Should we always wait until the user presses the enter key?

@agramner
Copy link
Author

I don't have a very strong opinion but for me it feel most intuitive to either filter while typing or filter on submit (enter).

@lastzero
Copy link
Member

While typing would be possible, but then the user interface lags, which was the original problem...

lastzero added a commit that referenced this issue Mar 25, 2022
As discussed/requested in the issue comments.
@lastzero
Copy link
Member

@agramner It seems reasonable not to change the current behavior in order to stay consistent, so the automatic update of search results on blur/change has been disabled. A new preview build will be available soon. I'd appreciate if you could test it again!

@lastzero lastzero changed the title UX: Faster and more intuitive handling of search query changes UX: Typing lag when entering queries in the search toolbar Mar 25, 2022
@agramner
Copy link
Author

Yes it works as before now 👍

@lastzero lastzero changed the title UX: Typing lag when entering queries in the search toolbar Search: Typing lag when entering queries in the search toolbar Mar 31, 2022
@lastzero lastzero removed the please-test Ready for acceptance test label Apr 16, 2022
@lastzero
Copy link
Member

Fix needs to be reworked. The new implementation does not retain the query when the initial value is set by navigating to the search page and you then - for example - change the view type in the toolbar.

@lastzero lastzero added help wanted Well suited for external contributors! frontend Requires experience with HTML/JS/CSS labels Apr 16, 2022
@lastzero lastzero added please-test Ready for acceptance test and removed help wanted Well suited for external contributors! labels Apr 16, 2022
@lastzero
Copy link
Member

Would be great if you (and everyone else) could test it again! 🙏

@lastzero lastzero added help wanted Well suited for external contributors! easy Easy issue for beginners labels Apr 16, 2022
@agramner
Copy link
Author

I can test it but I don't really understand what the problem was. Can you elaborate? 🙂

@lastzero
Copy link
Member

In short, the problem was a workaround solution instead of passing changes to the parent component using a callback, which then updates the search field value. This is important as Vue JS reuses component instances if you navigate to a different page that is based on the same components. This got the field and the actual search query out of sync previously.

lastzero added a commit that referenced this issue Apr 18, 2022
Search results don't update automatically otherwise.
@agramner
Copy link
Author

Ok, I understand. I can verify that the search query persists while changing between different view types on the search page.

@lastzero
Copy link
Member

The text field wasn't working at all before I fixed it this morning. It was connected to the wrong event handler, which I hadn't noticed before. So every little bit of testing helps! 🐣

@dotfortun3
Copy link

If this is still being working on, could a potential fix could be adding a small delay before updating between only update the view if say 500/1000ms has passed since the last keystroke?

Not sure if this is a solution you would consider, but it is something I have done before to get the results to update as you type, but not lag with large datasets.

I could also look into adding this as a PR, but I don't want to do that if a solution has been completed

@lastzero
Copy link
Member

I think searching on enter / submit is OK, so you are control and don't have to type faster just to avoid premature search requests.

@graciousgrey graciousgrey added released Available in the stable release and removed help wanted Well suited for external contributors! please-test Ready for acceptance test labels May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working easy Easy issue for beginners frontend Requires experience with HTML/JS/CSS released Available in the stable release ux Impacts User Experience
Projects
Status: Release 🌈
Development

No branches or pull requests

5 participants