I'm planning to refactor Tree.java by moving selected nodes from TreeExpansionModel into TreeSelectionModel. No changes in tree.js are expected. Please go ahead.
On Sun, Jul 3, 2011 at 6:55 PM, Josh Canfield <[email protected]>wrote: > Igor or Howard (or anyone else) are you actively working on the tree > component? I'd like to refactor the javascript a bit but I don't want > to make a merge for you difficult. > > Currently all of the javascript closures are being copied for every > node. In the TreeDemo integration test expanding the Numbers node, > which has 1000 elements , Chrome is reporting ~11.5MB added to the > total memory usage of the page. 4.78MB of that is in closures. (I > haven't tried profiling Firefox) > > I'm working on a project that will likely have a large number of > nodes, probably not all under one branch, but since the Tree component > is caching the objects opening and closing the branches is going to > make the total number of nodes pile up quickly. I'd like to try and > get it down to only increasing the memory usage by html elements and > the equivalent of the spec objects. > > > Josh > > > On Fri, Jul 1, 2011 at 5:32 PM, Igor Drobiazko <[email protected]> > wrote: > > You think we should add TreeSelectionModel? > > > > On Sat, Jul 2, 2011 at 2:02 AM, Howard Lewis Ship <[email protected]> > wrote: > > > >> I'm not sure that we want the TreeExpansionModel to also be the Tree's > >> selection model; I think of those as separate and possibly orthogonal. > >> > >> On Fri, Jul 1, 2011 at 1:24 PM, <[email protected]> wrote: > >> > Author: drobiazko > >> > Date: Fri Jul 1 20:24:03 2011 > >> > New Revision: 1142070 > >> > > >> > URL: http://svn.apache.org/viewvc?rev=1142070&view=rev > >> > Log: > >> > TAP5-1562: Made tree leafs selectable > >> > > >> > Modified: > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/EventConstants.java > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Tree.java > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/DefaultTreeExpansionModel.java > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/TreeExpansionModel.java > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.css > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.js > >> > > >> > > tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/TreeTests.groovy > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/EventConstants.java > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/EventConstants.java?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/EventConstants.java > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/EventConstants.java > >> Fri Jul 1 20:24:03 2011 > >> > @@ -209,4 +209,20 @@ public class EventConstants > >> > */ > >> > public static final String PREALLOCATE_FORM_CONTROL_NAMES = > >> "preallocateFormControlNames"; > >> > > >> > + /** > >> > + * Event triggered by the {@link > >> org.apache.tapestry5.corelib.components.Tree} > >> > + * component when a leaf node is selected. > >> > + * > >> > + * @since 5.3.1 > >> > + */ > >> > + public static final String NODE_SELECTED = "nodeSelected"; > >> > + > >> > + /** > >> > + * Event triggered by the {@link > >> org.apache.tapestry5.corelib.components.Tree} > >> > + * component when a leaf node is unselected. > >> > + * > >> > + * @since 5.3.1 > >> > + */ > >> > + public static final String NODE_UNSELECTED = "nodeUnselected"; > >> > + > >> > } > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Tree.java > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Tree.java?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Tree.java > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Tree.java > >> Fri Jul 1 20:24:03 2011 > >> > @@ -14,21 +14,17 @@ > >> > > >> > package org.apache.tapestry5.corelib.components; > >> > > >> > +import java.util.Arrays; > >> > import java.util.List; > >> > > >> > -import org.apache.tapestry5.BindingConstants; > >> > -import org.apache.tapestry5.Block; > >> > -import org.apache.tapestry5.ComponentResources; > >> > -import org.apache.tapestry5.Link; > >> > -import org.apache.tapestry5.MarkupWriter; > >> > -import org.apache.tapestry5.annotations.Environmental; > >> > -import org.apache.tapestry5.annotations.Parameter; > >> > -import org.apache.tapestry5.annotations.Persist; > >> > -import org.apache.tapestry5.annotations.Property; > >> > +import org.apache.tapestry5.*; > >> > +import org.apache.tapestry5.annotations.*; > >> > +import org.apache.tapestry5.corelib.internal.InternalFormSupport; > >> > import org.apache.tapestry5.dom.Element; > >> > import org.apache.tapestry5.func.F; > >> > import org.apache.tapestry5.func.Flow; > >> > import org.apache.tapestry5.func.Worker; > >> > +import org.apache.tapestry5.internal.util.CaptureResultCallback; > >> > import org.apache.tapestry5.ioc.annotations.Inject; > >> > import org.apache.tapestry5.json.JSONObject; > >> > import org.apache.tapestry5.runtime.RenderCommand; > >> > @@ -49,6 +45,7 @@ import org.apache.tapestry5.tree.TreeNod > >> > */ > >> > @SuppressWarnings( > >> > { "rawtypes", "unchecked", "unused" }) > >> > +@Events({EventConstants.NODE_SELECTED, > EventConstants.NODE_UNSELECTED}) > >> > public class Tree > >> > { > >> > /** > >> > @@ -162,30 +159,34 @@ public class Tree > >> > boolean hasChildren = !node.isLeaf() && > >> node.getHasChildren(); > >> > boolean expanded = hasChildren && > >> expansionModel.isExpanded(node); > >> > > >> > - if (hasChildren) > >> > - { > >> > - String clientId = > jss.allocateClientId(resources); > >> > + String clientId = jss.allocateClientId(resources); > >> > > >> > - e.attribute("id", clientId); > >> > + JSONObject spec = new JSONObject("clientId", > clientId); > >> > > >> > + e.attribute("id", clientId); > >> > + > >> > + if (hasChildren) > >> > + { > >> > Link expandChildren = > >> resources.createEventLink("expandChildren", node.getId()); > >> > Link markExpanded = > >> resources.createEventLink("markExpanded", node.getId()); > >> > Link markCollapsed = > >> resources.createEventLink("markCollapsed", node.getId()); > >> > > >> > - JSONObject spec = new JSONObject("clientId", > >> clientId, > >> > - > >> > - "expandChildrenURL", expandChildren.toString(), > >> > - > >> > - "markExpandedURL", markExpanded.toString(), > >> > - > >> > - "markCollapsedURL", markCollapsed.toString()); > >> > + spec.put("expandChildrenURL", > >> expandChildren.toString()) > >> > + .put( "markExpandedURL", > >> markExpanded.toString()) > >> > + .put("markCollapsedURL", > >> markCollapsed.toString()); > >> > > >> > if (expanded) > >> > spec.put("expanded", true); > >> > + } > >> > + else > >> > + { > >> > + Link toggleLeaf = > >> resources.createEventLink("toggleLeaf", node.getId()); > >> > > >> > - jss.addInitializerCall("treeNode", spec); > >> > + spec.put("toggleLeafURL", toggleLeaf.toString()); > >> > } > >> > > >> > + jss.addInitializerCall("treeNode", spec); > >> > + > >> > writer.end(); // span.tx-tree-icon > >> > > >> > // From here on in, we're pushing things onto the > queue. > >> Remember that > >> > @@ -263,6 +264,37 @@ public class Tree > >> > return new JSONObject(); > >> > } > >> > > >> > + Object onToggleLeaf(String nodeId) > >> > + { > >> > + TreeNode node = model.getById(nodeId); > >> > + > >> > + String event; > >> > + > >> > + if(expansionModel.isSelected(node)) > >> > + { > >> > + expansionModel.unselect(node); > >> > + > >> > + event = EventConstants.NODE_UNSELECTED; > >> > + } > >> > + else > >> > + { > >> > + expansionModel.select(node); > >> > + > >> > + event = EventConstants.NODE_SELECTED; > >> > + } > >> > + > >> > + CaptureResultCallback<Object> callback = > >> CaptureResultCallback.create(); > >> > + > >> > + resources.triggerEvent(event, new Object [] { nodeId }, > >> callback); > >> > + > >> > + final Object result = callback.getResult(); > >> > + > >> > + if(result != null) > >> > + return result; > >> > + > >> > + return new JSONObject(); > >> > + } > >> > + > >> > public TreeExpansionModel getDefaultTreeExpansionModel() > >> > { > >> > if (defaultTreeExpansionModel == null) > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/DefaultTreeExpansionModel.java > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/DefaultTreeExpansionModel.java?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/DefaultTreeExpansionModel.java > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/DefaultTreeExpansionModel.java > >> Fri Jul 1 20:24:03 2011 > >> > @@ -30,34 +30,73 @@ public class DefaultTreeExpansionModel<T > >> > { > >> > private final Set<String> expandedIds = > CollectionFactory.newSet(); > >> > > >> > + private final Set<String> selectedIds = > CollectionFactory.newSet(); > >> > + > >> > public boolean isExpanded(TreeNode<T> node) > >> > { > >> > - assert node != null; > >> > - > >> > - return expandedIds.contains(node.getId()); > >> > + return contains(expandedIds, node); > >> > } > >> > > >> > public void markExpanded(TreeNode<T> node) > >> > { > >> > + add(expandedIds, node); > >> > + } > >> > + > >> > + public void markCollapsed(TreeNode<T> node) > >> > + { > >> > + remove(expandedIds, node); > >> > + } > >> > + > >> > + public boolean isSelected(TreeNode<T> node) > >> > + { > >> > + return contains(selectedIds, node); > >> > + } > >> > + > >> > + public void select(TreeNode<T> node) > >> > + { > >> > + add(selectedIds, node); > >> > + } > >> > + > >> > + public void unselect(TreeNode<T> node) > >> > + { > >> > + remove(selectedIds, node); > >> > + } > >> > + > >> > + public void clear() > >> > + { > >> > + clearSet(expandedIds); > >> > + > >> > + clearSet(selectedIds); > >> > + } > >> > + > >> > + private void add(Set<String> ids, TreeNode<T> node) > >> > + { > >> > assert node != null; > >> > > >> > - if (expandedIds.add(node.getId())) > >> > + if (ids.add(node.getId())) > >> > markDirty(); > >> > } > >> > > >> > - public void markCollapsed(TreeNode<T> node) > >> > + private void remove(Set<String> ids, TreeNode<T> node) > >> > { > >> > assert node != null; > >> > > >> > - if (expandedIds.remove(node.getId())) > >> > + if (ids.remove(node.getId())) > >> > markDirty(); > >> > } > >> > > >> > - public void clear() > >> > + private boolean contains(Set<String> ids, TreeNode<T> node) > >> > + { > >> > + assert node != null; > >> > + > >> > + return ids.contains(node.getId()); > >> > + } > >> > + > >> > + private void clearSet(Set<String> set) > >> > { > >> > - if (!expandedIds.isEmpty()) > >> > + if (!set.isEmpty()) > >> > { > >> > - expandedIds.clear(); > >> > + set.clear(); > >> > markDirty(); > >> > } > >> > } > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/TreeExpansionModel.java > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/TreeExpansionModel.java?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/TreeExpansionModel.java > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/tree/TreeExpansionModel.java > >> Fri Jul 1 20:24:03 2011 > >> > @@ -42,6 +42,12 @@ public interface TreeExpansionModel<T> > >> > /** Marks the node as collapsed (not expanded). */ > >> > void markCollapsed(TreeNode<T> node); > >> > > >> > + boolean isSelected(TreeNode<T> node); > >> > + > >> > + void select(TreeNode<T> node); > >> > + > >> > + void unselect(TreeNode<T> node); > >> > + > >> > /** Marks all nodes as collapsed. */ > >> > void clear(); > >> > } > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.css > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.css?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.css > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.css > >> Fri Jul 1 20:24:03 2011 > >> > @@ -51,6 +51,10 @@ SPAN.t-tree-icon.t-leaf-node { > >> > background-position: -32px -16px; > >> > } > >> > > >> > +SPAN.t-tree-label.t-selected-leaf-node-label { > >> > + font-weight: bold; > >> > +} > >> > + > >> > SPAN.t-tree-icon.t-empty-node { > >> > cursor: default; > >> > background-position: -32px 0px !important; > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.js > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.js?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.js > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tree.js > >> Fri Jul 1 20:24:03 2011 > >> > @@ -63,6 +63,8 @@ T5.extendInitializer(function() { > >> > function initializer(spec) { > >> > var loaded = spec.expanded; > >> > var expanded = spec.expanded; > >> > + var selected = false; > >> > + > >> > if (expanded) { > >> > > $(spec.clientId).addClassName("t-tree-expanded") > >> > } > >> > @@ -95,16 +97,41 @@ T5.extendInitializer(function() { > >> > > >> > } > >> > > >> > + function toggleLeafHandler(reply) { > >> > + var response = reply.responseJSON; > >> > + > >> > + $(spec.clientId).update(""); > >> > + > >> > + Tapestry.loadScriptsInReply(response, function() { > >> > + loading = false; > >> > + loaded = true; > >> > + expanded = true; > >> > + selected = !selected; > >> > + }); > >> > + } > >> > + > >> > function doLoad() { > >> > if (loading) > >> > return; > >> > > >> > loading = true; > >> > > >> > - $(spec.clientId).addClassName("t-empty-node"); > >> > + if(spec.expandChildrenURL) > >> > + { > >> > + > >> $(spec.clientId).addClassName("t-empty-node"); > >> > + } > >> > + else > >> > + { > >> > + > >> > > $(spec.clientId).next("span.t-tree-label").addClassName("t-selected-leaf-node-label"); > >> > + } > >> > $(spec.clientId).update("<span > >> class='t-ajax-wait'/>"); > >> > > >> > - Tapestry.ajaxRequest(spec.expandChildrenURL, > >> successHandler); > >> > + var requestURL = spec.expandChildrenURL? > >> spec.expandChildrenURL:spec.toggleLeafURL; > >> > + > >> > + var handler = spec.expandChildrenURL? successHandler: > >> toggleLeafHandler; > >> > + > >> > + Tapestry.ajaxRequest(requestURL, handler); > >> > + > >> > } > >> > > >> > $(spec.clientId).observe("click", function(event) { > >> > @@ -117,6 +144,25 @@ T5.extendInitializer(function() { > >> > return; > >> > } > >> > > >> > + if(spec.toggleLeafURL) > >> > + { > >> > + var label = > $(spec.clientId).next("span.t-tree-label"); > >> > + > >> > + if(selected) > >> > + { > >> > + > label.removeClassName("t-selected-leaf-node-label"); > >> > + } > >> > + else > >> > + { > >> > + > label.addClassName("t-selected-leaf-node-label"); > >> > + } > >> > + selected = !selected; > >> > + > >> > + Tapestry.ajaxRequest(spec.toggleLeafURL, {}); > >> > + > >> > + return; > >> > + } > >> > + > >> > // Children have been loaded, just a matter of > >> toggling > >> > // between showing or hiding the children. > >> > > >> > > >> > Modified: > >> > tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/TreeTests.groovy > >> > URL: > >> > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/TreeTests.groovy?rev=1142070&r1=1142069&r2=1142070&view=diff > >> > > >> > ============================================================================== > >> > --- > >> > tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/TreeTests.groovy > >> (original) > >> > +++ > >> > tapestry/tapestry5/trunk/tapestry-core/src/test/groovy/org/apache/tapestry5/integration/app1/TreeTests.groovy > >> Fri Jul 1 20:24:03 2011 > >> > @@ -19,10 +19,6 @@ import org.testng.annotations.Test > >> > > >> > class TreeTests extends SeleniumTestCase > >> > { > >> > - /** > >> > - * Haven't figured out how to get Selenium to click the actual > >> elements, never mine > >> > - * coordinate the wait for the Ajax. So far its just been manual > >> testing. > >> > - */ > >> > @Test > >> > void basics() { > >> > > >> > @@ -32,20 +28,41 @@ class TreeTests extends SeleniumTestCase > >> > > >> > clickAndWait "link=clear expansions" > >> > > >> > - if (false) { > >> > - click "//span[@class='t-tree-icon'][2]" > >> > + //Click on Games > >> > + click "//div[@class='t-tree-container > >> test-hook']/ul/li[2]/span[@class='t-tree-icon']" > >> > > >> > - sleep 100 > >> > + waitForCSSSelectedElementToAppear "span.t-tree-expanded" > >> > > >> > - click "//span[@class='t-tree-icon'][3]" > >> > + assertTextPresent "Board Games" > >> > > >> > - sleep 100 > >> > + //Click on Board Games > >> > + click "//div[@class='t-tree-container > >> test-hook']/ul/li[2]/ul/li/span[@class='t-tree-icon']" > >> > > >> > - assertTextPresent "Agricola" > >> > + //Assert the leafs are displayed > >> > + waitForCondition "window.\$\$(\"span.t-leaf-node\").size() >= > >> 5", "45000" > >> > > >> > - clickAndWait "link=Redraw" > >> > + clickAndWait "link=Redraw" > >> > > >> > - assertTextPresent "Agricola" > >> > - } > >> > + assertTextPresent "Settlers of Catan", "Agricola" > >> > + } > >> > + > >> > + @Test > >> > + void selectLeaf() { > >> > + > >> > + openBaseURL() > >> > + > >> > + clickAndWait "link=Tree Component Demo" > >> > + > >> > + clickAndWait "link=clear expansions" > >> > + > >> > + click "//span[@class='t-tree-icon']" > >> > + > >> > + waitForCSSSelectedElementToAppear "span.t-leaf-node" > >> > + > >> > + assertTextPresent "Oscar", "Gromit", "Max", "Roger", "Cooper" > >> > + > >> > + click "//span[@class='t-tree-icon t-leaf-node']" > >> > + > >> > + waitForCSSSelectedElementToAppear > >> "span.t-selected-leaf-node-label" > >> > } > >> > } > >> > > >> > > >> > > >> > >> > >> > >> -- > >> Howard M. Lewis Ship > >> > >> Creator of Apache Tapestry > >> > >> The source for Tapestry training, mentoring and support. Contact me to > >> learn how I can get you up and productive in Tapestry fast! > >> > >> (971) 678-5210 > >> http://howardlewisship.com > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [email protected] > >> For additional commands, e-mail: [email protected] > >> > >> > > > > > > -- > > Best regards, > > > > Igor Drobiazko > > http://tapestry5.de > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > -- Best regards, Igor Drobiazko http://tapestry5.de
