Bug 135025 - improve the import plugins/fragments filter
Summary: improve the import plugins/fragments filter
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 enhancement with 1 vote (vote)
Target Milestone: 3.4 M2   Edit
Assignee: Adam Archer CLA
QA Contact:
URL:
Whiteboard: Autoed,G
Keywords: contributed
: 149365 194121 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-04-05 10:21 EDT by Rodrigo Peretti CLA
Modified: 2010-03-24 23:10 EDT (History)
8 users (show)

See Also:
agarcher: review? (caniszczyk)


Attachments
patch (12.38 KB, patch)
2007-08-14 11:31 EDT, Adam Archer CLA
no flags Details | Diff
patch (21.87 KB, patch)
2007-08-14 16:40 EDT, Adam Archer CLA
no flags Details | Diff
mylyn/context/zip (1.74 KB, application/octet-stream)
2007-08-14 18:07 EDT, Chris Aniszczyk CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rodrigo Peretti CLA 2006-04-05 10:21:29 EDT
Eclipse 3.2M6

When you try to import plugins/fragments you are provided with a filter where you can type the plugin name or wild cards like "*". The problem is when using wildcards the plugins from the list are only selected and not filtered out. If you have 300 plugins and you type "*ui" as the filter, you still have to scroll the list of 300 plugins in order to find the ones that match your filter.

Besides being hard to use it is inconsistent with other filters provided by PDE. For example, when you want to add a dependency to your plugin, when you type "*ui" all plugins that do not match the filter are removed from the list. This is the behavior I would expect from the import wizard.
Comment 1 Wassim Melhem CLA 2006-07-05 19:00:13 EDT
*** Bug 149365 has been marked as a duplicate of this bug. ***
Comment 2 Chris Aniszczyk CLA 2006-07-23 22:55:52 EDT
will investigate using some of those fancy JFace filters...
Comment 3 Aaron Digulla CLA 2006-11-27 15:30:44 EST
Can we have this one for 3.3? *beg*
Comment 4 Chris Aniszczyk CLA 2006-11-27 15:35:41 EST
I agree this needs some work. We have to be a bit careful with this one though to make sure the user experience is solid.

I'll put it under my plate so I don't forget.
Comment 5 Wassim Melhem CLA 2006-11-27 15:41:00 EST
the filtering makes the semantics of all the buttons on that page more vague.

What does 'Add All -->' mean now, for example?  all visible/filtered plugins?  all visible/invisible plugins?

What about the count?  xxx out of yyy selected.
Comment 6 Chris Aniszczyk CLA 2006-11-27 16:06:54 EST
(In reply to comment #5)
> the filtering makes the semantics of all the buttons on that page more vague.
> 
> What does 'Add All -->' mean now, for example?  all visible/filtered plugins? 
> all visible/invisible plugins?

I would say add all means add all visible plug-ins.

> What about the count?  xxx out of yyy selected.

I think count should apply to everything. Maybe we can preface it with something like xxx out of yyy total plug-ins selected.

Comment 7 Aaron Digulla CLA 2006-11-27 16:40:14 EST
Usually, the user will expect the number to represent the amount what he can see by scrolling (ie. not the total amount but what is left after filtering).

The filtering is an act by the user so she'll remember that she just did something and will expect the numbers to reflect this action.

If you must, why not add "xxx of yyy plugins selected (zzz hidden)"? But I think that's more confusing than helping since the filter is visible in the same window and the time between filtering and doing something with the filtered set is short.
Comment 8 Wassim Melhem CLA 2006-11-27 16:58:25 EST
What about 'Required Plug-ins --->'?

It may not be a good idea to limit this to the visible set of plug-ins.  Plug-ins matching a particular regexp patter hardly constitute an entire/coherent list of dependencies.

So this would cause mixed semantics (act using the filter with one button, but disregard the filter with another button).
Comment 9 Wassim Melhem CLA 2007-07-15 01:57:48 EDT
*** Bug 194121 has been marked as a duplicate of this bug. ***
Comment 10 Adam Archer CLA 2007-08-14 11:31:39 EDT
Created attachment 76050 [details]
patch

This patch implements the filtering support. Here's what it does:

A ViewerFilter is added to the available viewer so that any time a full refresh is done on the viewer, the filter is automatically applied. This filter will always exist. It will always filter out items that exist on the import list (are selected). This is because on a full refresh, the list gets repopulated with all items. In addition it will apply the filter specified in the filter field at all times.

However, to improve performance, the tree viewer is only fully refreshed (and therefore, the filter is only run) when the user types in that field. When adds, removes, etc. are performed, we instead manually add and remove items from the list and obey the semantics of the filter.

I would like to revisit a few issues that this change raises that could use some more discussion.

-The xxx out of yyy label: In this patch, yyy still gives the total number of plug-ins, as opposed to the number matching the filter criteria as suggested in comment 7. Changing this behaviour could be highly misleading since the user can actually have plug-ins that DO NOT match the filter selected (the filter is only applied to the available list). Thus, if they added a few plug-ins to the list and then typed a filter and added all the resulting matches, they could wind up with a current number greater than the total number (xxx > yyy). For this reason, I think this should be left as is, as suggested by Chris in comment 6.

-Swap: When a filter exists and the swap button is pressed, we currently add all plug-ins in the available list (plug-ins NOT already selected that DO match the filter) to the import list and remove all plug-ins that were selected. Any of the removed plug-ins that meet the filter are displayed again in the available list (some of them may not and these ones will disappear). If we don't do this check against the filter, we will be invalidating the filtered state of the available list (can't do that). Unfortunately, this can make the swap operation quite confusing since the lists are not actually being swapped. One way to solve this would be to disable the swap button any time a filter is selected. I'm not sure this is the best solution as this function could, theoretically, be useful. Perhaps it would be best for users to just learn this nuance for themselves.

