Skip to main content

This site requires you to update your browser. Your browsing experience maybe affected by not having the most up to date version.

Archive /

Our old forums are still available as a read-only archive.

Moderators: martimiz, Sean, biapar, Willr, Ingo, simon_w

Tree control tweaks


Go to End
Reply


15 Posts   6473 Views

Avatar
Steve

Community Member, 1 Post

17 June 2007 at 9:21am

I've been using the tree control with some *big* outlines (~5000 entries), and have shaken out a few nits:

* I made it about 30% faster for expand/collapse (simply by iterating over the LIs instead of over the spans)

* I added openall and closeall support

* I'm adding support for noticing if a server-side process has already kind put in all the spanA/spanB/spanC structure, and skip initTrees if so (doing this server-side saves about 20 seconds for the user in big cases).

I've also fixed what I think of as a bug, namely that when you click on a title, in addition to following the link, it also toggles the expansion there as if you'd clicked on the +/-.

I've also added internal documentation, since it took me a while to figure out some of the details of how it's doing what it does.

If this would be of interest, I'll be happy to send you my changes.

Steve

Avatar
elijahlofgren

Google Summer of Code Hacker, 222 Posts

18 June 2007 at 8:37am

Hi Steve,

These changes sound great! Those sort of improvements would be very useful for anyone who has a large website using SilverStripe.

If you don't have a website where you can upload your changes, you can email me them and I'd be happy to upload it to http://www.elijahlofgren.com/silverstripe/patches/by-others/ so that the core dev team can easily access it when they get around to it.

My email is "elijah (at) silverstripe.com"

Thanks for your contribution, I'm sure it will be much appreciated!

Sincerely,

Elijah Lofgren

Avatar
Tim

Core Development Team, 201 Posts

18 June 2007 at 9:24am

Hi Steve

Thanks for your work, these changes are going to be very useful. Could you please send them to Elijah so we can get it into the core :-)

Avatar
Sean

Forum Moderator, 922 Posts

18 June 2007 at 6:02pm

Edited: 18/06/2007 6:04pm

This is great. Elijah, like Tim said - could you make sure this gets into the GSoC branch of SilverStripe. :-)

Also, Andy, could you update the 2.0 branches with this?

Cheers!
Sean

Avatar
elijahlofgren

Google Summer of Code Hacker, 222 Posts

19 June 2007 at 3:42am

> This is great. Elijah, like Tim said - could you make sure this gets into the GSoC branch of SilverStripe. :-)

I'll plan on working on it once I get the code from Steve. :)

I've posted a reminder for myself here: http://www.elijahlofgren.com/silverstripe/
I tried creating a Trac Ticket, but got an error. I've emailed Hayden about that.

I'm looking forward to trying out your code changes Steve. :) Thanks again!

Blessings,

Elijah

Avatar
elijahlofgren

Google Summer of Code Hacker, 222 Posts

21 June 2007 at 4:37am

I have still not received the changes from Steve.

He may be expecting to get an email notification of replies to this topic and not have checked back here yet.

Sean, Tim: Could one you perhaps email Steve if you have his email address (I don't see it listed in his profile)?

Thanks,

Elijah

Avatar
elijahlofgren

Google Summer of Code Hacker, 222 Posts

22 June 2007 at 11:16am

Edited: 22/06/2007 11:48am

It took me a while to figure out that whitespace was causing the patch to not apply.

It case it is helpful to someone, here is a patch that applies cleanly: http://www.elijahlofgren.com/silverstripe/patches/by-others/Steves-Site-Tree-Tweaks-fixed-whitespace.diff
if this command is run:
patch --ignore-whitespace < /home/elijahlofgren/silverstripe/patches/by-others/Steves-Site-Tree-Tweaks/Steves-Site-Tree-Tweaks-fixed-whitespace.diff

Note: The patch is against: http://www.silverstripe.com/tree-control/

Now that I have it applying cleanly (took my longer than it should, mainly because of my lack of knowledge), I will work on applying it to the gsoc branch.

Avatar
elijahlofgren

Google Summer of Code Hacker, 222 Posts

22 June 2007 at 2:48pm

Edited: 22/06/2007 2:51pm

It took a *lot* longer than I thought it would (mainly due to my lack of experience merging patches), but I have finally merged and committed most of the Tree control tweaks into the gsoc branch with these changes: http://www.elijahlofgren.com/silverstripe/patches/Merge-Site-Tree-Tweaks-From-Steve-jsparty-gsoc-r37261.patch

> * I made it about 30% faster for expand/collapse (simply by iterating over the LIs instead of over the spans)

I assume that the following change was responsible for that fix (I couldn't find any others that seemed to fit):

@@ -237,10 +268,13 @@
      startingPoint = 0;
      childUL = null;
      for(j=0;j<li.childNodes.length;j++) {
+         // Find last div before first ul (unnecessary in our usage)
+         /*
         if(li.childNodes[j].tagName && li.childNodes[j].tagName.toLowerCase() == 'div') {
            startingPoint = j + 1;
            continue;
         }
+         */
   
         if(li.childNodes[j].tagName && li.childNodes[j].tagName.toLowerCase() == 'ul') {
            childUL = li.childNodes[j];

> * I added openall and closeall support

This is really nice! We could possible add "Collapse All" and "Expand All" links to show when the "Reorder..." button is clicked:
http://www.elijahlofgren.com/silverstripe/diffs/Add-Site-Tree-Collapse-All-and-Expand-All-links-cms-gsoc-r37191.diff

> * I'm adding support for noticing if a server-side process has already kind put in all the spanA/spanB/spanC structure, and skip initTrees if so.

NOTE: I was unable to merge this in because initTree() no longer exists and I can't find a function that matches it close enough to merge this code.

> I've also fixed what I think of as a bug, namely that when you click on a title, in addition to following the link, it also toggles the expansion there as if you'd clicked on the +/-.

This bug did not seem to exist in the gsoc version of tree.js so I didn't merge in the workaround for it.

> I've also added internal documentation, since it took me a while to figure out some of the details of how it's doing what it does.

Thanks for this Steve!

Note: Many (that is, a lot!) of the changes in the diff I received were simply formatting changes of this nature:

83c145
<    if(li) {
---
> if (li) {
129c192
< }
---
> } /* treeToggle */

I did not merge those in for the sake of clarity of changes. ;)

Let me know if there are any more changes I should make regarding this code.

Thanks,

Elijah

Go to Top