Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree stops functioning after calling grid.set('columns', columns) with previous columns #1255

Closed
kriszyp opened this issue Mar 3, 2016 · 1 comment
Milestone

Comments

@kriszyp
Copy link
Member

kriszyp commented Mar 3, 2016

This is very easy to reproduce, once you have created a tree grid, simply set the columns again with the same columns:

treeGrid.set('columns', treeGrid.columns)

And you will no longer be able to expand nodes. I reproduced it on common_cases.html with the tree grid there.

The cause of the issue is that the tree column will set _isConfiguredTreeColumn to true. When you later reset the columns, _destroyColumns will remove the expando listener. When the columns are reconfigured, _configureTreeColumn will be properly called, but will immediately return because _isConfiguredTreeColumn is still true, so the listener will never be re-added. Also, _treeColumn is nulled in _destroyColumns, and won't get reset by _configureTreeColumn, so even if expand does get called, it will immediately return as well because _treeColumn isn't set.

This is actually a regression due to:
26a6d00

Prior to that commit, _configureTreeColumn properly scoped the column configuration code inside the conditionally branch of _isConfiguredTreeColumn, and properly re-added the listener and _treeColumn when tree columns were reconfigured.

I could put together a pull request, but clearly the regression commit was trying to accomplish something, so I thought I would start by just identifying the exact cause.

@edhager edhager self-assigned this Mar 3, 2016
edhager added a commit to edhager/dgrid that referenced this issue Mar 4, 2016
Allow Tree event handlers to be reconnected even after the column configuration has been processed by the Tree mixin.
kfranqueiro pushed a commit that referenced this issue Mar 11, 2016
Allow Tree event handlers to be reconnected even after the column configuration has been processed by the Tree mixin.

(cherry picked from commit 930c634)
@kfranqueiro
Copy link
Member

Thanks for reporting this, Kris. In 0.4.1 / 1.0.0, we had a couple of fixes in that area having to do with avoiding redundantly hooking up renderCell and event handlers, but it looks like at some point we oversimplified. I think it slipped my mind that those event handlers were in fact being removed in _destroyColumns.

@kfranqueiro kfranqueiro added this to the 1.0.1 milestone Apr 25, 2016
@edhager edhager removed their assignment May 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants