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

Correctly handle focus changes between working set and file tree when activating document from find in filesfixes #1280 #1290

Merged
merged 3 commits into from
Jul 21, 2012
Merged
Show file tree
Hide file tree
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
8 changes: 4 additions & 4 deletions src/project/FileViewController.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ define(function (require, exports, module) {
* in the working set (the open files) or in the file tree, but not both.
* @param {String} fileSelectionFocus - either PROJECT_MANAGER or WORKING_SET_VIEW
*/
function setFileSelectionFocus(fileSelectionFocus) {
function setFileViewFocus(fileSelectionFocus) {
if (fileSelectionFocus !== PROJECT_MANAGER && fileSelectionFocus !== WORKING_SET_VIEW) {
throw new Error("Bad parameter passed to FileViewController.setFileSelectionFocus");
throw new Error("Bad parameter passed to FileViewController.setFileViewFocus");
}

_fileSelectionFocus = fileSelectionFocus;
$(exports).triggerHandler("documentSelectionFocusChange");
$(exports).triggerHandler("fileViewFocusChange");
}

/**
Expand Down Expand Up @@ -206,7 +206,7 @@ define(function (require, exports, module) {
exports.getFileSelectionFocus = getFileSelectionFocus;
exports.openAndSelectDocument = openAndSelectDocument;
exports.addToWorkingSetAndSelect = addToWorkingSetAndSelect;
exports.setFileSelectionFocus = setFileSelectionFocus;
exports.setFileViewFocus = setFileViewFocus;
exports.WORKING_SET_VIEW = WORKING_SET_VIEW;
exports.PROJECT_MANAGER = PROJECT_MANAGER;
});
19 changes: 8 additions & 11 deletions src/project/ProjectManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,14 @@ define(function (require, exports, module) {
}
return null;
}

function _fileViewFocusChange() {
_redraw(true);
}

var _documentSelectionFocusChange = function () {
function _documentSelectionFocusChange() {
var curDoc = DocumentManager.getCurrentDocument();
if (curDoc && _hasFileSelectionFocus()) {

// Don't update the file tree selection to the current open doc when there is a directory
// already selected
var selected = getSelectedItem();
if (selected && selected.isDirectory) {
return;
}

$("#project-files-container li").is(function (index) {
var entry = $(this).data("entry");
if (entry && entry.fullPath === curDoc.file.fullPath && !_projectTree.jstree("is_selected", $(this))) {
Expand All @@ -192,7 +188,7 @@ define(function (require, exports, module) {
}

_redraw(true);
};
}

/**
* Returns the root folder of the currently loaded project, or null if no project is open (during
Expand Down Expand Up @@ -375,7 +371,7 @@ define(function (require, exports, module) {
}
});
} else {
FileViewController.setFileSelectionFocus(FileViewController.PROJECT_MANAGER);
FileViewController.setFileViewFocus(FileViewController.PROJECT_MANAGER);
// show selection marker on folders
_redraw(true);

Expand Down Expand Up @@ -953,6 +949,7 @@ define(function (require, exports, module) {

// Event Handlers
$(FileViewController).on("documentSelectionFocusChange", _documentSelectionFocusChange);
$(FileViewController).on("fileViewFocusChange", _fileViewFocusChange);
$("#open-files-container").on("contentChanged", function () {
_redraw(false); // redraw jstree when working set size changes
});
Expand Down
8 changes: 8 additions & 0 deletions src/project/WorkingSetView.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
/**
* WorkingSetView generates the UI for the list of the files user is editing based on the model provided by EditorManager.
* The UI allows the user to see what files are open/dirty and allows them to close files and specify the current editor.
*
* Events dispatched:
* documentSelectionFocusChange - indicates a document change has caused the focus to change between the working
* set and file tree.
* fileViewFocusChange - indicates the selection focus has changed between the working set and the project tree, but the
* document selection has NOT changed
*/
define(function (require, exports, module) {
"use strict";
Expand Down Expand Up @@ -355,6 +361,8 @@ define(function (require, exports, module) {
_handleDocumentSelectionChange();
_fireSelectionChanged();
});

$(FileViewController).on("fileViewFocusChange", _updateListSelection);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems inconsistent that the fileViewFocusChange event calls _updateListSelection() directly, and the documentSelectionFocusChange event calls _handleDocumentSelectionChange() (which simply call _updateListSelection()). I think it would be easier to understand this code if both events called the same function. Can we get rid of the _handleDocumentSelectionChange() function?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is to move the call to _fireSelectionChanged() into the _handleDocumentSelectionChange() function to make the event binding cleaner,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes pushed


// Show scroller shadows when open-files-container scrolls
ViewUtils.addScrollerShadow($openFilesContainer[0], null, true);
Expand Down