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

Bug 484774

Summary:	Adopt JavaFX properties for all GEF4 components
Product:	[Tools] GEF	Reporter:	Alexander Nyßen <nyssen>
Component:	GEF Common	Assignee:	gef-inbox <gef-inbox>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	matthias.wienand
Version:	0.2.0
Target Milestone:	4.0.0 / 3.11.0 (Neon) M5
Hardware:	All
OS:	All
Whiteboard:

Description Alexander Nyßen

2015-12-21 13:49:09 EST

While GEF4 FX makes use of JavaFX properties, GEF4 Graph, Layout, and the non JavaFX-related parts of MVC are based on simple property change notifications. Those modules that dependent on non JavaFX-related parts but provide JavaFX dependencies somehow provide a mixture (e.g. MVC.FX, Zest.FX). 

As property change notification are not type-safe (no generics supported) and as we have adopted JavaSE-1.7 BREE throughout all our modules already, we should adopt JavaFX properties, which are not toolkit dependent, for all our modules.

There are two prerequisites that are needed to make this feasible. First, we will have to provide own property implementations for properties based on SetMultimap and Multiset (which are used in MVC). Further we will have to add support for delaying notifications (which is also needed in MVC).

Comment 1 Alexander Nyßen

2016-01-14 10:22:41 EST

I pushed the following changes to origin/bug_484774:

- Add ObservableSetMultimap and related SetMultimapChangeListener, as well as SetMultimapChangeListenerHelper support class. Add ObservableMultiset and related MultimapChangeListener, as well as MultisetChangeListenerHelper support class.
- Add ObservableSetMultimapWrapper and ObservableMultisetWrapper, which can be used to easily create ObservableSetMultimap and ObservableMultiset instances.
- Add SimpleSetMultimapProperty and ReadOnlySetMultimapWrapper, as well as SimpleMultisetProperty and ReadOnlyMultisetWrapper to realize writable and read-only property support. 
- Moved ReadOnlyMapPropertyEx to org.eclipse.gef4.common.beans.property, changed it to also fix problems related to unbinding of BidirectionalBindings related to it, and added ReadOnlySetPropertyEx and ReadOnlyListPropertyEx to fix the same problem.
- Added SimpleMapPropertyEx to fix the listener issue also holding for ReadOnlyMapPropertyEx.
- Add test cases for ObservableSetMultimap and ObservableMultiset, related properties, and for replacement classes.

Having reconsidered my earlier statement I think delaying notifications is not required. We should expose read-only property instead on those places where we need full control about the point in time of manipulation. 

As such, we now have everything in place to adopt JavaFX properties as a replacement of IPropertyChangeNotifier.

Comment 2 Matthias Wienand

2016-01-21 12:08:04 EST

The following changes have been pushed to origin/bug_484774:

- Ensure AbstractAnchor correctly (un-)registers VCLs on anchored Scene changes.
- Add UnmodifiableMultimapWrapper and UnmodifiableSetMultimapWrapper.
- Add Bindings2 and extract logic from properties.
- Fixed that clickable area shape was not properly registered at/unregistered from visual part map (leading to problems related to selection of connections).
- Added doActivate/doDeactivate to AbstractBehavior to guard behaviors against repeated activation/deactivation.
- Introduced activateChilren/deactivateChildren callbacks so doActivate() and doDeactivate() can be safely overwritten without affecting the IVisualPart contract.
- Fix hover handle construction after activation. A false flag prevented the construction of the handles.
- Use immutable copies are for the initial state (e.g. "anchorages", "children") within the operations.
- Ensure hover handles are removed when its host is removed.
- Remove FXCollections.unmodifiableSet(...) call (Java 8). Create UnmodifiableObservableSetWrapper in GEF4 Common and use it within HidingModel instead of the JavaFX 8 API.

Therefore, the IPropertyChangeNotifier is now completely replaced with JavaFX properties. The code is published on the master branch, therefore, I resolve this ticket as fixed for 3.11.0M5.

However, we are not completely satisfied with the new API, yet. The remaining issues have been summarized in Bug #486294.