[
https://issues.apache.org/jira/browse/WICKET-1600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735733#action_12735733
]
Eelco Hillenius commented on WICKET-1600:
-----------------------------------------
I'm still not too bothered by the fact that Wicket's tree uses some Swing
classes. When writing the initial implementation, I actually had the
requirement of being able to reuse the model in a Swing application, and that
worked well back then.
That said, I'm not against a more straightforward approach either. There are
several things to not like in this tree's implementation and some of the Swings
classes. However, the real bottleneck with our tree and I presume the rewrite
as well, has to do with the fact that we send Ajax updates using markup. With a
larger tree this results in really large updates (100k+ for a single Ajax
request). A smarter tree build from the ground up would only send node deltas
and let the tree figure out after an initial render how to go about displaying
that. Some loss of flexibility maybe, but for a huge efficiency gain.
> 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
> Assignee: Matej Knopp
> Priority: Minor
> Fix For: 1.5-M1
>
> Attachments: tree.diff
>
>
> 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.
