Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 165088 - Add a "Open Remote Task" dialog
Summary: Add a "Open Remote Task" dialog
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: 0.9   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Willian Mitsuda CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 156389 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-11-18 19:26 EST by Willian Mitsuda CLA
Modified: 2006-12-18 18:15 EST (History)
2 users (show)

See Also:


Attachments
Screenshot (36.19 KB, image/gif)
2006-11-20 11:05 EST, Willian Mitsuda CLA
no flags Details
Initial patch (53.60 KB, patch)
2006-11-20 20:43 EST, Willian Mitsuda CLA
no flags Details | Diff
Initial patch (53.60 KB, patch)
2006-11-20 20:43 EST, Willian Mitsuda CLA
no flags Details | Diff
mylar/context/zip (47.00 KB, application/octet-stream)
2006-11-20 20:43 EST, Willian Mitsuda CLA
no flags Details
Screenshot for the new prototype (33.94 KB, image/gif)
2006-11-22 21:57 EST, Willian Mitsuda CLA
no flags Details
Patch n. 2 (61.95 KB, patch)
2006-11-22 22:08 EST, Willian Mitsuda CLA
no flags Details | Diff
mylar/context/zip (58.56 KB, application/octet-stream)
2006-11-22 22:09 EST, Willian Mitsuda CLA
no flags Details
Screenshot n. 3 (31.55 KB, image/gif)
2006-11-23 22:17 EST, Willian Mitsuda CLA
no flags Details
Patch n.3 (77.81 KB, patch)
2006-11-23 22:34 EST, Willian Mitsuda CLA
no flags Details | Diff
mylar/context/zip (62.10 KB, application/octet-stream)
2006-11-23 22:34 EST, Willian Mitsuda CLA
no flags Details
Changed file (3.10 KB, text/plain)
2006-11-23 23:15 EST, Willian Mitsuda CLA
no flags Details
mylar/context/zip (64.97 KB, application/octet-stream)
2006-11-27 15:32 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 Willian Mitsuda CLA 2006-11-18 19:26:56 EST
One thing that I didn't understand when I started to use Mylar is the concept that to be able to simply see a bug report I have to add it to the task list, either by using "add existing task" or by creating a query.

This is confusing to newbies and hard to people who don't want to follow the Mylar workflow (let's say someone just wants a bugzilla plugin for Eclipse) or simply wants to browse to an arbitrary bug.

I propose to create a "Open Remote Task" dialog, like "Open Task", where you can enter the bug ID and select the repository in a list.

Also, I think it can substitute the (confusing) "Add Existing Task" with advantages, by having some checkbox at bottom: "Add to task list".

Having it checked, it would have the same behavior the "Add Existing Task" has, but in just one dialog (no 2 page wizard).

Having it unchecked, it would just open the remote bug editor, like when you click on a bug hyperlink.

Another thing I miss on the current "Add Existing Task" wizard, and that we can include on this dialog is a combobox to optionally select in what category the bug should be added.

At last, it can be implemented as a command, so we can assign a shortcut key (any ideas on what can be the key sequence?).

I'm not sure if this is what Eugene suggests on item 3 of bug#152173 (Eugene? can you hear me? :)
Comment 1 Willian Mitsuda CLA 2006-11-20 11:05:52 EST
Created attachment 54176 [details]
Screenshot

Update: I'm working on a patch for this, and soon will attach it for comments.

For now, this is a screenshot of what I have in mind.
Comment 2 Eugene Kuleshov CLA 2006-11-20 15:23:46 EST
Hmm, I'd suggest to use a drop down for repository choice and show if there are matching tasks already in the list...
Comment 3 Willian Mitsuda CLA 2006-11-20 20:43:15 EST
Created attachment 54231 [details]
Initial patch
Comment 4 Willian Mitsuda CLA 2006-11-20 20:43:40 EST
Created attachment 54232 [details]
Initial patch
Comment 5 Willian Mitsuda CLA 2006-11-20 20:43:48 EST
Created attachment 54233 [details]
mylar/context/zip
Comment 6 Willian Mitsuda CLA 2006-11-20 20:52:28 EST
The previous patch adds only the simple dialog from screenshot, bound to Ctrl+F12 shortcut key.

In the patch I included some TODOs on RemoteTaskSelectionDialog about copy and pasted code that I think would be candidate to be a API method in some place.

Also, I include some code that turns the "Add Repository" action into a command, reimplement the AddRepositoryAction class based on this command. I was working on this code for another patch, but I included here because I use the command to implement the "Add Task Repository" button.

I didn't include the toolbar button, menu item, neither changed the "Add Existing Task" implementation for now, because I'd like to hear your opinions about this whole thing.

And of course, ask if there is a chance to include this on friday's feature freeze :)
Comment 7 Willian Mitsuda CLA 2006-11-20 20:57:04 EST
Eugene, I'm not sure if I got your point: are you suggesting a mix of this functionality with task list's tasks?

I agree that it is somewhat strange at first to have a "Open Task" and a "Open Remote Task" dialogs, but I couldn't imagine how the 2 interfaces can be merged, because the 2 use cases are different:

"Open Task" is basically to browse/select the task list based on id/summary filtering.

"Open Remote Task" is about getting a remote task exclusively by id (however it can be in the task list).
Comment 8 Eugene Kuleshov CLA 2006-11-20 22:24:24 EST
I often have no idea if task is already in my task list (i.e. when opening tasks from cvs/svn history). So such quick "preview" would be really handy and will save whole use of another open dialog.

PS: weird, but I wasn't been able to submit this comment using rich editor.
Comment 9 Willian Mitsuda CLA 2006-11-21 08:23:52 EST
Humm... I'm getting some ideas... I'll think a little about it and try to work on a second prototype...
Comment 10 Mik Kersten CLA 2006-11-22 15:38:28 EST
Willian: could you post a screenshot of your current UI?  I need to figure out whether we can try to squeeze this into 1.0 or if we should wait until after (probably the latter, but this might simplify things for people).
Comment 11 Willian Mitsuda CLA 2006-11-22 16:42:38 EST
I had not time yesterday to take a look into a second prototype. I'll see if I can get into this today tonight.

Do you agree about this feature? And about the scope of changes to be made on current UI?
Comment 12 Eugene Kuleshov CLA 2006-11-22 17:51:29 EST
Willian, so it is going to be like?

Task id(s): [_______]
[ Preview list ]
[Repository dropdown] | [ Add Repository button]
[x] Add to Task List  [Category]

Repository and category could be preselected in some cases (when called from project resource, or task list category/query/task).
Comment 13 Mik Kersten CLA 2006-11-22 18:35:19 EST
Looks good!  And then we remove the "Add Existing Repository Task" button.  Maybe call "Preview" Search, since it will have to run a search.
Comment 14 Mik Kersten CLA 2006-11-22 18:44:04 EST
Oh, and yes, I agree with this feature, think it would simplify things, and it would be fantastic to get it in by Friday so that it can make Mylar 1.0, assuming it works for, or is easy to make work for Trac and JIRA.  We'll still have to figure out all the places to add it but we can refine that later.  I'm thinking:
* Maybe Task List popup menu in the Open section
* Maybe File menu, it is similar to Open File...
* Navigate menu (but not toolbar)
Comment 15 Willian Mitsuda CLA 2006-11-22 19:09:19 EST
(In reply to comment #12)
> Willian, so it is going to be like?
> 
> Task id(s): [_______]
> [ Preview list ]
> [Repository dropdown] | [ Add Repository button]
> [x] Add to Task List  [Category]
> 

I don't like very much the idea of a combobox for the repository list. Maybe a little version of the current list view. Think +- like the folder pane on bottom of the old (Eclipse 3.2) "Open Resource" dialog.

(In reply to comment #13)
> Maybe call "Preview" Search, since it will have to run a search.

I think what Eugene means is to only preview if it exists currently on task list, not a background search (am I right, Eugene?)

(In reply to comment #14)
> Oh, and yes, I agree with this feature, think it would simplify things, and it
> would be fantastic to get it in by Friday so that it can make Mylar 1.0,
> assuming it works for, or is easy to make work for Trac and JIRA. 

Regarding the "open remote task" stuff, it is only a matter of implementing AbstractRepositoryConnectorUi.openRemoteTask(...).

Comment 16 Eugene Kuleshov CLA 2006-11-22 19:18:34 EST
(In reply to comment #15)
> I don't like very much the idea of a combobox for the repository list. Maybe a
> little version of the current list view. Think +- like the folder pane on
> bottom of the old (Eclipse 3.2) "Open Resource" dialog.

I suggested combobox because it takes less space. But you may want to try two lists with a Sash bar in between.

> > Maybe call "Preview" Search, since it will have to run a search.
> I think what Eugene means is to only preview if it exists currently on task
> list, not a background search (am I right, Eugene?)

Yes, it should be the same thing as in current "Open Task" dialog. So, would make sense to use "Matching Tasks:" label like in there.

Now I am thinking maybe it should be an "Add" button in that "Open Task" dialog that will somehow jump into the adding UI. In good case - double click on that button, in worst case - click, select repository and category and then click again.
Comment 17 Willian Mitsuda CLA 2006-11-22 21:57:59 EST
Created attachment 54384 [details]
Screenshot for the new prototype
Comment 18 Willian Mitsuda CLA 2006-11-22 22:04:17 EST
I tried to put 2 panes: one for repository list and another for task matches, but it doesn't look good. The split approach should not get better too.

In the new prototype, I tried to mix repository list with task matches, but I'm not sure if it is good yet... We got a simple dialog, but the mix looks strange.

There is a problem with the decoration too: I can't make the make my custom TaskElementOrRepositoryLabelProvider work with the DecoratingLabelProvider, so the tasks don't get the color/font according to its state. Is there any decorator wiz here that can take a look at this?
Comment 19 Willian Mitsuda CLA 2006-11-22 22:08:37 EST
Created attachment 54385 [details]
Patch n. 2

This patch adds the task matching to the dialog. Personally I don't liked it, although it was the better I could do until now.

It is getting hard to conciliate all those things in a simple interface...

Suggestions?
Comment 20 Willian Mitsuda CLA 2006-11-22 22:09:48 EST
Created attachment 54386 [details]
mylar/context/zip
Comment 21 Mik Kersten CLA 2006-11-22 22:43:52 EST
This is starting to look good Willian (I only reviewed the screenshot).  I don't think that we should ever mix repositories and tasks showing up in the same list in this way because it will be very confusing to non-experts.  So I think that this either has to use the MultiRepositoryAwareWizard and have a first page that allows you to select the repository (which would involve it being a wizard and not a dialog), or to have two combos.  But as you indicated previously the two combo thing makes good sense because it is similar to how the Open Resource dialog has you choose the container if more than one is available.  If you can address that I can take a pass at integrating the patch.
Comment 22 Eugene Kuleshov CLA 2006-11-23 00:03:49 EST
Willian, that is why I suggested to have a drop down for servers. Not sure what you and Mik meant by "combos". Anyways the whole point is to see if there is existing issues before even selecting repository.

I would pretty much like to have it all on one page, but you may try to make a two page wizard. First page will be like Open Issue dialog (including open with browser checkbox). So you can quickly open task from that page if it does exist. On the second one we could have repository selection and other related stuff. Sort of reversed way from MultiRepositoryAwareWizard.
Comment 23 Willian Mitsuda CLA 2006-11-23 09:24:43 EST
(In reply to comment #21)
> This is starting to look good Willian (I only reviewed the screenshot).  I
> don't think that we should ever mix repositories and tasks showing up in the
> same list in this way because it will be very confusing to non-experts. 

I'm glad that you didn't like it too :)

(In reply to comment #22)
> Willian, that is why I suggested to have a drop down for servers. Not sure what
> you and Mik meant by "combos". 

combo == combobox == drop-down list

> Anyways the whole point is to see if there is
> existing issues before even selecting repository.
> 
> I would pretty much like to have it all on one page, but you may try to make a
> two page wizard. 

I'm heavily against the use of wizards. The point is to make something simple and quick to use.

I'm not sure if I expressed correctly my use case: what I want I simply something that can be accessed quickly (Ctrl+F12, for example), open a dialog, type a bug number, press down to focus the repository list, select the repository (if it don't came preselected), hit enter, and it is done.

If the bug actually exists on task list, open it. If don't, open try to open from remote repository.

The "Add to task list" and the category combo is only a convenience, non-harmful feature if you want to add a non-existing task on tasklist.

Lets try to organize our use cases. I see at least 3 use cases for me (all of that happened):

1 - lets say your co-worker in the cubicle in front of you say: "hey, can you take a quick look at bug XYZ and say what you think?". You Ctrl+F12, type XYZ, hit enter. It opens, you take a look, says to your co-worked to blame the user, etc, Ctrl+F4 to close the editor and never mind about that again because it was not added to task list. It does not matter if it is present or not on tasklist, if it exists, it will be opened.

2 - lets say your co-worker says: "can you please close bug XYZ?". You hit Ctrl+F12, type XYZ, close the bug, submit. It doesn't matter if if was on task list or not.

3 - lets say your co-worker says: "can you please reopen that bug about the tiny reply icon that is hard to see?". On this case you have some alternatives:

- You say: "what is the bug number?", and go to use case 2.
- You don't know the bug number, so type Ctrl+Shift+F11, try to type some keyword like "reply icon" and see if it gets caught by the filter.
- You don't know the bug number, and the filter does help, or it is not on task list. On this case, you have nothing to do unless to make a bugzilla search for the bug number using some keyword, or anything else you can, but the final result is to get the bug number. On this case, we return to case 2.

On all this use cases I don't see a need for the matching tasks filter, and the first patch fits exactly my needs.

Having said that, I'd like to hear yours to see if they are incrementally compatible with what I need.
Comment 24 Willian Mitsuda CLA 2006-11-23 09:27:03 EST
Also, I don't like the combo/drop down idea because you have either:

- To use the mouse.
- Iterate over the elements using the arrow keys to see all existent options.
Comment 25 Eugene Kuleshov CLA 2006-11-23 15:12:57 EST
(In reply to comment #23)
> I'm not sure if I expressed correctly my use case: what I want I simply
> something that can be accessed quickly (Ctrl+F12, for example), open a dialog,
> type a bug number, press down to focus the repository list, select the
> repository (if it don't came preselected), hit enter, and it is done.

I agree. Thought I'd really would like to see if that issue in the task list or not, so I can choose to add it there.

> If the bug actually exists on task list, open it. If don't, open try to open
> from remote repository.

The thing is that number is not always unique and such preview would help to find the right one, including the case when you made typo in bug id or want to use quicksearch. Not to mention that you won't have to type the whole bug number if it is already in the task list.

Also, I find it cumbersome and confusing to have two dialogs that open issues. It is unclear when to use each one. that is why I've been suggesting to merge them.

> Also, I don't like the combo/drop down idea because you have either:
> - To use the mouse.
> - Iterate over the elements using the arrow keys to see all existent options.

You also have an option to learn keyboard shortcuts. Then you jump into read-only drop down list (ie, using keyboard accelerators or Tab) it will expand for you. Also, you can use Alt-Down key to expand it. Note that there will be only few repositories (if not one), so it is really waste of space to use a list control for those. All in all I think that drop down with "Add repository" button on the same line is a better choice here.
Comment 26 Mik Kersten CLA 2006-11-23 15:26:59 EST
Willian: I wouldn't worry too much about the combo/pull-down right now, because if you just remember the last selection it will be right most of the time, since for many developers most work happens on one repository, so it's OK to impose the click when it doesn't.

I totally agree about it being 10x better to have a one page dialog than a click-through wizard.

I think that it's fine not to do the search and merge with other open dialog at this point, because this already does the big favor of removing Add Existing Task, and it will be hard to merge that UI without being confusing (one is about Task List, one isn't, and we might make Task List vs. server more transparent post 1.0 anyway).
Comment 27 Willian Mitsuda CLA 2006-11-23 15:46:39 EST
(In reply to comment #25)
> I agree. Thought I'd really would like to see if that issue in the task list or
> not, so I can choose to add it there.

Hum... if you like it on task list, you can check the "add to list". It will be added if not present; it will simply be opened if it is.

> The thing is that number is not always unique and such preview would help to
> find the right one

This is new for me. I thought the bug number (or whatever equivalent is other repositories) was the "primary key" of the bug inside its repository (or should be?).

The problem is that the search by bug number is something that exists on every bug tracking system I know, and it is something Mylar doesn't have until now, at least on a easy way.

I.e.: every bug tracking system has some textbox somewhere in the page where you can just type the bug ID, hit enter, and get to it, or get a error screen if it does not exist.

In Mylar, the shortest path is to right-click the task list, go to the New submenu, select "Add Existing Task", where you go into a 2 page wizard, where first you have to select the repository, second you have to type the bug number and hit finish. Now you finally got the bug. After that, if you don't want it to pollute you task list, you have to delete it.

Got the point? Sometimes I don't want to use the tasklist! Probably some new users will get confused too for not being able to just browse an arbitrary bug, and claim that Mylar forces them to use the tasklist workflow (like I did in the beginning).

> Also, I find it cumbersome and confusing to have two dialogs that open issues.
> It is unclear when to use each one. that is why I've been suggesting to merge
> them.

The problem here is to find the best combination. I'm thinking this will be not possible, at least for now.

> You also have an option to learn keyboard shortcuts. Then you jump into
> read-only drop down list (ie, using keyboard accelerators or Tab) it will
> expand for you. Also, you can use Alt-Down key to expand it. Note that there
> will be only few repositories (if not one), so it is really waste of space to
> use a list control for those. All in all I think that drop down with "Add
> repository" button on the same line is a better choice here.
> 

Too many keystrokes :)
Comment 28 Willian Mitsuda CLA 2006-11-23 15:57:07 EST
(In reply to comment #26)
> Willian: I wouldn't worry too much about the combo/pull-down right now, because
> if you just remember the last selection it will be right most of the time,
> since for many developers most work happens on one repository, so it's OK to
> impose the click when it doesn't.

Hum... ok. There is a minor issue here that we'll not get the icon on combobox.

> 
> I totally agree about it being 10x better to have a one page dialog than a
> click-through wizard.

Great.

> 
> I think that it's fine not to do the search and merge with other open dialog at
> this point, because this already does the big favor of removing Add Existing
> Task, and it will be hard to merge that UI without being confusing (one is
> about Task List, one isn't, and we might make Task List vs. server more
> transparent post 1.0 anyway).
> 

If I understood right, you are favor of maintaining 2 open dialogs:

1 - The original "Open Task" with focus to tasklist tasks.
2 - The "Open Remote Task" which is the suject of discussion of this bug, with focus on server tasks (which can or can't be on tasklist).

If so, isn't maintaining the matching tasks list on "Open Remote" contradictory? Can we just go with my first patch/screenshot?

We can go with this simple dialog just to have this feature on 1.0 (the reasons why I think it is important to have this I explained in my previous comment), and try to get feedback after it.

Anyway, I'll try/post a third prototype with the matching tasks and a repository combobox, just to let us see how it could get.
Comment 29 Mik Kersten CLA 2006-11-23 17:36:08 EST
Yes, 2 dialogs for now.  It is a "worse is better" solution, but I totally agree that this flexibility is needed especially when someone wants not to be forced to add it to their Task List and just check out a bug.  We already allow searching in a way that is orthogonal to the Task List, and that used to be the way to open a repository task (it would immediately open by ID if that's all you specified).  

Please do post the third prototype, assuming it is reasonable I'll apply the patch and then we can streamline it.  That problem of not having icons in the combo box is annoying and an issue on the Task Search page too.
Comment 30 Eugene Kuleshov CLA 2006-11-23 18:24:31 EST
(In reply to comment #27)
> Hum... if you like it on task list, you can check the "add to list". It will be
> added if not present; it will simply be opened if it is.

But I may not know nothing about bug at this point. More over it could be already in my task list and I don't want to add another copy.

> This is new for me. I thought the bug number (or whatever equivalent is other
> repositories) was the "primary key" of the bug inside its repository (or should
> be?).

True for bugzilla, but may not true for web connector in some cases.

> The problem is that the search by bug number is something that exists on every
> bug tracking system I know, and it is something Mylar doesn't have until now,
> at least on a easy way.

That is a separate story and I actually suggested to add some quick search.
163341: Support for the repository quick search
https://bugs.eclipse.org/bugs/show_bug.cgi?id=163341

> In Mylar, the shortest path is to right-click the task list, go to the New
> submenu, select "Add Existing Task", where you go into a 2 page wizard, where
> first you have to select the repository, second you have to type the bug number
> and hit finish. Now you finally got the bug. After that, if you don't want it
> to pollute you task list, you have to delete it.
> 
> Got the point? Sometimes I don't want to use the tasklist! Probably some new
> users will get confused too for not being able to just browse an arbitrary bug,
> and claim that Mylar forces them to use the tasklist workflow (like I did in
> the beginning).

I know that and it is a great improvement. However it is only a half way and can work better if we could have one dialog for opening existing tasks and tasks from the repository.

We can try to collapse/hide the whole search preview list if there is no hits... So, repository only shown if local search don't find anything...

OR, another option is to use tree like on your last screenshot, but add node for Task List and show local hits as childs under that node.

Also, we seem to support need several parallel options: add or not to task list, open with reach editor or browser. I.e. allow to just add to the task list without opening and allow to open without adding to the task list. In total, there is 5 possible combinations of Add, Open and Open in Browser.

> Too many keystrokes :)

Alr-R, [Down]{0..5}, Enter ?
Comment 31 Willian Mitsuda CLA 2006-11-23 22:17:34 EST
Created attachment 54452 [details]
Screenshot n. 3

Afterall, it looks good for me.

You type the bug id, if it matches some existent on task list, it shows on the "Matching tasks" panel.

On this case, you can:

- Hit <down> to go to the pane and select some task and hit <enter> to open it.
- Just hit <enter> if there is some default repository preselected; on this case it will try to open the bug id on selected repository.
- Hit <tab>s to go to the repository combo and select a different one. The selection change on combo clear the selection on tasks pane, just in case you accidentally selected something while moving the focus using the keyboard.

I change the button text from "Add Task Repository" to simply "Add...". The older takes much space, is very invasive; the new label is simpler and communicates just the necessary.
Comment 32 Willian Mitsuda CLA 2006-11-23 22:34:51 EST
Created attachment 54453 [details]
Patch n.3

I hope this will be the last patch!

Unfortunately, I had not time to refactor the code and elliminate the copy'n pasted methods (they are marked with TODOs).
Comment 33 Willian Mitsuda CLA 2006-11-23 22:34:55 EST
Created attachment 54454 [details]
mylar/context/zip
Comment 34 Mik Kersten CLA 2006-11-23 22:54:34 EST
Reviewing now...
Comment 35 Mik Kersten CLA 2006-11-23 23:04:09 EST
Willian: I'm getting a compile error.  Did you miss a change to AbstractRepositoryClientWizard?  If so feel free just to paste in the new constructor.
Comment 36 Willian Mitsuda CLA 2006-11-23 23:13:07 EST
Ops, yes I did some modifications while working on turning the "add task repository" into a command.

I'll attach the source file.
Comment 37 Willian Mitsuda CLA 2006-11-23 23:15:04 EST
Created attachment 54455 [details]
Changed file
Comment 38 Mik Kersten CLA 2006-11-23 23:33:00 EST
Patch applied and verified.  Great stuff Willian!  I find the fact that local tasks show up a little confusing, but I should be able to make that clear with cleaning up the labels.  I'll iterate on integrating this with the UI tomorrow.
Comment 39 Willian Mitsuda CLA 2006-11-23 23:38:06 EST
Good! Thanks for your patience on integrating this feature!
Comment 40 Eugene Kuleshov CLA 2006-11-24 00:30:52 EST
So, are we doing to dich the "Add existing task" wizard now?
Comment 41 Eugene Kuleshov CLA 2006-11-24 00:58:08 EST
Forgot to mention that repositories in that drop down are inconsistent with whet we see in repository view and on wizards (should at least have label up front the url). Drop downs better to be aligned to each other.

Also, is this new dialog is only available trough Ctrl-F12? Should it be in Navigation menu and maybe replace "Open Task" dialog on the main toolbar too?

There is also weird delay after hitting ok, while task is synchronized, but I guess it is a common Mylar's issue. It should open editor quickly and then lazily fetch any required content...
Comment 42 Willian Mitsuda CLA 2006-11-24 08:43:19 EST
(In reply to comment #40)
> So, are we doing to dich the "Add existing task" wizard now?
> 

I'm really not sure what to do about it now. This dialog can perfectly replace that wizard, but the UI doesn't communicate that intention. Thoughts?

(In reply to comment #41)
> Forgot to mention that repositories in that drop down are inconsistent with
> whet we see in repository view and on wizards (should at least have label up
> front the url). Drop downs better to be aligned to each other.

Agree. That bottom part of dialog still looks a little strange. I didn't get to make the "Add to task list" part looks well together with the rest of UI.

> Also, is this new dialog is only available trough Ctrl-F12? Should it be in
> Navigation menu and maybe replace "Open Task" dialog on the main toolbar too?

Yes, I think at least it can appear in the navigation menu, so people know that it exists.

> There is also weird delay after hitting ok, while task is synchronized, but I
> guess it is a common Mylar's issue. It should open editor quickly and then
> lazily fetch any required content...
> 

I'm just calling existing methods used in other places. This is only a matter of optimization...
Comment 43 Eugene Kuleshov CLA 2006-11-24 12:13:36 EST
 (In reply to comment #42)
> > There is also weird delay after hitting ok, while task is synchronized, but I
> > guess it is a common Mylar's issue. It should open editor quickly and then
> > lazily fetch any required content...
> I'm just calling existing methods used in other places. This is only a matter of optimization...

I am not blaming you, just a side remark to bug Mik. :-)
Comment 44 Mik Kersten CLA 2006-11-27 15:32:26 EST
Done.  It's great that you got this done in time for 1.0 Willian.  I made the following changes, details will be in the New & Noteworthy:
* Refactored common code into TasksUiUtil.getSelectedRepository(StructuredViewer) and inlined some additional resolution.
* Made it an action like the other Open/activate actions, so I got rid of the defaultHandler
* Simplified shortcuts for all actions to only use F9 and F12, and I'm very happy with the result because it should be easier to remember.  Also, F9 is has an eject key on the ThinkPad (and perhaps other) keyboards, making it easier to remember.  Please take a look at the new shortcut mappings in the Navigate menu.
* Got rid of the "Add Existing Task" action (yay!)
Comment 45 Mik Kersten CLA 2006-11-27 15:32:29 EST
Created attachment 54582 [details]
mylar/context/zip
Comment 46 Willian Mitsuda CLA 2006-11-27 15:50:49 EST
(In reply to comment #44)
> * Made it an action like the other Open/activate actions, so I got rid of the
> defaultHandler

Humm... I disagree... I'm not sure if this is the "correct" way to do because you are moving after core logic into an action and it appears to be a deprecated practice.

I was even thinking about redesign the existing open/activate/deactivate task actions into handlers.

Unfortunately, there is no good documentation about the "right" way to decouple action from commands, the best thing I got was the following page:

http://wiki.eclipse.org/index.php/Platform_Command_Framework

From there, I think it is better to make the command implementation a handler and to have an action just call a handler via IHandlerService.

> * Simplified shortcuts for all actions to only use F9 and F12, and I'm very
> happy with the result because it should be easier to remember.  Also, F9 is has
> an eject key on the ThinkPad (and perhaps other) keyboards, making it easier to
> remember.  Please take a look at the new shortcut mappings in the Navigate
> menu.

I'll take a look on next build.

> * Got rid of the "Add Existing Task" action (yay!)

Great! Less confusion!
Comment 47 Mik Kersten CLA 2006-11-27 17:02:56 EST
Yes, once there is a clear convention we should move all of the Task List actions into commands.  That's bug 145884, so it would be great if you could post that link there.
Comment 48 Eugene Kuleshov CLA 2006-11-27 20:00:03 EST
 (In reply to comment #44)
> * Refactored common code into TasksUiUtil.getSelectedRepository(StructuredViewer) and inlined some additional resolution.

I think it is a bad idea to pass a viewer like this. It seems like we should have an Adaptable and new interface like LinkedRepositoryInfo, which could be adapted to various things...
Comment 49 Willian Mitsuda CLA 2006-11-27 20:23:05 EST
(In reply to comment #44)
> * Simplified shortcuts for all actions to only use F9 and F12, and I'm very
> happy with the result because it should be easier to remember.  Also, F9 is has
> an eject key on the ThinkPad (and perhaps other) keyboards, making it easier to
> remember.  Please take a look at the new shortcut mappings in the Navigate
> menu.

I'll take some days to assimilate the new keybindings, but they look very good. Very cool icons too.
Comment 50 Mik Kersten CLA 2006-11-28 10:16:23 EST
Willian: I figured that the change of shortcut mappings would be hardest on you ;)  

I'm not loving the idea of passing the viewer either, partly because it goes against the Eclipse selection service ideas.  I actually reverted to Willian's duplicate code, and we should clean up that method on TasksUiUtil.  What do you guys think if we make all appropriate ITaskListElements adapt to TaskRepository?

Comment 51 Eugene Kuleshov CLA 2006-11-28 14:48:16 EST
Mik, you would also have to adapt projects (and other resources), stuff from history view, and lots of other stuff (maybe even somehow adapt editors).
Comment 52 Eugene Kuleshov CLA 2006-11-28 16:50:13 EST
 (In reply to comment #50)
> I'm not loving the idea of passing the viewer either, partly because it goes against the Eclipse selection service ideas.  I actually reverted to Willian's duplicate code, and we should clean up that method on TasksUiUtil.

BTW, can you please make sure that you fix preselecting repository when new wizards are called from the Task List view. It is broken in last release.
Comment 53 Eugene Kuleshov CLA 2006-11-30 15:48:45 EST
*** Bug 156389 has been marked as a duplicate of this bug. ***
Comment 54 Willian Mitsuda CLA 2006-12-03 13:23:01 EST
 (In reply to comment #50)
> I'm not loving the idea of passing the viewer either, partly because it goes against the Eclipse selection service ideas.  I actually reverted to Willian's duplicate code, and we should clean up that method on TasksUiUtil.  What do you guys think if we make all appropriate ITaskListElements adapt to TaskRepository?

No doubt that this is the right way to do not only this, but eliminate all of the instanceof ifs.

Each of them can be turned on a adapter to TaskRepository, and the code will be simple as of:

ISelection selection = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getSelectionService().getSelection();
if (selection instanceof IStructuredSelection) {
    Object elem = ((IStructuredSelection) selection).getFirstElement();
    return (TaskRepository) Platform.getAdapterManager().getAdapter(elem, TaskRepository.class);
}
return null;

I saw this "pattern" in some other places, so a refactoring like this can benefit and simplify many other classes.
Comment 55 Eugene Kuleshov CLA 2006-12-03 16:44:16 EST
Don't you have to check if elem is itself an IAdaptable?
Comment 56 Willian Mitsuda CLA 2006-12-03 19:25:19 EST
 (In reply to comment #55)
> Don't you have to check if elem is itself an IAdaptable?

I'm not sure, but I think if IAdapterManager is safe about passing a non-IAdaptable object.
Comment 57 Eugene Kuleshov CLA 2006-12-03 19:48:53 EST
(In reply to comment #56)
>  (In reply to comment #55)
> > Don't you have to check if elem is itself an IAdaptable?
> 
> I'm not sure, but I think if IAdapterManager is safe about passing a
> non-IAdaptable object.

It is safe, but I thought that AdapterManager only handle adapters managed by adapter factories. At least you can see it all in the Platform that classes using IAdaptable check instance first and only then delegate to the platfrom AdapterManager.
Comment 58 Mik Kersten CLA 2006-12-04 21:02:00 EST
Willian: please make a new bug out of comment#54.  
Comment 59 Eugene Kuleshov CLA 2006-12-04 21:58:57 EST
(In reply to comment #58)
> Willian: please make a new bug out of comment#54.  

Mik, in a mean time, can you please fix preselection of the repository?

Comment 60 Willian Mitsuda CLA 2006-12-17 22:43:12 EST
(In reply to comment #58)
> Willian: please make a new bug out of comment#54.  
> 

Done. bug#168376.
Comment 61 Willian Mitsuda CLA 2006-12-18 00:10:50 EST
(In reply to comment #57)
> It is safe, but I thought that AdapterManager only handle adapters managed by
> adapter factories. At least you can see it all in the Platform that classes
> using IAdaptable check instance first and only then delegate to the platfrom
> AdapterManager.
> 

I just read the chapter about the "adapter" pattern from "Contributing to Eclipse" book, and it seems that the preferred way to use adaptables is to check if the object is a instanceof IAdaptable and them make a getAdapter().

The object can either extends PlatformObject (that implements IAdaptable and delegates the adapter checking to platform's adapter manager) or implement IAdaptable, delegating to platform's adapter manager on last chance.