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

Solved starter bug: Issue #1976 - Rename on Second Click #2356

Merged
merged 3 commits into from
Dec 17, 2012
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/project/FileViewController.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ define(function (require, exports, module) {
* @returns {$.Promise}
*/
function openAndSelectDocument(fullPath, fileSelectionFocus) {
var result;
var rename = _fileSelectionFocus === fileSelectionFocus && fileSelectionFocus === WORKING_SET_VIEW,
result;

if (fileSelectionFocus !== PROJECT_MANAGER && fileSelectionFocus !== WORKING_SET_VIEW) {
throw new Error("Bad parameter passed to FileViewController.openAndSelectDocument");
Expand All @@ -156,8 +157,12 @@ define(function (require, exports, module) {
// in this case to signify the selection focus has changed even though the current document has not.
var curDoc = DocumentManager.getCurrentDocument();
if (curDoc && curDoc.file.fullPath === fullPath) {
_selectCurrentDocument();
result = (new $.Deferred()).resolve().promise();
if (rename) {
result = CommandManager.execute(Commands.FILE_RENAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

Gabriel,

Thanks for submitting contributions to Brackets, but this code doesn't belong in the openAndSelectDocument() function. This will change the entire meaning of the function.

In general, the code should work like this: when a file is clicked in either the working set list or the project tree, and it is not already selected, the openAndSelectDocument() function should be called. If the file is already selected, then code should be called to rename it.

Also note that your code currently only works on second click in the working set, but I think it should also work in the project tree.

Not sure how familiar you are with github, but after you have updated your code, you don't need to submit a new pull request -- just push you're changes to this same branch and they will show up here. Also, add a comment to the pull request to say you are "ready for another review" so I will get notified with an e-mail.

Thanks,
Randy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Randy,

Be welcome. It's really a pleasure contribute to Brackets. I'm sorry that i changed it context, but I was confused because of that call of openAndSelectDocument() at drop function on WorkspaceSetView module. I thought that it was only to a drag and drop event. Now, I fixed it.

I just think it's a bit confusing have a rename second click event on project tree, it already have a double click event, that is add the selected file to Workspace Set. I can implement it to project tree, but how I said to me it's a bit confusing have two effects and a same gesture to project tree. Let me know if you decide that is better implement it to project tree too.

I'm ready for another review.

Thanks,
Gabriel

} else {
_selectCurrentDocument();
result = (new $.Deferred()).resolve().promise();
}
} else {
result = CommandManager.execute(Commands.FILE_OPEN, {fullPath: fullPath});
}
Expand Down