Comment 1 Frank Becker 2012-02-18 04:22:27 EST

Create review http://review.mylyn.org/304

Comment 2 Frank Becker 2012-02-18 04:22:32 EST

Created attachment 211229 [details]
mylyn/context/zip

Comment 3 Frank Becker 2012-02-18 11:48:01 EST

Create Patchset 2 based on the comments

Comment 4 Frank Becker 2012-02-18 11:48:08 EST

Created attachment 211232 [details]
mylyn/context/zip

Comment 5 Frank Becker 2012-02-18 15:28:49 EST

Create Patchset 3 based on the comments

Comment 6 Frank Becker 2012-02-18 15:28:52 EST

Created attachment 211238 [details]
mylyn/context/zip

Comment 7 Frank Becker 2012-02-19 08:13:25 EST

Fix for failing TracTests is now in Patchset 5

Comment 8 Steffen Pingel 2012-02-21 15:04:43 EST

Created attachment 211361 [details]
screenshot

Comment 9 Steffen Pingel 2012-02-21 15:07:13 EST

Changes look good to me. I have put this on the agenda for the next meeting to do a UI review.

Comment 10 Steffen Pingel 2012-02-24 12:59:57 EST

One suggestion that came up on the UI review was to move the "Validate on Finish" button next to the validate button but there may not be enough horizontal space when the page is embedded in a multi-page wizard that has previous/next buttons. 

Shawn raised a concern that if the page had a scroll bar the "Validate on Finish" may not be visible and users may not understand how to exit the wizard.

Overall we decided to leave the current UI design for now.

Comment 11 Frank Becker 2012-02-24 13:14:04 EST

Code from review is now in HEAD so we can close this.

Comment 12 Frank Becker 2012-02-24 13:14:06 EST

Created attachment 211592 [details]
mylyn/context/zip

Comment 13 Steffen Pingel 2012-03-08 13:34:51 EST

I noticed that the Finish button is still enabled on Gtk during automatic validation (see screenshot).

Comment 14 Steffen Pingel 2012-03-08 14:06:37 EST

Created attachment 212322 [details]
enabled finish button

Comment 15 Frank Becker 2012-03-08 23:48:56 EST

(In reply to comment #13)
> I noticed that the Finish button is still enabled on Gtk during automatic
> validation (see screenshot).

This was not only on Gtk. I creat review http://review.mylyn.org/363

Comment 16 Frank Becker 2012-03-08 23:49:12 EST

Created attachment 212354 [details]
mylyn/context/zip

Comment 17 Steffen Pingel 2012-03-09 17:44:54 EST

Frank, can you reliably reproduce the problem? We can apply a work-around similar to what you have suggested but I would prefer if we fixed the actual enablement of the button.

Comment 18 Steffen Pingel 2012-03-09 17:53:24 EST

Alternative proposal for a fix: http://review.mylyn.org/366.

Comment 19 Steffen Pingel 2012-03-10 06:21:07 EST

I have committed the fix proposed in comment 18.

Comment 20 Frank Becker 2012-03-10 09:34:57 EST

(In reply to comment #19)
> I have committed the fix proposed in comment 18.
Steffen,

I did not see how this can fix the problem.

WizardDialog.updateButton set the finish button enable/disable with the result from wizard.canFinish(). 

wizard.canFinish()
 call EditRepositoryWizard.canFinish()
   call BugzillaRepositorySettingPage.isPageComplete()

That was the reason for my change.

When I use your review http://review.mylyn.org/366 I get an enabled Finish button during validate on my MacBook Pro

Comment 21 Steffen Pingel 2012-03-10 16:00:40 EST

Thanks for clarifying. The fix does not address the enabled finish button but it avoids the problem of running the Finish job in parallel several times. I have opened bug 373877 to fix the button enablement.