Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Prevent duplicate tree nodes when renaming the only file in a directory #6522

Merged
merged 1 commit into from
Jan 15, 2014

Conversation

jasonsanjose
Copy link
Member

Use jstree open_node in corner case #6474 where renaming the only file in a directory caused duplicate tree nodes to appear

…e in a directory caused duplicate tree nodes to appear
@ghost ghost assigned couzteau Jan 15, 2014
@jasonsanjose
Copy link
Member Author

Assigning @couzteau for review

@couzteau
Copy link
Member

will look at this later tonight.

// jstree attempts to load empty nodes during the create workflow,
// resulting in duplicate nodes for the same entry, see
// https://github.com/adobe/brackets/issues/6474.
if (wasOpen && isClosed) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I understand correctly the fix works because catching the case where the folder was open before and is closed now allows to avoid creating dupe nodes by calling _projectTree.jstree("open_node", $directoryNode); instead of _createNode($directoryNode in line 1987. That seems to be working fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. What would happen is that within jstree's create_node workflow, it would see the parent folder as !is_loaded, call our _treeDataProvider function to populate the list of children, then continue to finish create_node by adding the duplicate here.

@couzteau
Copy link
Member

Done with initial review. The fix works great. I have just one Q regarding the additional redraw, see comment above. While testing I have seen a few cases where the tree wouldn't update at all after renaming a file. In these situations I was also not hitting any breakpoints in the _fileSystemChange handler. Couldn't really repro, but I have seen it a couple of times. That would be a different issue I think, but for the time being I cannot really put my finder on it.

@jasonsanjose
Copy link
Member Author

The additional redraw (to fix the selection marker drawing) is necessary since the first synchronous redraw will happen when the nodes are deleted for this event. This additional async redraw happens after jstree calls into the FileSystem when completing open_node.

@jasonsanjose
Copy link
Member Author

As far as not hitting breakpoints in _fileSystemChange, that would be a different issue. Feel free to merge if I answered your questions.

@couzteau
Copy link
Member

Thanks, I understand now. Actually it dawned on me lats night that the redraw handler and the open node call are directly related ;) - Of course they are! Thanks. This is looking good. The fix is working well and the tests are passing. Merging.

couzteau added a commit that referenced this pull request Jan 15, 2014
Prevent duplicate tree nodes when renaming the only file in a directory
@couzteau couzteau merged commit 8fd0139 into master Jan 15, 2014
@couzteau couzteau deleted the jasonsanjose/issue-6474 branch January 15, 2014 20:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants