Created attachment 93196 [details]
Module which demonstrates the problem
For direct compatibility with TreeTableView, we retained the setProperties(Property) method for assigning the >0 column contents.
However, this results in two very awkward things:
1. If you don't have a Node yet, you need to write a subclass of PropertySupport.ReadOnly, just to pass the property name and type in - very non-obvious and wasteful of classes.
2. There is no practical reason why a column must contain only objects of the same type, yet exact type matching between the prototype property and the real property is enforced.
Say you have a Node which a property which is an Enum; and the Node has another property that will determine what concrete subclass of Enum will be used. The property sheet will handle this fine and display the new property. In the OutlineView, the column will be blank.
3. Type matching, if necessary at all, ought to handle supertypes. If I need a prototype property, it ought to be possible to create a prototype property with its value set to Object as a wildcard. In fact, this is not possible - so you cannot display a property in the UI unless the UI code knows the *exact* type (not, say, an API-level supertype) of the class. This encourages more coupling than necessary.
I suggest that setProperties() be deprecated, and a simpler
public void setPropertyNames(String...)
be used as a replacement.
I'm attaching a module which demonstrates the problem. Note two things:
- The listening on the root node in the TopComponent should not be necessary
- The prototype property class should not be necessary (in this case, in fact, it isn't; in the real world, often you don't have the root node when constructing the UI)
- A single PropertySupport.ReadWrite<T extends Enum> class should be usable for both nested Property subclasses in MyNode
Created attachment 93202 [details]
Patch w/ tests & api change notes
The attached patch adds the following mutator methods for the set of columns:
public void addPropertyColumn(String name, String displayName)
public void removePropertyColumn(String name)
public void setPropertyColumns(String... namesAndDisplayNames)
Under the hood, these use an internal Property subclass which does its equals() matching only using getName(), to avoid a common class of problem (prototype property value type not an exact match for real property).
I believe this addresses all of the problems. Admittedly, passing an array of alternating programmatic name, display name, programmatic name, display name in setPropertyColumns is a little ugly, I think less so than adding yet another "property descriptor thing" to the API.
Created attachment 93203 [details]
Considerably shorter and clearer version of the original module using this patch
BTW, given further thought, points 2. and 3. aren't quite valid, since if you override equals() and hashCode() on your prototype property subclass to only test the property name, it will have the expected effect. Nonetheless, you need to know to do this, and why are we making people write Property subclasses just to aggregate a name and display name and possibly a type, anyway?
No objections during review period, implementing.
Fixed in main/ a183d81ffa42
Just yesterday, while struggling with OutlineView, I complained to myself who could design API with such a poor usability (me and Jano R. did it for TreeTableView). Thanks for this improvement, it makes my code nicer a easier to understand:
Integrated into 'main-golden', will be available in build *201001240200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
User: Tim Boudreau <firstname.lastname@example.org>
Log: #179397 - eliminate need for prototype property objects to set columns of OutlineView