Wicket tree improvement
-----------------------
Key: WICKET-1600
URL: https://issues.apache.org/jira/browse/WICKET-1600
Project: Wicket
Issue Type: Improvement
Components: wicket
Affects Versions: 1.4-M1
Reporter: Sven Meier
Priority: Minor
I see the following issues with Wicket's tree implementation that should be
solved:
AbstractTree and its subclasses were written with the Swing JTree API in mind.
This is not a bad thing per se, but the JTree abstractions are not very well
suited for a web application. Matej recently removed some of these
dependencies, but there's still a lot of code that uses TreeNode,
TreeModelEvent and such.
AbstractTree holds a TreeModel in its model, attaching a listener to it. This
is an unusual approach for a Wicket component:
- It implies that changes in the TreeModel are automatically propagated to the
user interface. This is not always the case, as in an ajax request the
AbstractTree has to be explicitely notified to update itself.
- It requires the AbstractTree to monitor the model for a replaced TreeModel.
Most importantly the TreeModel API is complicated and ambiguous (just see the
javadoc of TreeModelEvent and TreeModelListener) which makes life harder
especially for those who want to use AbstractTree with their own TreeModel
implementation (which is difficult enough). Although not directly visible in
the API, an implementor has to privide the parent of a node - see
AbstractTree#getParentNode(). Tree listeners and events are an annoyance which
doesn't match Wicket's elegance.
Currently many components in the AbstractTree hierarchy hold a reference to
real nodes of the TreeModel (e.g. junctionLink). TreeState seems like a foreign
concept to Wicket, holding references to nodes too. To support detachment
AbstractTree tests wether a node implements IDetachable, effectively
duplicating functionality that is normally provider by Component and its model
out of the box.
The nodes are currently used to identify state (e.g. the parent) in the tree.
To add a node, the nodes have to implement equals() and hashCode() to be
correctly identified in the tree state. This might be unacceptable, e.g. if
entities (business objects) are used as nodes, probably loaded from a database.
Therefore I propose to refactor AbstractTree and subclasses as follows:
A new interface INodeProvider (similar to IDataProvider) defines a concise API
to define a tree of nodes. The method #model(Object) gives an implementor the
possibility to wrap nodes in a suitable model, e.g. a detachable one. The
provided model can handle #equal() and #hashcode() for identification of nodes
in the tree.
References to nodes are always indirect through the model, never to the real
node object.
Handling of node parents is completely managed inside AbstractTree, no need for
an ExtendedTreeModel.
The model of an AbstractTree is used for node selection, similar to
AbstractChoices.
Listeners are replaced with notification methods on AbstractTree. No need for
tree paths on changes.
Expansion state is held in the AbstracTree#TreeItems (instead of former
ITreeState) - similar to component visibility.
The attached patch tries to achieve all this (or at least show how a solution
could look like), additionally:
- AbstractTree now utilizes AbstractRepeater and
#setOutputMarkupPlaceholderTag() instead of custom solutions.
- The examples are modified and enhanced with 'add' and 'remove' functionality,
org.apache.wicket.examples.nested.Home shows usage of an Swing TreeModel
adapter.
- Lots of generics are added (Swing's TreeModel will probably never be generic).
- The code is (hopefully) simplified.
I apologies for putting all this stuff into a single patch, but I wasn't able
to split these issues apart - it all depends on the proposed INodeProvider
interface.
For users of AbstractTree this solution has the following impacts:
- minor API changes for subclasses
- if a node is collapsed, all state of descendants will be lost, i.e. if the
node is expanded once again, all previously expanded children will stay
collapsed (similar to how Gnome handles trees)
- optimization for adding multiple new children in one AJAX request is not
(yet) supported, this was part of the former custom markup handling.
I will continue working on AbstractTree with tests and documentation. But
before doing that, I'd like to get a feedback from wicket committers
(especially Matej), if this proposed solution matches their conception.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
