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

Combine tree edit #191

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Combine tree edit #191

wants to merge 12 commits into from

Conversation

yassam
Copy link

@yassam yassam commented Jun 5, 2012

I've edited dgrid so that it's possible to to combine tree and editor. For it to work, the editor has to wrap a tree, e.g. editor(tree(column)).

I haven't added any test code nor updated any of the docs, and I've only tested it against my own specific case where the tree is only 1 level deep.

yassam added 12 commits May 24, 2012 13:05
…ssfully handled independently of the activate editor event
If the editor wraps a tree, and you edit a child node, then when editing is
complete it renders with the correct level of indentation. To do this I had to
make sure that the options object used to render original child is used at the
end of the edit.
…an editor widget is invoked. Both the row data object and the widget are passed
Conflicts:
	tree.js - resolved by using upstream version
key won't try to expand tree.

Make sure 'this' points to column object when calling
originalRenderCell from editor (so that tree can see correct version
of this).
@ghost
Copy link

ghost commented Jun 5, 2012

Thanks for this pull request! I actually had been in progress of working on this from the opposite angle (wrapping tree around editor) and just committed that; however, I saw a couple of other ideas in here that might also be useful in the future.

I would say I'm a bit concerned at the approach here which seems to put some amount of tree-specific logic within the editor module; such conflation of concerns was something I tried to avoid. However, it'd certainly be ideal if both directions can eventually be supported.

@yassam
Copy link
Author

yassam commented Jun 6, 2012

I actually had been in progress of working on this from the opposite angle (wrapping tree around editor) and just committed that

No problem - I'll check out your changes soon. I'm not precious about my code so feel free to close this pull request if you don't need the code. I'd rather use the upstream version than maintain my own.

I would say I'm a bit concerned at the approach here which seems to put some amount of tree-specific logic within the editor module

I was concerned about that too. It felt wrong to wrap the tree in an editor rather than vice-versa, but I started off with that and I was in a hurry so I kept going with it.

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

Successfully merging this pull request may close these issues.

1 participant