-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Zanqi/issue 1793: F2 key should be assigned to rename selected file/folder command. #1836
Conversation
This reverts commit 1c18a8c.
src/command/Menus.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the shortcut is the same for both platforms, you can just put "F2" alone (see examples elsewhere in this file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
F2 is usually a Windows only shortcut. Mac generally uses Enter key for rename shortcut.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enter is currently used to enter a new line into the selected file...Do we want to change that for mac?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, that's because the file tree cannot yet take focus. We definitely do not want to override "enter new line".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an open issue to allow file tree to take focus?
|
This will crash if you hit the shortcut while the selection is in the working set (the list of open files shown above the tree). You may be able to fix that for most cases using the recently-added NAVIGATE_SHOW_IN_FILE_TREE command to fix it (you'll probably need to change it to return a Promise so you can track when it's actually done). But there will be a few cases where the file doesn't exist in the tree at all -- e.g. if you use File > Open to open something outside the project folder -- and in those cases this shouldn't crash either. |
|
One last note: GitHub is showing several older commits from Brackets core in the list of things this pull request will try to merge in. Since those commits are already in master, it seems like something might be screwy with your branch history. I'm not sure if that will have any ill effects, but it might be worth checking into... |
|
I was trying to keep the branch up to date with the Brackets core, so I followed "Tracking Changes from the Main Repository" on How to Hack on Brackets. Maybe I shouldn't commit those merges? Is there a recommended way to keep a branch in sync with the main Repo? |
|
I tried hitting the shortcut while the selection is in the working set, nothing happened. I also tried opening a file that's outside the project folder. It didn't crash. The working set list don't have rename command in their context menu, so I don't think they are affected. |
|
Perhaps "crash" was a bad word choice on my part. If you look in the console (Debug > Show Developer Tools) you'll see that an exception is thrown in both of those cases. We don't want the command to throw exceptions. Also, when the selection is in the working set but the file does exist in the project folder, ideally the result of hitting F2 wouldn't be a no-op -- it could pop open the appropriate folder(s) in the tree to show the rename UI. That's where the new "show in tree" API could come in handy. |
… Enter to rename file for mac
|
Issue: jstree listens to "Enter" key keyup event to finish renaming node, that terminates renaming files in mac when user releases Enter key after pressing Enter to trigger rename. |
|
@zanqi: thanks for your work on this! I didn't realize you were going to make the project tree focusable. Unfortunately, I think we'd rather not make that change at this point. We're aiming for a focus model where the editor retains focus most of the time, even when you've clicked in other places such as the project tree. Ideally, I think we should have the F2 shortcut that Win users are familiar with plus the ability to rename by clicking the already-selected item (a common gesture on both Mac and Win). So, adding F2 alone seems like a good place to start. I think it's a pretty simple change now -- #1919 has fixed the FILE_RENAME command so that you don't have to play tricks with showInTree() anymore. I think all you need to do now is register the shortcut in Menus.js. Could you test that out and put up a new pull request containing only that change? Thanks again for working on this! |
|
p.s. I would say we could just cherry-pick your existing commit 66ed34c, but we should probably clean that up by merging the mac/win keystroke declarations (since they're identical). |
|
Sure, I'll open a new pull request. |
|
Great -- thanks! |
|
Here's the new pull request: #1922 |
Assign F2 key to rename selected file/folder.