-Functions that do not depend on the filter: The Existing Plug-ins, Existing Unshared, and Required Plug-ins buttons currently do the same thing they did before which means they DO NOT depend on the filter at all. They will add exactly the plug-ins that meet the specified criteria and remove the rest. This has the same problem with the swap operation in that removed plug-ins may disappear entirely. It also has the additional problem that we are adding plug-ins that are not appearing in the available view due to the filter. I'm not sure what would be best here. Should we only add the plug-ins that meet the criteria AND the filter?
Comment 11 Chris Aniszczyk CLA 2007-08-14 12:12:28 EDT
Adam, I like the patch, however.

1) Should we consider creating a FilteredTable similar to FilteredTree and use
that instead. This would give us a bit more space and we can apply a
"FilteredTable" on both sections (if it makes sense). However, a FilteredTable
doesn't exist yet and we would have to create one for usage within PDE. Or, do
we leave the one filter up on top and apply it to both tables? Something about
that filter up there doesn't feel right. If it only applies to one table, it
should be positioned in such a way.

2) "Locate Plug-ins" is a bad name. We should rename this to something like
"Filter Plug-ins and Fragments" (similar to how we do it in the New Extension
wizard).

3) I'm noticing a weird flicker when we run the refresh job on the list. ie.,
type a filter, com.ibm and you should notice it.

4) The buttons, Add, Remove, Swap, Required Plug-ins depend on certain
selections. These should be disabled if selection criteria isn't met.

In the end... this is significantly better than what we had before.

Brian/Mike, what are your thoughts? This is a tricky wizard ;D
Comment 12 Adam Archer CLA 2007-08-14 16:40:17 EDT
Created attachment 76084 [details]
patch

(In reply to comment #11)
> 1) Or, do we leave the one filter up on top and apply it to both tables?
I don't think there is any reason to filter the import table. If the user does not want to import something they should remove it from the list (or never add it in the first place). I agree that having the field at the top spanning the whole window is misleading, however, and I think the solution here is to localize it to be above the available table.

> 2) "Locate Plug-ins" is a bad name.
Changed this to "Filter Available Plug-ins and Fragments" as a semi-workaround to (1). Though we might need to get rid of this label altogether in the future depending on how we address (1).

> 3) I'm noticing a weird flicker
Fixed.

> 4) The buttons, Add, Remove, Swap, Required Plug-ins depend on certain
> selections. These should be disabled if selection criteria isn't met.
Done. Included enablement for Add All and Remove All also. Left enablement for Swap alone (still always enabled).

All that's left is issue (1). That's a substantial overhaul to the wizard's UI and I think we should address it in another bug.
Comment 13 Chris Aniszczyk CLA 2007-08-14 18:06:47 EDT
Looks great. Dani and other esteemed clients of this wizard page will be happy with the new changes.
Comment 14 Chris Aniszczyk CLA 2007-08-14 18:07:02 EDT
Created attachment 76091 [details]
mylyn/context/zip