SpiderStrategies / kalpa-tree

A tree implementation backed by D3
http://spiderstrategies.github.io/kalpa-tree/
ISC License
4 stars 2 forks source link

Forests incorrectly animate initialSelection #258

Closed mattsgarlata closed 8 years ago

mattsgarlata commented 8 years ago

I'm pretty sure they aren't supposed to do that, but @scottoreilly will need to confirm.

https://files.slack.com/files-pri/T0293R8M7-F0J2MLH3N/download/forests_incorrectly_animate_initialselection.mov

nathanbowser commented 8 years ago

This is a bug based on code intentions and previous discussions. The initial load isn't supposed to animate. I'll fix it.

nathanbowser commented 8 years ago

Hey @mattsgarlata, I'm having trouble reproducing this. https://files.slack.com/files-pri/T0293R8M7-F0J3K2T7U/download/cannot-reproduce.mov

Mind sending me the JSON and options you're passing to the tree?

mattsgarlata commented 8 years ago

The tree options are an object. Here is the JSON.stringify version

{
    "accessors": {
        "icon": "type"
    },
    "forest": true,
    "stream": {
        "_events": {},
        "writable": true,
        "readable": true,
        "paused": false,
        "autoDestroy": true
    }
}

Here is a screenshot of the object:

screen shot 2016-01-11 at 9 12 16 am

Here's the JSON

[ {
  "id" : 231599463,
  "label" : "folder",
  "parentId" : -1,
  "organizationId" : 2,
  "type" : "folder"
}, {
  "id" : 67731457,
  "label" : "background image",
  "parentId" : 231599463,
  "type" : "dashboard"
}, {
  "id" : 231600470,
  "label" : "folder 2",
  "parentId" : 231599463,
  "type" : "folder"
}, {
  "id" : 220561408,
  "label" : "drilldown",
  "parentId" : 231600470,
  "type" : "dashboard"
}, {
  "id" : 231576461,
  "label" : "export",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 8617985,
  "label" : "Manufacturing Gauge WOBs",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 39878658,
  "label" : "New Chart Types",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 216596480,
  "label" : "report",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 16875527,
  "label" : "sample",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 7864320,
  "label" : "Strategy Map",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 67796992,
  "label" : "test",
  "parentId" : -1,
  "type" : "dashboard"
}, {
  "id" : 18907136,
  "label" : "test1",
  "parentId" : -1,
  "type" : "dashboard"
} ]
nathanbowser commented 8 years ago

Thank you!

mattsgarlata commented 8 years ago

I think I see what's going on here. When I switch sections, the application retrieves the tree by org ID (using URL http://localhost:8080/cms/api/organizations/2/dashboards/tree) and doesn't know the ID of the first item in the tree. I'm highlighting the first item with this code

    // ensures something in the tree is selectd
    _ensureSelection: function() {
        if (!this.gardener.options.id) {
            this.gardener.once('node', function(root) {
                this.gardener.tree.select(root.id, {silent: true})
            }.bind(this))
        }
    },

I think I just need to pass in animate: false or something.

mattsgarlata commented 8 years ago

toggleOnSelect: false fixed the animation. But should the folder be expanded? This is what it looks like now when I first switch to the dashboards section.

screen shot 2016-01-11 at 9 17 10 am

nathanbowser commented 8 years ago

It might be some weird race issue. If you select a node, it should expand it, but when you call that select, it doesn't have children. Can you wait until the tree has all its nodes?

mattsgarlata commented 8 years ago

I tried that out, but the trouble is passing in toggleOnSelect: false not only prevents the animation, but also prevents the node from being expanded. So it seems like some sort of change will be needed in the tree here to allow me to select the first item without causing an animation. Perhaps we need a new option selectFirstNode: true

nathanbowser commented 8 years ago

Makes sense. I'll take a look.

nathanbowser commented 8 years ago

@mattsgarlata This works for me in the example page, but things could still be different because of timing. When you get a minute, see if this does what you want:

tree.once('node', function (root) {
  tree.select(root.id, {silent: true, animate: false})
})
mattsgarlata commented 8 years ago

I'll try to get to that this week, but just looking at it, I bet that's all I need. Thanks for your help!

mattsgarlata commented 8 years ago

Yep, that did the trick. Thanks again!