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: don't trigger search on each cursor keys press ... #11296

Closed
wants to merge 1 commit into from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Feb 22, 2023

instead, hook up to signals QCompleter::highlighted() and QLineEdit::selectionChanged() and accept the completer proposal if it's selection is changed (call slotTextChanged()).

This is an alternative to #11289

In my tests this works reliably because the signals are emitted like this:

  • type some text the completer can pick up
  • the completer appends the missing part and selectes it
  • the QLineEdit emits (textChanged) and selectionChanged
  • the QCompleter emits highlighted

instead, hook up to signals QCompleter::highlighted() and QComboBox::selectionChanged()
and 'accept' the completer proposal if it's selection is changed
@github-actions github-actions bot added the ui label Feb 22, 2023
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first approach was similar, but it turns out that this was not robust, because the completer is just temporary.

I did a brief test and it confirms the issue. The completion is not searched after the window has lost and get the focus again. It is also lost if you toggle the library to full screen. In keeping track of the completion state in an extra bool feels fragile and connecting to a signal is a locking call.

This can be fixed by using our own completer.

Sure that this are all corner cases, but we have a working alternative solution already: #11289

@ronso0
Copy link
Member Author

ronso0 commented Feb 23, 2023

The issues you mention are caused by refreshing the "Enable auto-complete" state on each FocusIn event because -for now- the preferences have no connection to WSearchLineEdit and set a static bool instead. This reliefs us of having to reload the skin when the setting is toggled. Resetting the completer could be done only on demand, if the static bool doesn't match the current completer (valid pointer or nullptr).

Anyways, I'm not going to fight for this fix here, but eventually I'll revive this branch.

Gonna take a look at #11289 soonish.

@ronso0 ronso0 closed this Feb 23, 2023
@ronso0 ronso0 deleted the search-completion-left-right branch March 9, 2023 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants