Bug 74795 - [Viewers] Generic quick filter for large tree-based views
Summary: [Viewers] Generic quick filter for large tree-based views
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: PC All
: P4 enhancement (vote)
Target Milestone: 3.2 M5   Edit
Assignee: Karice McIntyre CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
: 78972 98826 101614 117489 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-09-23 12:10 EDT by Eugene Kuleshov CLA
Modified: 2006-07-24 16:14 EDT (History)
13 users (show)

See Also:


Attachments
"Look for" filter combo in Microsoft Outlook (15.22 KB, image/gif)
2005-12-30 09:01 EST, Mik Kersten CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Kuleshov CLA 2004-09-23 12:10:53 EDT
In general it is difficult to use some Eclipse views that have a large tree of
components. That includes Navigator, Package Explorer, CVS Repositories and
probably some other views. It would be nice to be able to have entry field at
the top of these views that will allow to specify quick filter (like
*/proj*/**/*) to hide unmatching nodes. Actually similar UI work very well for
Ctrl-O/Ctrl-T pop-ups in Java editor.

Perhaps this functionality could be implemented on a tree widget level.
Comment 1 Tod Creasey CLA 2004-09-24 09:05:31 EDT
If you have a suggested implementation it would be very useful if you attached 
it here. I am not sure if we would package it with Eclipse but it wuld be a 
nice thing to make generally available.
Comment 2 Eugene Kuleshov CLA 2004-09-24 10:52:54 EDT
I can probably made up some mock screenshots. General idea would be the same as
for new popup search toolbar on Mozilla FireFox 1.0.
Comment 3 Eugene Kuleshov CLA 2005-02-16 12:13:07 EST
Tod, I noticed that similar work is being done for tree widged in all properties
dialogs. Is there are any plans to make it generic or anyhom make it easier for
other view to provide similar functionality?
Comment 4 Tod Creasey CLA 2005-02-16 12:19:18 EST
This is the FilteredTree which is currently internal. If you would like it 
promoted to API please vote on this bug.
Comment 5 Eugene Kuleshov CLA 2005-02-16 12:32:39 EST
Thanks Tod. By the way, is there a feature request for FilteredTree? I'd like to
suggest some enhancements for a filter definition.
Comment 6 Tod Creasey CLA 2005-02-16 13:37:35 EST
I think this PR pretty much covers it Eugene. Please let us know any 
suggestions and if you have some suggested code / improvments feel free to 
post it here.
Comment 7 Eugene Kuleshov CLA 2005-02-16 13:56:25 EST
My major concern about filter implementation in properties tree is that it does
not allow to specify patterns and also does not remeber history of entered
filters (like history drop-down). Besides that I believe it would be pretty much
it for UI part.

There are several tricky parts from a backend point of view for generic
FilteredTree. There is number of views in Eclipse that are loads trees lazily.
One of such view is CVS Repositories and it is the most one that need such
filtering. We had some discussion about it in bug 45936. So, concrete view may
need to contribute some info to filter, e.g. how to behave for lazily loaded
nodes, should filter automatically force tree load up to specified depth (in
case of CVS view it could be down to module level, or depth could be part of
filter expression).
Comment 8 Gunnar Wagenknecht CLA 2005-11-05 03:11:46 EST
(In reply to comment #4)
> This is the FilteredTree which is currently internal. If you would like it 
> promoted to API please vote on this bug.

Sould the summary on this bug changed to "Make FilteredTree API" or does this
bug cover additional items?

CCing Karice as she knows about remaining issues in FilteredTree that might
block this from getting API. Karice, are there plans to get FilteredTree into
the 3.2 API?
Comment 9 Karice McIntyre CLA 2005-11-07 10:41:33 EST
Making the filtered tree API has been listed on the M4 milestone plan as 
something the UI team should look into.
Comment 10 Boris Bokowski CLA 2005-11-09 15:54:12 EST
*** Bug 98826 has been marked as a duplicate of this bug. ***
Comment 11 Boris Bokowski CLA 2005-11-09 15:56:32 EST
Relevant comments copied from bug :

Comment #1 From Tod Creasey  2005-06-07 15:58

What is wrong with the FilteredTree?


Comment #2 From Kim Horne 2005-06-07 16:26

Well, currently it's internal and it takes up a lot of space... perhaps if the
search combo could be collapsed somehow.  Also, the behaviour on very large
trees would need to be negotiated - as it is, it expands the entire model
internally.

Comment 12 Boris Bokowski CLA 2005-11-09 15:57:05 EST
bug 98826 of course...
Comment 13 Karice McIntyre CLA 2005-11-09 16:28:13 EST
Some things to consider if filtered tree is to become API:

1. RCP - some rcp apps don't always need the filtered tree to appear (this 
should be done whether this becomes API or not).  see bug 107271.

2. performance - perhaps we could employ an indexing scheme to prevent 
performance slow down for large trees (i.e. a navigator with hundreds of 
projects, thousands of files).

3. real estate for the filtered tree widgets - apparently the Mac demonstrates 
an eloquent way of dealing with this problem.
Comment 14 Karice McIntyre CLA 2005-11-24 13:04:20 EST
*** Bug 117489 has been marked as a duplicate of this bug. ***
Comment 15 Mik Kersten CLA 2005-12-30 09:00:52 EST
Mylar has been using the FilteredTree for it's task list view since mid summer, and it has been indespensible (uses a tree with columns, see http://eclipse.org/mylar/doc/new.php for some screenshots).  The history combo has been requested by users and would be great.  Here are a couple more things that would be helpful:

- Peformance: overall prefromance is good even with very large task lists.  But when there are hundreds of elements in the model, and the user types in a term like "hello", after typing the "h" a good portion of the elements in the model will show causing very large expansion and delay in the UI before the "e" shows up, filters, then the "l", and so on.  This could be addressed by introducing a short delay (< 1s) to wait for the user to finish typing the sequence of characters before starting the filter.  That would probably be better than requiring "enter" to be pressed, but one or the other is needed for very large trees.  Same problem occurrs for backspacing over a term until it is blank--just before the filter is removed the tree will show every match of "h".  

- Positioning: there is currently no control over positioning of the search widget, which wastes real-estate and looks bad for those who position the task list as a wide view.  A similar problem would be noticed by other clients who use a wide tree with columns--you get a wide bar across the top, and a long distance to the Clear button.  Also, some clients will probably want to put widgets to the left and/or right of the filter combo (e.g. Microsoft Outlook screenshot attached).  It would be great if there were some way to make the location flexible (even move it to the toolbar, since it in a view it feels something like a toolbar), or to specify additional widgets that go on this "filter bar" as in the attached screenshot, and minimally to be able to specify text prefixing the combo (e.g. "Find: ").  That "type filter text" approach is fine for dialogs, but turned out to be really distracting in a view so we don't use it, at the cost of some confusion to new users.
Comment 16 Mik Kersten CLA 2005-12-30 09:01:53 EST
Created attachment 32353 [details]
"Look for" filter combo in Microsoft Outlook
Comment 17 Boris Bokowski CLA 2006-01-19 14:07:02 EST
Because FilteredTree uses the Jobs API, we cannot easily put it into JFace.  We decided to put it in the package org.eclipse.ui.dialogs.

Moving to Karice.
Comment 18 Karice McIntyre CLA 2006-01-26 10:31:59 EST
Released filtered tree as API into org.eclipse.ui.dialogs for now.  This will be considered experimental API until the end of M5.  Feedback can be entered in this bug.  

Mik, I think most, if not all of your concerns in comment #15 can now be properly addressed by overriding the behavior of the filtered tree in your own custom subclass.  We have provided a basic implementation that does not remember history (since such a feature may be overkill for some use cases).  

You can set the text to whatever you prefer using setInitialText(String).  Also, there is currently a delay of 200 ms in the job that refreshes the tree when the text is changed (e.g. a keystroke is made).  Hitting enter selects the first matching item in the tree.  
Comment 19 Karice McIntyre CLA 2006-02-07 17:07:13 EST
Just released revisions to API.
Comment 20 Mik Kersten CLA 2006-02-07 17:19:13 EST
I'm now working bootstrapped on a subtype of FilteredTree, the changes are working very nicely (e.g. having the Find: prefix, way better performance with a delay).  I really like the flexibility that comes from what you've exposed as protected.  Just a couple of issues came up:

1) For a large tree a delay of 200ms is too small since it requires the user to be typing at 60 words/minute (.2*60*5).  I've experimented and having it around 600-800 works well.  But overriding this via textChanged() requires FilteredTree.refreshJob to be made protected.  Is it possible to get that changed?

2) Once the delay is set to over 200, hitting the clear button inherits an overly long delay.  Making the clearText() method call refreshJob.schedule() directly will solve this.  If (1) is done subclasses can do this, but it seems generally applicable.

Comment 21 Karice McIntyre CLA 2006-02-07 17:54:44 EST
I didn't want to allow subclasses to change the refresh job so that the behavior of the filtered text is as consistent as possible.  What if I added a textChanged(int) method that allowed someone to change the refresh delay?  Then you could just override clearText(), which is just a special case of textChanged(), to have the delay you want.  OR I could add a method setRefreshDelay(int). 200 would be the default.  I am not entirely sure why 200ms was chosen as the original delay, but 600-800 seems quite long.
Comment 22 Mik Kersten CLA 2006-02-07 18:16:23 EST
That would work.  Although setRefreshDelay(int) seems a bit simpler, I vote for textChanged(int refreshDelay) because it would address both (1) and (2) in comment#20.  I stil lthink it makes sense to make the FilteredTree.clearText() call this with 0, because it doesn't seem to make sense to delay at all after the user presses the clear button, but yup, I can make my subclass do it.

Regarding my setting such a long delay--when typing at 15 words/min it takes 800ms to type a word (5 chars).  On my tree with well over a thousand elements, if the filter kicks in after I only type 2 letters I dozens or hundreds of matches, causing a long refresh delay before I can type the 3rd character.  Setting the delay to around 700 allows me to type 3 or more characters before the filter kicks in, and only get the matches I'm interested in.
Comment 23 Karice McIntyre CLA 2006-02-07 18:34:39 EST
I admit I may not have thought enough about your solution in comment #20 - things are a bit frantic this week.  Before I make any changes I will think about it some more.
Comment 24 Markus Keller CLA 2006-02-08 06:20:36 EST
I think the delay should depend on the size of the pattern already entered. E.g. after entering the first character in the field, a delay of 800ms-1500ms seems appropriate, but as soon as there are only a few items left in the tree, you want immediate feedback (e.g. 200ms).

Maybe the refresh job could count items and delay those updates a bit which would filter out many items at once? This would avoid the flickering seen when e.g. entering "a" in the preferences dialog, but would still show items quickly when typing subsequent characters of e.g. "accessibility".
Comment 25 Mik Kersten CLA 2006-02-08 12:33:34 EST
I really like Markus' idea of scaling the delay by the size of the text, have it running with the code blow, and it works way better than the original long delay I put in.  To find this bug in my task list if I type "qu", I have delay of 400 and get a ton of items as expected (thousands of items in total becuase the list includes an archive).  But if I type "quick" by the time I finish typing the 1/2 dozen items of interest appear immediately, and adding " f" immediately scopes it down to the bug report of interest.  It also addresses comment#20 (2) by making the delay 0 when the text has been cleared.  

Markus, I also agree with your comment that an ideal delay would vary with the number of items in the tree.  However, the code below (with the max set to 800) works great on both dozens and several thousand items.  So I think this is sufficient to make the prefs dialog flickering go away.  I imagine that if we got to tens of thousands (e.g. adding this to a navigator view) we might want a bigger delay, but at that point we would need a threshold on how many items are returned anyway in order to prevent blowing up the view.

    protected void textChanged() {
    	int MAX_REFRESH_DELAY = 800;
    	int textLength = filterText.getText().length();
    	int refreshDelay = 0;
    	if (textLength > 0) {
    		refreshDelay = MAX_REFRESH_DELAY / textLength;
    	}
    	refreshJob.schedule(refreshDelay); 
    }
Comment 26 Karice McIntyre CLA 2006-02-08 16:35:26 EST
Upon further discussion with Tod, I found out that the 200 ms number comes from usability research at IBM.  Any delay greater than 200 ms will be perceived as non-responsiveness by some users.  Eclipse UI consistently uses this number as a benchmark for the threshold where we provide feedback to the user.  
Comment 27 Karice McIntyre CLA 2006-02-08 16:45:24 EST
Removed experimental tag from class comments for FilteredTree and PatternFilter.
Comment 28 Mik Kersten CLA 2006-02-08 19:23:35 EST
Are you going to add the parameter to control the delay, i.e. textChanged(int refreshDelay)?  From what's documented here there still no mechanism in the API to control the delay (my code in comment#25 relies on a protected refreshJob field). 

Regarding the 200ms convention, I'm not sure how much it applies in the case of large filtered trees since the 200ms delay is irrelevant when it takes over a second to refresh before you can type the 2nd letter of your filter pattern.
Comment 29 Tod Creasey CLA 2006-02-09 07:44:59 EST
If it takes more than 200ms than the delay doesn't matter anyways as you say - it is only there to prevent the reprocessing of the filter too often. 

If it takes more than 200ms to filter we should be looking at the general performance of the filtering to see if we can speed it up. Can you give us an idea of how large the tree you are talking about is?

I can see wanting to reduce the refresh delay to Math.max(0,200ms - time to process) or else we are adding in a delay unneccessarily.
Comment 30 Karice McIntyre CLA 2006-02-09 11:29:22 EST
Mik, I think adding the API is a hack workaround for the performance problem you found.  This problem should be fixable without the user having to deal with knowing about refresh delay times.  

You should open a new bug that describes the performance issue.  If you could describe what you are seeing, how long the delay is, and the size of the tree you are using that would be very helpful in proceeding with addressing this performance issue.
Comment 31 Mik Kersten CLA 2006-02-09 11:38:45 EST
I can submit a new bug, but I don't quite see how this report is resolved given it's summary, and this is a usability issue and not a performance issue.  

The usability breaks with just a couple hundred nodes (my case is the Mylar task list, which can have thousands nodes that are typically filtered, but become unfiltered when a pattern is entered).   As described in comment#15 (first bullet), comment#20, comment#22 (2nd paragraph), this is not about refresh performance.  When you want to search for a word in a large tree you *don't* want the filtering to kick in after you've typed one letter.  As Markus indicates in comment#24, go to the Preferences view and type "acc".  Do you really want to see the tree bounce three times?  The issue is 100x worse with a tree that has a few hundred nodes, try it by mocking up a tree that large.  Has this been done on your end?  I can provide you with a tree that large but you would need to check out 2 Mylar projects to test it.  There seem to be two possible solutions:

1) Have a policy of not filtering until the user hits "enter".  No refresh delay (i.e. what MS Outlook's pattern filter does).  Key problem: lack of consistency with other filtered trees that appear in Eclipse.

2) Adapt the refresh delay for large trees to the size of text entered (comment#25), or allow subtypes to do it by adding the int parameter to textChanged().  Works beautifully.

As it stands, the FilteredTree is not usable for large trees and I think this bug should be reopened until it is, or the class should be marked as such.  The only reason I can use the FilteredTree is that I'm hacking in my own refresh job, but that's a very brittle approach.  
Comment 32 Mik Kersten CLA 2006-02-13 15:57:47 EST
Reopening--this is 99% of the way there, I just hope that the API doesn't get frozen without properly supporting large trees.  If the overridable refresh is not doable then something like making a subclass specify that the filtering shouldn't happen until the user hits Enter is needed.  

If this is working for anyone else on a large tree (with or without columns) please let me know what you are doing--the solution I am using is to make FilteredTree.refreshJob protected in order to be able to use the code in comment#25 in my subclass.  
Comment 33 Karice McIntyre CLA 2006-02-16 11:17:48 EST
This bug is being used to track the addition of the FilteredTree as API.  The issue with the delay time of the refresh job is now being tracked in bug 128233.
Comment 34 Karice McIntyre CLA 2006-02-16 11:18:37 EST
Verified in I20060214-0800
Comment 35 Nick Edgar CLA 2006-02-20 09:55:57 EST
*** Bug 101614 has been marked as a duplicate of this bug. ***
Comment 36 Susan McCourt CLA 2006-07-24 16:14:02 EDT
*** Bug 78972 has been marked as a duplicate of this bug. ***