Bugzilla – Full Text Bug Listing
| Summary: | No SelectionChangeEvent on Graph.setSelection() | ||
|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Christian Sowada <eclipse> |
| Component: | GEF-Legacy Zest | Assignee: | Fabian Steeg <steeg> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | ahunter.eclipse, irbull, steeg |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=322054 | ||
| Whiteboard: | |||
(In reply to comment #0) I wrote some tests to reproduce the problem, and the proposed solution of calling fireWidgetSelectedEvent() from setSelection() and selectAll() looks good to me. It would also fix bug 322054, which is specifically about the selectAll() part. Ian, can I commit away on zest.core or would you prefer if I attached a patch? Reading up a bit on events in SWT it seems not sending events on programmatic selection changes is actually by design: http://www.eclipse.org/swt/faq.php#noeventfire So that probably means resolve as WORKSFORME or WONTFIX. OTOH, if we don't send events on programmatic selection changes, we should probably support programmatically notifying the listeners. Currently, calling notifyListeners() on a Graph with registered SelectionListeners has no effect. If I'm not mistaken, we want to be able to do something like this: graph.notifyListeners(SWT.Selection, event); I have tweaked the tests for this and implemented notifyListeners() to pass the event wrapped as a SelectionEvent to the listeners. Does this make any sense? Hi Fabian, you are a committer on the Dot4Zest component, not Zest proper. So before you commit anything, I would ask both you and Ian agree by email on gef-dev that everyone is OK with you being a committer on Zest proper. Just a formality I suppose, but Ian needs to confirm as component owner. (In reply to comment #4) I assumed that formally I already could commit on Zest proper, since my nomination was for Zest and dot4zest - but that I should ask before I break anything and not just commit, so I guess it comes down to the same thing :-) (In reply to comment #5) > (In reply to comment #4) > > I assumed that formally I already could commit on Zest proper, since my > nomination was for Zest and dot4zest - but that I should ask before I break > anything and not just commit, so I guess it comes down to the same thing :-) If everything works out, Fabian and I are actually going to see each other next week. We'll chat then, but having an extra Zest committer is actually a good thing, and Fabian has a good handle on the code. (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > > I assumed that formally I already could commit on Zest proper, since my > > nomination was for Zest and dot4zest - but that I should ask before I break > > anything and not just commit, so I guess it comes down to the same thing :-) > > If everything works out, Fabian and I are actually going to see each other next > week. We'll chat then, but having an extra Zest committer is actually a good > thing, and Fabian has a good handle on the code. Hi again, I agree 100%, more committers is good. I just wanted to make sure Ian agreed, and sounds like we are good. So this means go for committing the changes and tests? Ian, is it correct that when done, I change the status to RESOLVED - FIXED and reassign to you for verification? (In reply to comment #8) > So this means go for committing the changes and tests? > > Ian, is it correct that when done, I change the status to RESOLVED - FIXED and > reassign to you for verification? As I mentioned to Fabian in Germany last week, he can go ahead and release this. Fabian knows the Zest code as well as anybody, and he is already a GEF/Dot4Zest committer. Thanks Fabian. (In reply to comment #9) Awesome - OK, so programmatic notification can now be triggered in HEAD with: graph.notifyListeners(SWT.Selection, event); See GraphSelectionTests#testAddSelectionListenerNotifyListeners() for details. |
On one side I've registered a SelectionChangeListener on a Graph Object and on another side I've set the selection of the graph by a key event. But after the setSelection i get no SelectionChangeEvent, selecting by mouse works perfect. In our graph you can navigate withe the arrow keys to select the next GraphNode. I've fixed this by adding the fireWidgetSelectedEvent() in the loop of the setSelection() method. I think the same for selectAll() method. Here /** * Changes the selection to the list of items * * @param l */ public void setSelection(GraphItem[] nodes) { clearSelection(); if (nodes != null) { for (int i = 0; i < nodes.length; i++) { if (nodes[i] != null && nodes[i] instanceof GraphItem) { selectedItems.add(nodes[i]); (nodes[i]).highlight(); fireWidgetSelectedEvent(nodes[i]); } } } } I hope it's right. By the way, why so many methods are private? So I can't fix or extends classes.