Bug 273654 - [target] Allow multiselection and delete key on target location tab
Summary: [target] Allow multiselection and delete key on target location tab
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.5   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.6 M1   Edit
Assignee: Curtis Windatt CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
Depends on:
Blocks:
 
Reported: 2009-04-24 17:21 EDT by Curtis Windatt CLA
Modified: 2009-08-04 10:56 EDT (History)
2 users (show)

See Also:


Attachments
Patch (2.32 KB, patch)
2009-05-07 08:17 EDT, Ankur Sharma CLA
no flags Details | Diff
Patch with KeyListner (4.55 KB, patch)
2009-05-11 16:11 EDT, Ankur Sharma CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Curtis Windatt CLA 2009-04-24 17:21:35 EDT
Right now on the locations tab of the target editor/wizard you only get single selection.  There is a removeall button, but out of habit I always hit Ctrl-A, Del to remove things (and sometimes I want to remove one or two specific containers).

Adding this should be an easy polish item, we just have to be careful in the button handling/enablement.
Comment 1 Ankur Sharma CLA 2009-05-07 08:17:38 EDT
Created attachment 134771 [details]
Patch

Added SWT.MULTI to the tree. This enabled Ctrl + A automatically.
Comment 2 Darin Wright CLA 2009-05-07 09:26:50 EDT
Thanks Ankur. Applied patch. Curtis, please verify.
Comment 3 Curtis Windatt CLA 2009-05-07 11:03:34 EDT
Reopening

1) Ctrl-A works, but the delete key doesn't.

2) Hitting the remove button with a multi-selection only removes the first selected entry.

3) There is no need for a remove all button

4) Remove button enablement is broken, it only checks part of what is selected.  I only found this noticeable when I had error statuses showing as children of the locations.  Note that the edit button's enablement has the same problems.  The enablement code assumed that the tree was single selection.
Comment 4 Darin Wright CLA 2009-05-07 11:12:36 EDT
Oops, I was testing the wrong page... I was testing the pref page not the locations tab... My mistake.
Comment 5 Chris Aniszczyk CLA 2009-05-07 15:20:26 EDT
Isn't this more of an enhancement?
Comment 6 Curtis Windatt CLA 2009-05-07 15:29:17 EDT
(In reply to comment #5)
> Isn't this more of an enhancement?
> 

I would describe this as a polish bug.  On the preference page and elsewhere in the IDE we support multi-selection.  This tree originally had a lot of mixed content so it was simpler to go with single selection.  Now this page primarily displays just locations and any other items in the tree can be ignored (they cannot be edited or removed).

This is not a large fix, though it is more than one line :)
Comment 7 Darin Wright CLA 2009-05-08 11:31:28 EDT
I reverted the MULTI setting since the fix was incomplete.
Comment 8 Chris Aniszczyk CLA 2009-05-09 18:31:17 EDT
RC1 is closing soon... are we going to punt on this?
Comment 9 Curtis Windatt CLA 2009-05-11 08:56:21 EDT
Yes, there are other things to worry about for 3.5.
Comment 10 Ankur Sharma CLA 2009-05-11 16:11:18 EDT
Created attachment 135210 [details]
Patch with KeyListner

This patch has multi selection, ctl+a for select all, del key for remove, all selected remove using del ley or remove button click.

However, I feel its incomplete without Undo and Redo functionality on target editor. But that is bigger chunk of work and has to go in 3.6 now.
Comment 11 Curtis Windatt CLA 2009-06-17 15:41:28 EDT
Ankur, if you want to look into undo/redo support for the TargetLocationGroup, cool, if not we will put this in for M1.
Comment 12 Curtis Windatt CLA 2009-06-30 16:02:27 EDT
Fixed in HEAD, see TargetLocationGroup.java

Had to redo the fix as the patch resulted in the tree being refreshed multiple times and didn't handle multi selection very well.
Comment 13 Darin Wright CLA 2009-08-04 10:56:53 EDT
Verified.