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

Use a combobox instead of a lineedit widget for library search history #3171

Merged
merged 27 commits into from
Oct 29, 2020

Conversation

poelzi
Copy link
Contributor

@poelzi poelzi commented Oct 12, 2020

Use a combobox instead of a lineedit widget and utalize the combobox
selection mechanism for restoring and saving search queries.
This allows to go back to earlier queries easily and also
allows to restore the last search from a different feature.

Entries in the combobox are added after a 5 second timeout or
when the search is changed via api.

Properly query the frame width from the stylesheet and not assume 1.

The current data model is only in memory and not saved to disk.

selection mechanism for restoring and saving search queries.
This allows to go back to earlier queries easily and also
allows to restore the last search from a different feature.

Entries in the combobox are added after a 5 second timeout or
when the search is changed via api.

Properly query the frame width from the stylesheet and not assume 1.

The current data model is only in memory and not saved to disk.
@poelzi
Copy link
Contributor Author

poelzi commented Oct 12, 2020

Screenshot_20201013_004910

@Be-ing
Copy link
Contributor

Be-ing commented Oct 12, 2020

Saving these persistently could be added later after the basic feature is merged.

@Be-ing
Copy link
Contributor

Be-ing commented Oct 12, 2020

How about autocompletion?

@ronso0
Copy link
Member

ronso0 commented Oct 12, 2020

It was in a b2b session just a few days ago that I decided to take a stab on this because it is sooo annoying to retype queries like bpm:=122-128 or tech vocal.
thanks for working on this!

@ronso0

This comment has been minimized.

@ronso0
Copy link
Member

ronso0 commented Oct 13, 2020

at least with arrow buttons recalling previous queries feels a bit defective

  • search BOO, Enter
  • search FAR, Enter
  • search OOPS, Enter
  • clear search for a new querie
  • press down repeatedly: FAR, BOO show up
  • press up: BOO, FAR, OOPS

@ronso0
Copy link
Member

ronso0 commented Oct 13, 2020

We should remove line 105 setFocusPolicy(Qt::ClickFocus); in order to make the lineedit focusable again so keyboards and controllers can access the query history. (librarycontrol would need to be adjusted, too, I guess)

@ronso0
Copy link
Member

ronso0 commented Oct 13, 2020

After having selected any feature where searching is disabled, the placeholder is added to the query list
image

@ronso0
Copy link
Member

ronso0 commented Oct 13, 2020

Applying the library font is fixed with ronso0@0d491ad
It was both an issue in the cpp as well as a hard-coded font for QComboBox in LateNight.
Though, increasing the font makes the searchbox grow immediately while decreasing the font doesn't shrink the box until the text is changed. This is bareable IMO...

I can help with auto-resizing the down button later on if you like.

Update internal metrics to calculate button positions on font changes.

Move the new entry to the top of the list when an old entry was found.
This way the search behaves more like expected.
Avoid duplicates when Enter is pressed.
Save new query term if keydown is pressed or popup opened.
@poelzi
Copy link
Contributor Author

poelzi commented Oct 13, 2020

@ronso0 I fixed the resize problem and changed the history algorithm to move the old search entry to the top of the list and in general should behave more like expected.
@Be-ing autocompletion works out of the box with the default model/completer. This of course can be enhanced later on, like suggesting filter terms like "~key" etc. I could also imagine a dao for saving the queries in the database, but at this point, I'm ok with having this only at runtime.

Cancle timer when selection changes to prevent history being
messed up with arrow keys/controls.
@poelzi
Copy link
Contributor Author

poelzi commented Oct 13, 2020

I think this is ready for review. I did some more testing and tweaked the behavior and think it behaves now as one would expect.

@ronso0
Copy link
Member

ronso0 commented Oct 13, 2020

We should remove line 105 setFocusPolicy(Qt::ClickFocus); in order to make the lineedit focusable again so keyboards and controllers can access the query history.

...with Tab, Up, Down and existing library controls like [Library],MoveFocus, ..MoveVertical

With combobox reaching the search field is usefull again since it
allows you to scroll previous filters.
@poelzi
Copy link
Contributor Author

poelzi commented Oct 14, 2020

@ronso0 fixed

src/widget/wsearchlineedit.cpp Outdated Show resolved Hide resolved
src/widget/wsearchlineedit.h Outdated Show resolved Hide resolved
src/widget/wsearchlineedit.h Outdated Show resolved Hide resolved
src/controllers/controlpickermenu.cpp Outdated Show resolved Hide resolved
src/library/librarycontrol.cpp Outdated Show resolved Hide resolved
src/preferences/dialog/dlgpreflibrary.cpp Outdated Show resolved Hide resolved
src/widget/wsearchlineedit.cpp Show resolved Hide resolved
src/widget/wsearchlineedit.cpp Outdated Show resolved Hide resolved
src/widget/wsearchlineedit.cpp Outdated Show resolved Hide resolved
src/widget/wsearchlineedit.cpp Outdated Show resolved Hide resolved
…ueries

with [Library],MoveVertical, ..MoveDown & MoveUp controls.
@ronso0
Copy link
Member

ronso0 commented Oct 15, 2020

@poelzi
I was wrong, there's more necessary to allow scrolling through the search queries with [Library],Move...
check 118f4e5 (maybe this can be optimized, or I didn't cover all cases)

@ronso0
Copy link
Member

ronso0 commented Oct 16, 2020

...as well as 5797c14: [Library],GoToItem jumps from searchbox to tracks table (skips Clear button & sidebar)

have used this branch in a few sessions now and it's very handy. Thanks!

@Be-ing Be-ing changed the title Use a combobox instead of a lineedit widget and utalize the combobox Use a combobox instead of a lineedit widget for library search history Oct 27, 2020
@Be-ing
Copy link
Contributor

Be-ing commented Oct 27, 2020

@uklotzde anything left to do here?

@uklotzde
Copy link
Contributor

@uklotzde anything left to do here?

I will not re-review or test before the pre-commit issues have been fixed and the CI builds succeed.

@poelzi
Copy link
Contributor Author

poelzi commented Oct 29, 2020

@uklotzde it's green now :)

@ronso0
Copy link
Member

ronso0 commented Oct 29, 2020

nice!
I adjusted the drop-down styles in LateNight.
Do we want to add this here or in another PR?

@ronso0
Copy link
Member

ronso0 commented Oct 29, 2020

nice!
I adjusted the drop-down styles in LateNight.
Do we want to add this here or in another PR?

forget about it... I'll do this all at once later on.

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

The width of the search box (and sidebar) grows with the longest search string. Suddenly the sidebar takes half of the screen's width and cannot be shrunk back.

Expected behavior: The width of the sidebar is controlled by the user and does not depend on the contents of the search combo box.

@poelzi
Copy link
Contributor Author

poelzi commented Oct 29, 2020

@uklotzde good catch. fixed

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

Thanks for this powerful extension! Works really good now, especially in conjunction with the related tracks search.

I have only done a quick test, no keyboard navigation, no controller interaction.

@Be-ing Be-ing merged commit 524c41a into mixxxdj:main Oct 29, 2020
@ronso0
Copy link
Member

ronso0 commented Oct 29, 2020

🎉

@ronso0
Copy link
Member

ronso0 commented Oct 31, 2020

@poelzi Please document the new controls and shortcut(s) in the PR description.

@uklotzde
Copy link
Contributor

uklotzde commented Nov 3, 2020

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

Successfully merging this pull request may close these issues.

5 participants