Bugzilla – Full Text Bug Listing
| Summary: | Strange looking 'Delete Server' dialog with strange behaviour | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP ServerTools | Reporter: | Stefan Dimov <stefan.dimov> | ||||||
| Component: | wst.server | Assignee: | Elson Yuen <eyuen7> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | Elson Yuen <eyuen7> | ||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | dimitar.giormov, kaloyan, stryker | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | 3.2.5 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows Vista | ||||||||
| See Also: | https://git.eclipse.org/r/109065 | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
Ok, I've started to look at the code and now it seems more confusing. Appears that DeleteServerDialog accepts in its constructor an array of IServer objects. Now the purpose of the checkbox became clear. If it's checked then the frameworks is supposed to delete the running servers along with the stopped. If it's not - only the stopped. Now it makes sense, but since we're four people here, which got confused, seems that the text of the dialog is not clear enough. Yes, the check box text says 'Delete Running Server(s)', but it's not clear that the initial idea is to remove the stopped servers only. So, the text of the dialog should be more explanatory. It's especially confusing if the user selects one server only. Now it becomes more confusing. All of the above makes sense if the user is able to delete multiple servers at once, but when the user selects more than one server in the view, the action 'Delete' disappears from the context menu and the Delete button doesn't work anymore. So, it seems that this dialog can be seen only if the user has selected and tries to delete one server only. So I see a few options here: 1. Allow deletion of multiple servers at once and make the text of the dialog more clear. In this case I still believe that the 'Delete running servers' should disappear in case there is only one server selected, because it's confusing. 2. Remove the checkbox completely and delete all the selected servers no matter if they're stopped or running. Since I don't know which one is preferrable, I'm going to propose a patch and will stick to the 'common denominator' of the two options - the patch won't enable multiple deletion, but it will remove the checkbox in case there is only one server selected. It's up to you to decide if you wanna go for one of the options. Created attachment 198077 [details]
patch
I can think of one more option which: - allows multiple deletion of servers - Instead of 'Delete running servers' checkbox has 'Delete stopped servers only' - unchecked by default. - in case of one server selected - no checkbox for running/stopped servers Now that Indigo is released, would you see into this bug? Elson since you are working in some of the plans now, can we look into this one for 3.3.1? Elson, can you comment on the patch. Is it ok to have the fix for 3.3.1? Thanks for submitting the patch. I would take the route of not showing the checkbox when there is only one server selected. Created attachment 201581 [details]
v1.0
The logic of the patch looks good. Based on the submitted patch, I make a small change to not indenting the stop running server before delete checkbox if the delete running server checkbox does not show.
Changes released to 32M, 33M and HEAD I'd like to note that this is somewhat related to Bug 345087 which involves allowing the delete action to work on several servers. New Gerrit change created: https://git.eclipse.org/r/109065 |
The 'Delete Server' dialog (org.eclipse.wst.server.ui.internal.DeleteServerDialog) is looking strange. It has two check-boxes. The first one is 'Delete Running Server(s)'. I think this checkbox is redundant. If the user chose the 'Delete' action and then presses 'Ok' of the dialog, it should be enough to verify that she wants this server to be deleted from the 'Servers' view. Why is necessary to confirm this intention additionaly by checking some check box? And if the user unchecks it and presses 'OK' the behavior is more strange - nothing happens. I will propose a patch which will remove the 'Delete Running Server(s)' checkbox.