Rename file/folder issues

[marcelgerber]

This gif is probably pretty self-explaining.

Doubleclick-rename shouldn't work when clicking folders. And I've got no idea what caused the <span class="extension"> to be included in the input.

[redmunds]

I'm seeing a rename issue that may be the same underlying issue, so posting recipe here. Let me know if I should file this separately:

1. Select a file in a subfolder of a project
2. Click on the containing folder so it closes
3. Click on folder again so it re-opens

Result:
After a short delay, the file is changed to rename mode.

Expected:
Rename mode should only be entered to to user selection, not by programmatic selection.

[redmunds]

Marking this high priority. Added Release 41 milestone.

[njx]

This turns out to be tricky to fix, and I'm not sure I can finish the fix today.

The core bug is that when the timeout fires, the selection has already been changed back to the descendant item, so that's what gets renamed. This is easy to fix: we can just check that the selection is still on the node that was originally clicked on.

(There's a secondary issue, which is that clicking on the triangle shouldn't initiate a rename. That's easy to fix also, since we can just only do the rename if the user clicked on the actual <a>, which contains the text but not the triangle.)

The problem is that if you click anywhere on a folder node, it acts like a collapse/expand - not just clicking on the triangle. So, after fixing this bug, you can't rename the parent folder, because even if you click on the name, the selection will move back to the descendant before the timeout fires. (Renaming a folder still works if the previous selection wasn't a descendant.)

So I tried to fix that by adding a parameter to DocumentCommandHandlers.handleFileRename() that specifies the entry to rename (even if it's not the selected entry), and passing that from the timeout, so the folder gets renamed even if the child is selected.

Unfortunately, that ends up calling ProjectManager.renameItemInline(), which calls showInTree() on the folder, which then selects the folder again, causing it to collapse, which feels weird.

I tried adding an argument to showInTree() to tell it not to select the item, but that caused rename to break in some other way, and I just ran out of time to diagnose it.

You can see my attempted fixes in https://github.com/adobe/brackets/tree/nj/issue-8200-DONTMERGE - 44f489b if you want to pick this up.

@redmunds @ryanstewart - at this point given that we're trying to branch for release, I wonder if we should just back out the rename code for now (which is pretty easy - it's localized in two places) and try again next release.

[njx]

@redmunds @ryanstewart - @peterflynn pointed out that we probably should just disable rename for folders, given our current design. Since clicking on the name of a folder currently collapses/expands it, we can't really tell whether your intent is to rename the selected folder or toggle it.

What do you think? That would be an easy fix.

[ryanstewart]

Yeah, it seems reasonable to back it out. I didn't think rename folder was working in one of the initial iterations of the feature. But it seems like one of the various code changes tweaked that.

Go ahead and back it out, I'll take a stab at it for .42.

[ryanstewart]

Oh, and I agree with @peterflynn that we shouldn't do rename of folders. Like I said, in my initial testing it wasn't working, which I thought was a good thing.

[redmunds]

Another option would be to make clicking on a folder name invoke rename, and the only way to open/close is to click on the triangle or folder icon.

[njx]

I just put up a PR to turn off rename for folders: #8200 - take a look and see what you think.

@redmunds We could do that, but that would be a design change for how folders work - we would want @larz0 to weigh in on that. I'd hesitate to make that change right now since people might be used to the existing behavior.

[redmunds]

This fixes the bug I described.

@SAplayer Can you verify it fixes the problem you're seeing (I'm not 100% sure I know exactly what's going on in that animation).