Rename on Second Click fixes #2412
Conversation
ghost
assigned 
|
Note: holding off on this until Sprint 19 since the functionality that caused those bugs is being temporarily backed out for Sprint 18 -- see pull #2417. We can use this pull request to re-enable the functionality next sprint, with all the necessary fixes. |
|
@peterflynn This is ready for re-review. Please read details at the top of issue carefully because there are a lot a subtle cases fixed with this one. |
|
@redmunds: is double-click in the working set supposed to do anything? It doesn't seem to on master, and I can't find any code that implies otherwise... I think double-click is only a factor for the project tree. |
|
Double-click in the working set is not supposed to do anything. I added the double-click handler just for the sake of detecting a double click and prevent 2 single-click events from occurring (which would be: initiate rename, followed by cancel rename). |
| _projectTree.jstree("select_node", $node, false); | ||
| _suppressRename = false; |
There was a problem hiding this comment.
There seem to be a lot of other cases where we also programatically invoke select_node. Can we be sure none of them will ever happen when the item is already selected?
It seems safer to not have to rely on that sort of assumption. Is there a way for the select_node event handler above to distinguish user-driven selection from programmatic? (e.g. does it pass along a root-cause click event object or anything?). If not, I wonder if we could instead just have rename be driven by click events rather than select_node...
|
If I right-click the selected item in the file tree, I'm still seeing it go into rename mode... Doesn't happen for the working set though. |
| CommandManager.execute(Commands.FILE_RENAME); | ||
| } | ||
| _inClickCallback = false; | ||
| }; |
There was a problem hiding this comment.
There's a decent amount of code duplication between here and ProjectManager. If you don't think there's a clean way to centralize it, could we add a few comments so future modifiers of this code know changes should probably be mirrored in the other file? I'd suggest a comment in the timeout handler in each file, and in the place the timeout gets set in each file...
|
@redmunds: done reviewing. |
|
@peterflynn I guess there's no reason to leave this open. Closing. |
|
Sounds good. We can always leave the branch around for a while in case @AVGP or someone else wants to build off of it. |
This prevents rename on right click for #2394. This fix in WorkingSetView.js is simpler than it looks at first glance because I also did a minor logic cleanup.
This implements Rename on second click in project tree for #2411.
@peterflynn
I also looked at fixing the behavior of double-click in the working set, but couldn't get it working. Double-clicking in the project tree to open a file and also put it in the working set probably only works because the operation for single-clicking is a subset of it (only opens file). I'm guessing that double-click in project tree only seems to work because what single-click does is a subset of what double-click does.Double-click in either Working Set or Project Tree is now fixed so that it should never invoke Rename as mentioned in #2411.I noticed that re-order working set files using drag/drop while in Rename mode. Is this OK? I couldn't find any cases that seemed to cause any problems.
Fixed (untracked) issue where Navigate > Show in File Tree so Rename is not invoked. It was a tricky recipe to start with: