Community
Participate
Working Groups
The list of tasks displayed in the dialog can be very long when the task list has accumulated many tasks over time. It would be great if there was a way to toggle the list with a short cut to filter all completed tasks.
+1 This can be as simple as a checkbox near Open with Browser.
I was thinking of using a FilteredItemsSelectionDialog and using the view menu making it similar to the JDT "Open Type" dialog.
That would be great. We might get some better performance too. For some reason the current Open Task dialog is very slow to open, at least on my workspace with thousands of tasks. But it shouldn't be (performance prolbem could be on our end though).
-1 for checkbox +1 for drop down menu like in "Open Type" dialog (it could also have some sort modes too) I think current implementation is creating temporary list with the search results and populating list widget every time. It should instead use the same model as the Task List.
+1 for for drop down menu as well, that is the right place for filter/sorter UI as Eugene points out. This will be at the cost of discoverability but I think that the consistency is worth it.
It would also make it easier to merge "open..." dialogs.
I can take a look at this one. (In reply to comment #3) > That would be great. We might get some better performance too. For some reason > the current Open Task dialog is very slow to open, at least on my workspace with > thousands of tasks. But it shouldn't be (performance prolbem could be on our > end though). Current implementation calculates a set of all your tasks (TaskList.getAllTasks()) + all query hits (AbstractRepositoryQuery.getChildren()) to be the input of filtered tree. Perhaps (I didn't made any benchmarks) this is a expensive job. Question: with all API changes made on 2.0, is this still necessary? Is there a better way to get a union of all tasks + query hits from your workspace? The initial display is based on your task activation history. Just in case, how many tasks you have on your task list (+-)? And what is the size (+-) of your activation history?
Willian, in 2.0 TaskList.getAllTasks() will return all tasks because there is no hits anymore.
(In reply to comment #7) > Just in case, how many tasks you have on your task list (+-)? And what is the > size (+-) of your activation history? I have over 6000 tasks. Based on some research results (e.g. Gonzales et al.) and some of our histories a good benchmark for activation history is 16 x days worked, so let's say 20K for a history. But if that becomes a performance issue at any point we are likely to start collapsing it.
Update: I'm almost there and it is getting very good. I'm just fixing a few UI glitches. I'm following Stephen's suggestion and converting to use FilteredItemsSelectionDialog, and thanks to all its "magic" we got for free, the filtering is much more quicker.
Great to hear! I was itching for us to move to the new API :)
Created attachment 73797 [details] Proposed patch
Created attachment 73798 [details] mylyn/context/zip
Some comments about my last patch: 1 - I converted the dialog to use the FilteredItemsSelectionDialog, but there are some points (pros/cons) in adopting it: Behavior changes related to the old implementation: - This class is Eclipse 3.3 only. - The implementation wraps the label provider and "cancel" the IColorProvider, so we lost all color decoration for tasks. - This class provides its own history control. Previously I used the task list activation history. - The last accessed items are sorted alphabetically, rather than by last access order. This is because it uses the same comparator to history/other sections. - The filtering requires you to explicitly add a "*" to inform your filter is a regular expression, e.g., the old implementation allowed you to type "cvs" to match anything with "cvs" inside. The new implementation requires you to type "*cvs". Pros: - The filtering is very much faster than the old implementation, thanks the use of virtual tables, caches, etc. we get for free from that class. I think the gain is worthy and it will make it easier to improve the dialog in other ways in future. 2 - I also made some improvements over the old dialog: - I added a label decorator over the label provider. So now it shows the priority decorator on the left site of the task icon. - I enabled the status section on dialog and made it show the repository label for the task (like open resource shows the folder for files, or open type shows the class package). These are experiments, so feel free to remove if you think it bloats the dialog (although I like it). Regarding the status section: I noted the repository overlay has some white pixels which should be transparent. You can note this here because of dialog background color. 3 - A question about the history management: I used the AbstractTask.getHandleIdentifier() to persist/restore the task history. It worked, but is it reliable? Also, I had to use TaskList.getTask(String handle) to restore tasks from handles, but there is a TODO for removing it in future. Can you please check if the way I implemented is the right way?
(In reply to comment #14) > - The implementation wraps the label provider and "cancel" the IColorProvider, > so we lost all color decoration for tasks. Regarding this issue, I opened bug#196553.
This is a very cool patch. Thanks Wilian! (In reply to comment #14) > - The filtering requires you to explicitly add a "*" to inform your filter is a > regular expression, e.g., the old implementation allowed you to type "cvs" to > match anything with "cvs" inside. The new implementation requires you to type > "*cvs". After using this for a bit I find that a little bit annoying since tasks always start with the id, so I always have to type a * first to do a search by keyword. Is there any way to work around this by intercepting the search and adding a *?
(In reply to comment #16) > After using this for a bit I find that a little bit annoying since tasks always > start with the id, so I always have to type a * first to do a search by > keyword. Is there any way to work around this by intercepting the search and > adding a *? > I thought this too. I'll try to study how the matching works and see if can make it work this way.
Great to see this progress Willian. But I'm quite concerned about introducing a second history mechanism and would really like leave it to the single history in order to keep the UI simple and predictable (we can't expect users to understand the semantics of two different histories and Mylyn already overload's the Open Type dialog's history with the task context). Also, I wonder how big a problem the loss of color decorations is, because it means that users will be unable to distinguish between completed and non-completed tasks on non-Windows OS's and that we will lack consistency with the Task List. Since our current target is 3.3.x, if Platform applies the patch we could consider working around this by doing something like setting and overriding getItemsListLabelProvider(..) before creation. But if comment#16 and the above issues are seem problematic perhaps we should consider overriding even more of the dialog's behavior if that's possible?
(In reply to comment #18) > Great to see this progress Willian. But I'm quite concerned about introducing a > second history mechanism and would really like leave it to the single history in > order to keep the UI simple and predictable (we can't expect users to understand > the semantics of two different histories and Mylyn already overload's the Open > Type dialog's history with the task context). I didn't investigate before because for me it isn't really a big issue. But your point is valid, and I'll take a look if we can unify the history mechanism. > Also, I wonder how big a problem > the loss of color decorations is, because it means that users will be unable to > distinguish between completed and non-completed tasks on non-Windows OS's and > that we will lack consistency with the Task List. Since our current target is > 3.3.x, if Platform applies the patch we could consider working around this by > doing something like setting and overriding getItemsListLabelProvider(..) before > creation. I took a quick look at FilteredItemsSelectionDialog and the problem is that the getter methods and the label provider wrapper attribute are private. Even the ItemsListLabelProvider inner class is private, so we cannot extend it to include the missing interfaces. Any idea? Anyway, is it ok if my patch on bug#196553 gets accepted for 3.3.1? This would imply that Mylyn 2.1 must have a minimum requisite of 3.3.1 too, but we could avoid having to hack the dialog. If so, could would vote for this there?
Although I will see how the schedules overlap, it is unlikely that Mylyn 2.1 can't have a 3.3.1 dependency. But there are other ways that we can hack in the provider into 3.3.0 if they apply the patch for 3.3.1 (e.g. reflection) so I'm voting on that bug now.
(In reply to comment #20) > But there are other ways that we can hack in the > provider into 3.3.0 if they apply the patch for 3.3.1 (e.g. reflection) so I'm > voting on that bug now. Interesting. Do you have some sample code on how this works?
Created attachment 75413 [details] Updated patch
Created attachment 75414 [details] mylyn/context/zip
I resolved the following problems in the last patch: - No need to prepend the "*" to the filter. - Overrided the history management to use Mylyn's activation history. That was tough because the actual implementation is very tied to settings-file-based history storage. I had to override a lot of methods to cancel the default implementation, but it works. - Regarding the color/font problem, bug#196553 was commited to 3.4 M1, so if you want to experiment with my patch, you have to checkout the HEAD of org.eclipse.ui.workbench from CVS. - A fix in 3.3.1 was reported on bug#198549, and I hope it will be accepted. So, if Mylyn 2.1 could have a prerequisite of 3.3.1, the color/font problem hopefully won't require any hack on our side. But there is still a problem: although I said I resolved the history problem by making it to use Mylyn's task activation management, the dialog displays the history on alphabetical order, not the recent-to-past ordering that is used actually. The hard part is that the methods/attributes that control the sort order on FilteredItemsSelectionDialog are totally private.
Willian: sorry for the radio silence on this. I would like to apply this patch after next Friday's 2.1 release. At that point we will be able to make bigger UI changes of this sort, but for 2.1 the goal was to keep the UI as stable and consistent as possible.
Hi, I'm back ;) Sorry for the slowness. The current patch got stale. I'll provide a new one.
Created attachment 80391 [details] Updated patch
Created attachment 80392 [details] mylyn/context/zip
This is looking very nice Willian. I like how you've added the details pane with the associated repository. Just one outstanding issue I noticed. When a task is opened, it gets added to the task activation history (even thought the task isn't being added). This appears to be due to code in the TaskSelectionHistory. Once this is resolved this will be committed.
Created attachment 81232 [details] Updated patch
Created attachment 81233 [details] mylyn/context/zip
(In reply to comment #29) > This is looking very nice Willian. I like how you've added the details pane with > the associated repository. Just a little polish item you would like to check later: in the repository icon you mentioned, there are a few white solid pixels that should be transparent.
Rob, can you please review? (just a ping, in case this got lost)
Patch applied. This is great work Willian! Now we won't feel as bad at taking up a toolbar item with this dialog ;) What's affected on 3.3.0, is it just the missing decorations? Note that I made a minor change to the text for the separator (uses TaskListView.LABEL_PART). However, we have a small problem with the decoration, because of the fact that we are using Mylyn's wide icons. The problem is in the following code in ItemsListLabelProvider, which will shortcut the actual label provider (that knows about the wide icons). public Image getImage(Object element) { if (element instanceof ItemsListSeparator) { return WorkbenchImages .getImage(IWorkbenchGraphicConstants.IMG_OBJ_SEPARATOR); } return provider.getImage(element); } Steffen and I deliberated on this some, and given the blinking of the priority icons and the fact that it would be very hard to work around the above constraint we're leaning to leaving the priority decorations off. The blinking problem dominated. I have committed this change, could you try it out and see which approach you prefer? We can change back.
(In reply to comment #34) > What's affected on 3.3.0, is it just the missing decorations? Just color decorations. 3.3.1 is fine. > Note that I made a minor change to the text for the separator (uses > TaskListView.LABEL_PART). However, we have a small problem with the decoration, > because of the fact that we are using Mylyn's wide icons. The problem is in > the following code in ItemsListLabelProvider, which will shortcut the actual > label provider (that knows about the wide icons). Mik, I think something is wrong here. I can swear that the last time I submitted the patch I tested the dialog, and the icons were wide, and the priority decorating appeared nicely on the left of task icon. Do you remember any change that could possibly have affected that? > Steffen and I deliberated on this some, and given the blinking of the priority > icons and the fact that it would be very hard to work around the above > constraint we're leaning to leaving the priority decorations off. The blinking > problem dominated. I have committed this change, could you try it out and see > which approach you prefer? We can change back. The blinking problem seems to be from platform. It can be reproduced by opening the open resource dialog (Ctrl+Shift+R). The CVS overlay blinks as you navigate through resource names. I see no reason why they erase the decoration just to paint a second later. Maybe we should report them to platform?
(In reply to comment #35) > Mik, I think something is wrong here. I can swear that the last time I submitted > the patch I tested the dialog, and the icons were wide, and the priority > decorating appeared nicely on the left of task icon. I checked the commited code, and see that you changed the line: labelProvider = new TaskElementLabelProvider(false); In the original patch, it was passing true. If you change it, you will get wide icons. You can be sure it works by increasing CompositeTaskImageDescriptor.OFFSET_DECORATION from 6 to a bigger value, and see that the icon width increases. The only issue I see, is that it appears that the priority decoration misses 1 or 2 pixels of blank space on the left, because it seems to overlap with the selection rectangle border when selected (at least in Windows Vista).
Willian: sorry if I was not clean enough in the reasoning in comment#34. I deliberately switched to narrow icons because there are two problems with wide icons: 1) The blinking of decorations (this is Platform bug 130340, which I just put another comment on) 2) The fact that due to the wide image use, the separator icon was stretched and hence improperly rendered (as per comment#34 there is no straightforward solution to that). In addition, I'm not clear on how important it is to show the priority decorations in this dialog, since the user is searching by name, and they are visually loud. Could you try working with it both ways and let us know what your assessment is?
Ok, ok, I misunderstood. Sorry, I thought you were talking the wide icons weren't working for you. Now I see. And, yes, I think we can live without the priority icons. The core functionality, be able to filter completed tasks, and the migration to new dialog are done. Let's leave as is for now.
(In reply to comment #35) > Just color decorations. 3.3.1 is fine. Correcting myself: color and font decorations.
Great work Willian! I really like the new dialog. It is much faster and prettier than the old one.
