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.

Sign up for GitHub

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

Jump to bottom

[ENH] Combo Box Search Filter #3014

Merged
merged 17 commits into from
Aug 10, 2018
Merged

[ENH] Combo Box Search Filter #3014

merged 17 commits into from
Aug 10, 2018

Conversation

ales-erjavec
Copy link
Contributor

Issue

Implements gh-2972

Description of changes
  • Implement a QComboBox, such that the displayed popup list view implements text search.
  • Use it in 'Hierarchical Clustering' widget as an example.
Includes
  • Code changes
  • Tests
  • Documentation

@BlazZupan
Copy link
Contributor

Great prototype, works fast! Thanks.

On mouse over the fields, the selection (on mac, the coloring of the line indicating current position in the list) does not change. Following is an example of how this works for selection of linkage:

a

and how the line for Zzz3 is not marked with blue for selection of annotation:

b

Filter looks like an text entry box. When I first open the menu, I wanted to click somewhere inside the box to activate it and have the caret show, but action closes the pull down menu. I understand that the problem is with deciding which Qt widget is in focus. @markotoplak, @astaric, can you look at this and comment.

@codecov-io
Copy link

codecov-io commented Apr 23, 2018 •
edited
Loading

Codecov Report

Merging #3014 into master will increase coverage by 0.06%.
The diff coverage is 92.71%.

@@            Coverage Diff             @@
##           master    #3014      +/-   ##
==========================================
+ Coverage   82.64%   82.71%   +0.06%     
==========================================
  Files         342      342              
  Lines       59016    59062      +46     
==========================================
+ Hits        48774    48851      +77     
+ Misses      10242    10211      -31

@ales-erjavec ales-erjavec force-pushed the combobox-search-filter branch 2 times, most recently from 7445f41 to 0385d51 Compare April 26, 2018 07:30
@ales-erjavec ales-erjavec force-pushed the combobox-search-filter branch from 0385d51 to 3bdbec5 Compare May 11, 2018 14:17
@ales-erjavec ales-erjavec force-pushed the combobox-search-filter branch from 3bdbec5 to ea1f83b Compare May 24, 2018 13:20
@ajdapretnar
Copy link
Contributor

This works really well for me. Something we could merge soon?

@BlazZupan BlazZupan self-assigned this Jul 6, 2018
@ales-erjavec ales-erjavec changed the title [RFC][ENH] Combo Box Search Filter [ENH] Combo Box Search Filter Jul 6, 2018
@ales-erjavec ales-erjavec force-pushed the combobox-search-filter branch from 34c5fa3 to daec31b Compare July 6, 2018 11:52
@thocevar thocevar self-requested a review August 3, 2018 07:43
thocevar
thocevar approved these changes Aug 3, 2018
View reviewed changes
Copy link
Contributor

@thocevar thocevar left a comment

Choose a reason for hiding this comment

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

Works great for me as well. We have to start using this component in other widgets to discover and fix any hidden bugs.

Orange/widgets/utils/combobox.py Outdated

Behaves like a 'windows' styled combo box (i.e.
`styleHint(SH_ComboBoxPopup)` is False, but
the
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this should continue.

Orange/widgets/utils/combobox.py Outdated
return False
elif key in (Qt.Key_End, Qt.Key_Home) \
and not modifiers & Qt.ControlModifier:
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to have home/end key jump around on the search line when editing the query.

@lanzagar
Copy link
Contributor

lanzagar commented Aug 3, 2018

It is indeed great.
I suggest we merge this right after the current release and start adding it to widgets so that the next release has a batch of new combo boxes.
Both of @thocevar's comments make sense, I would also like to see home/end move the cursor where I am typing. But tweaks can also be made later, this has been open for too long already, so I don't mind merging even as it is now.

@lanzagar lanzagar added this to the 3.16 milestone Aug 3, 2018
@ales-erjavec
ComboBoxSearch: Add a new combo box control with filter/search
f7b92d4
@ales-erjavec
ComboBoxSearch: Add mouse release ignore timer
01af59e
@ales-erjavec
ComboBoxSearch: Draw separators
292c73d
@ales-erjavec
owhierarchicalclustering: Use ComboSearchBox for 'Annotations'
676b81d
@ales-erjavec
ComboBoxSearch: Improve focus tracking/proxying
e1e38ae
@ales-erjavec
ComboBoxSearch: Implement popup list mouse tracking
4640e15
@ales-erjavec
ComboBoxSearch: Use available desktop geometry, not the full screen
335758b
@ales-erjavec
ComboBoxSearch: Extract dropdown_popup_geometry
f843270
@ales-erjavec
ComboBoxSearch: Set focus policy
09619cd
@ales-erjavec
ComboBoxSearch: Add tests
9d8e511
@ales-erjavec
ComboBoxSearch: Do nothing on showPopup calles if popup already visible
56c3f7e
@ales-erjavec
ComboBoxSearch: Remove use of WA_DoNotShowOnScreen
2308c37 
Segfaults on headless xcb.
@ales-erjavec
ComboBoxSearch: Change test_click test
10c45b7 
Could segfault if the tested popup loses focus (is hidden and deleted)  due to external
factors.

It is now run without any delay in the mouse event dispatch.
@ales-erjavec
ComboBoxSearch: Fix popup hiding on Windows
b9acb4c 
The popup does not appear to hide automatically on windows when a mouse
button is pressed outside the popup window.
@ales-erjavec
ComboBoxSearch: Remove event filters from popup widget on hide
e0ebb58
@ales-erjavec
ComboBoxSearch: Improve event dispatch
5e41859 
Remove special casing Home/End key filter, dispatch directly to
the line edit with no event propagation up the parent chain.
@ales-erjavec ales-erjavec force-pushed the combobox-search-filter branch from daec31b to 8b3e8a1 Compare August 9, 2018 11:24
@ales-erjavec
Copy link
Contributor Author

Can you please test Home/End keys again to ensure they correspond to standard platform behavior.

@thocevar
Copy link
Contributor

Home/End keys work on windows.

@lanzagar
Copy link
Contributor

Works on linux too. Looks good to merge.

@lanzagar lanzagar merged commit fdac69b into biolab:master Aug 10, 2018
@ales-erjavec ales-erjavec deleted the combobox-search-filter branch September 12, 2018 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thocevar thocevar thocevar approved these changes

Assignees

@BlazZupan BlazZupan

Labels
None yet
Projects
None yet
Milestone
3.16
Development

Successfully merging this pull request may close these issues.
6 participants
@ales-erjavec @BlazZupan @codecov-io @ajdapretnar @lanzagar @thocevar