Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 334835

Summary:

Remove cancel button from project facets dialog

Product:

[WebTools] WTP Common Tools

Reporter:

Roberto Sanchez Herrera <shr31223>

Component:

Faceted Project Framework

Assignee:

Konstantin Komissarchik <konstantin>

Status:

RESOLVED FIXED

QA Contact:

Konstantin Komissarchik <konstantin>

Severity:

normal

Priority:

CC:

ccc, neil.hauge, thatnitind

Version:

3.2

Flags:

ccc: pmc_approved? (david_williams)
ccc: pmc_approved? (raghunathan.srinivasan)
ccc: pmc_approved? (naci.dai)
ccc: pmc_approved? (deboer)
neil.hauge: pmc_approved+
ccc: pmc_approved? (kaloyan)
ccc: review+

Target Milestone:

3.2.3

Hardware:

All

OS:

All

Whiteboard:

PMC_approved

Attachments:

Description	Flags
Disable Cencel button	none
Patch v2	none

Description Roberto Sanchez Herrera

2011-01-19 16:11:54 EST

Build Identifier: WTP M-3.2.3-20110113194837

This is a clone of bug 334438 for 3.2.3

The Cancel button in the Project Facets dialog does not revert changes when
pressed, so the result is the same as if OK is pressed. This behavior is
misleading, so disabling the Cancel button is proposed.

More details can be found here: 
http://www.eclipse.org/forums/index.php?t=rview&goto=639262#msg_639262

Reproducible: Always

Steps to Reproduce:
1.Open any new project wizard (new dynamic web project, EJB project, etc.)
2.Click on Modify...
3.Select any facet
4.Click Cancel. 
5.Click on Modify... again. The facet you selected in step 3 is still selected.

Reproducible: Always

Steps to Reproduce:
1.Open any new project wizard (new dynamic web project, EJB project, etc.)
2.Click on Modify...
3.Select any facet
4.Click Cancel. 
5.Click on Modify... again. The facet you selected in step 3 is still selected.

Comment 1 Roberto Sanchez Herrera

2011-01-19 16:19:51 EST

Created attachment 187151 [details]
Disable Cencel button

Comment 2 Carl Anderson

2011-01-19 16:45:18 EST

PMC Review requested due to "UI change" (although it really isn't).

    * Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 

While not technically a hotbug, an adopter (IBM) would like this to be fixed to improve the overall usability experience.

    * Is there a work-around? If so, why do you believe the work-around is insufficient?

There is no workaround.  Currently, the Cancel button does not do anything- this lulls the user into a false sense of security (that the changes are undo-able), whereas, in reality, they are not.

    * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added?

This has been tested by hand.  It is also in the WTP 3.3 code stream already.

    * Give a brief technical overview. Who has reviewed this fix?

Both Konstantin and myself have reviewed Roberto's fix.  The fix is to simply not enable the cancel button, thus demonstrating to the user that changes cannot be undone.

    * What is the risk associated with this fix?

This is an extremely low risk fix.

Comment 3 Konstantin Komissarchik

2011-01-23 20:01:45 EST

Created attachment 187392 [details]
Patch v2

Nitin has proposed a better solution that removes rather than disables the cancel button. Nitin's solution has been released to HEAD. If we are considering addressing this for 3.2.x, might as well consider applying the final solution.

Carl's discussion still applies. The only change is that the cancel button is not there instead of being disabled.

Comment 4 Konstantin Komissarchik

2011-01-23 20:05:05 EST

Forgot to reference Nitin's patch to HEAD... Bug 334844.

Comment 5 Roberto Sanchez Herrera

2011-01-26 16:14:16 EST

Hello, just wondering if this is going to be included in 3.2.3.
Thank you.

Comment 6 Konstantin Komissarchik

2011-01-27 03:32:17 EST

Not sure how many PMC votes are required for this to go forward...

Comment 7 Roberto Sanchez Herrera

2011-01-27 09:54:30 EST

Until yesterday, only 1 vote was necessary. Now I believe 2 votes are needed.

Comment 8 Konstantin Komissarchik

2011-01-27 12:18:47 EST

> Until yesterday, only 1 vote was necessary. Now I believe 2 votes are needed.

I don't know that that's correct since we are dealing both with RC requirements and UI change requirements. It would have been nice to get some clarity from someone on the PMC on this issue...

In any case, since there appear to be people anxiously waiting for this to go in, I took the risk of releasing the patch to 3.2.3 stream. If someone objects, we can revert.

Carl, I am leaving it up to you to decide whether to ask for a respin or to pick this up in next week's integration build.

Comment 9 Konstantin Komissarchik

2011-01-27 12:19:10 EST

Meant to mark it as fixed.

Comment 10 Neil Hauge

2011-01-27 12:38:13 EST

(In reply to comment #8)
> > Until yesterday, only 1 vote was necessary. Now I believe 2 votes are needed.
> 
> I don't know that that's correct since we are dealing both with RC requirements
> and UI change requirements. It would have been nice to get some clarity from
> someone on the PMC on this issue...

I assumed this was still a project level discussion (regarding what to do), but I'm happy to chime in.  I think the second solution is preferable in general and also acceptable for 3.2.3 given the context. Assuming this is the preferred direction of the project, I reiterate my +1 for the second patch.

If the patch gets into this week's build then I think we are done here.  If the patch goes into next week's build then it will need an additional +1 from the PMC.