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

Commit

Permalink
Merge pull request #3080 from TomMalbran/tom/fix-issue-2076
Browse files Browse the repository at this point in the history 
Fix #2076: Two indistinguishable events for different cases of working set reordering
  • Loading branch information
@redmunds
redmunds committed Jun 14, 2013
2 parents 10f4054 + 36e9904 commit 358b264
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 111 deletions.
12 changes: 6 additions & 6 deletions src/document/DocumentManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@
* 2nd arg to the listener is the removed FileEntry.
* - workingSetRemoveList -- When multiple files are removed from the working set (e.g. project close).
* The 2nd arg to the listener is the array of removed FileEntry objects.
* - workingSetReorder -- When the indexes of 2 files are swapped. Listener receives no arguments.
* - workingSetSort -- When the workingSet array is sorted. Listener receives no arguments.
* TODO (#2076): combine workingSetSort & workingSetReorder since they convey nearly identical information.
* - workingSetSort -- When the workingSet array is sorted. Notifies the working set view to redraw
* the new sorted list. Listener receives no arguments.
* - workingSetDisableAutoSorting -- When working set is manually re-sorted via dragging and dropping
* a file to disable automatic sorting. Listener receives no arguments.
*
* - fileNameChange -- When the name of a file or folder has changed. The 2nd arg is the old name.
* The 3rd arg is the new name.
Expand Down Expand Up @@ -343,9 +344,7 @@ define(function (require, exports, module) {
temp = _workingSet[index1];
_workingSet[index1] = _workingSet[index2];
_workingSet[index2] = temp;

// Dispatch event
$(exports).triggerHandler("workingSetReorder");
$(exports).triggerHandler("workingSetDisableAutoSorting");
}
}

Expand All @@ -362,6 +361,7 @@ define(function (require, exports, module) {
$(exports).triggerHandler("workingSetSort");
}


/**
* Indicate that changes to currentDocument are temporary for now, and should not update the MRU
* ordering of the working set. Useful for next/previous keyboard navigation (until Ctrl is released)
Expand Down
181 changes: 80 additions & 101 deletions src/project/WorkingSetSort.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ define(function (require, exports, module) {
PreferencesManager = require("preferences/PreferencesManager"),
AppInit = require("utils/AppInit"),
Strings = require("strings");

var defaultPrefs = {
currentSort: Commands.SORT_WORKINGSET_BY_ADDED,
automaticSort: false
};
currentSort: Commands.SORT_WORKINGSET_BY_ADDED,
automaticSort: false
};

/**
* @private
Expand Down Expand Up @@ -76,10 +76,21 @@ define(function (require, exports, module) {

/**
* Retrieves a Sort object by id
* @param {string} commandID
* @return {Sort}
* @param {(string|Command)} command A command ID or a command object.
* @return {?Sort}
*/
function get(commandID) {
function get(command) {
var commandID;
if (!command) {
console.error("Attempting to get a Sort method with a missing required parameter: command");
return;
}

if (typeof command === "string") {
commandID = command;
} else {
commandID = command.getID();
}
return _sorts[commandID];
}

Expand All @@ -90,75 +101,46 @@ define(function (require, exports, module) {
return _automaticSort;
}


/**
* @private
* Sets the value of Automatic Sort and updates the menu item.
* @param {boolean} value
*/
function _setAutomatic(value) {
_automaticSort = value;
_prefs.setValue("automaticSort", _automaticSort);
CommandManager.get(Commands.SORT_WORKINGSET_AUTO).setChecked(_automaticSort);
}

/**
* @private
* Removes current sort DocumentManager listeners.
* Removes the sort DocumentManager listeners.
*/
function _removeListeners() {
if (_currentSort && _currentSort.getEvents()) {
$(DocumentManager).off(".sort");
}
$(DocumentManager).off(".sort");
}

/**
* @private
* Disables Automatic Sort.
* Enables/Disables Automatic Sort depending on the value.
* @param {boolean} enable True to enable, false to disable.
*/
function _disableAutomatic() {
_setAutomatic(false);
_removeListeners();
function setAutomatic(enable) {
_automaticSort = enable;
_prefs.setValue("automaticSort", _automaticSort);
CommandManager.get(Commands.SORT_WORKINGSET_AUTO).setChecked(_automaticSort);

if (enable) {
_currentSort.sort();
} else {
_removeListeners();
}
}

/**
* @private
* Adds current sort DocumentManager listeners.
* Adds the current sort DocumentManager listeners.
*/
function _addListeners() {
if (_automaticSort && _currentSort && _currentSort.getEvents()) {
$(DocumentManager)
.on(_currentSort.getEvents(), function (event) {
_currentSort.callAutomaticFn(event);
.on(_currentSort.getEvents(), function () {
_currentSort.sort();
})
.on("workingSetReorder.sort", function () {
_disableAutomatic();
.on("workingSetDisableAutoSorting.sort", function () {
setAutomatic(false);
});
}
}

/**
* @private
* Enables Automatic Sort.
*/
function _enableAutomatic() {
_setAutomatic(true);
_addListeners();
_currentSort.callAutomaticFn();
}

/**
* Enables/Disables Automatic Sort depending on the value.
* @param {boolean} value
*/
function setAutomatic(value) {
if (value) {
_enableAutomatic();
} else {
_disableAutomatic();
}
}


/**
* @private
Expand Down Expand Up @@ -190,87 +172,88 @@ define(function (require, exports, module) {
* @constructor
* @private
*
* @param {!string} commandID - valid command identifier.
* @param {!function(FileEntry, FileEntry)} compareFn - a valid sort function (see register for a longer explanation).
* @param {!string} events - space-separated DocumentManager possible events ending on ".sort".
* @param {!function($.Event)} automaticFn - the function that will be called when an automatic sort event is triggered.
* @param {string} commandID A valid command identifier.
* @param {function(FileEntry, FileEntry): number} compareFn A valid sort
* function (see register for a longer explanation).
* @param {string} events Space-separated DocumentManager possible events
* ending with ".sort".
*/
function Sort(commandID, compareFn, events, automaticFn) {
this._commandID = commandID;
this._compareFn = compareFn;
this._events = events;
this._automaticFn = automaticFn;
this._commandID = commandID;
this._compareFn = compareFn;
this._events = events;
}

/** @return {CommandID} */
/** @return {string} The Command ID */
Sort.prototype.getCommandID = function () {
return this._commandID;
};

/** @return {CompareFn} */
/** @return {function(FileEntry, FileEntry): number} The compare function */
Sort.prototype.getCompareFn = function () {
return this._compareFn;
};

/** @return {Events} */
/** @return {string} The DocumentManager events */
Sort.prototype.getEvents = function () {
return this._events;
};

/** Calls automaticFn */
Sort.prototype.callAutomaticFn = function (event) {
return this._automaticFn(event);
};

/**
* Performs the sort and makes it the current sort method
* Performs the sort and makes it the current sort method.
*/
Sort.prototype.execute = function () {
_removeListeners();
_setCurrentSort(this);
_addListeners();
this.sort();
};

/**
* Only performs the working set sort if this is the current sort
* Only performs the working set sort if this is the current sort.
*/
Sort.prototype.sort = function () {
if (_currentSort === this) {
_removeListeners();
DocumentManager.sortWorkingSet(this._compareFn);
_addListeners();
}
};


/**
* Registers a working set sort method.
* @param {!string} commandID - valid command identifier used for the sort method.
* Core commands in Brackets use a simple command title as an id, for example "open.file".
* Extensions should use the following format: "author.myextension.mycommandname".
* For example, "lschmitt.csswizard.format.css".
* @param {!function(FileEntry, FileEntry)} compareFn - the function that will be used inside JavaScript's
* sort function. The return a value should be >0 (sort a to a lower index than b), =0 (leaves a and b
* unchanged with respect to each other) or <0 (sort b to a lower index than a) and must always returns
* the same value when given a specific pair of elements a and b as its two arguments.
* Documentation: https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/sort
* @param {?string} events - one or more space-separated event types that DocumentManger uses.
* Each event passed will trigger the automatic sort. If no events are passed, the automatic
* sort will be disabled for that sort method.
* @param {?function($.Event)} automaticFn - the function that will be called when an automatic sort
* event is triggered. If no function is passed the automatic sort will just call the sort function.
* @param {(string|Command)} command A command ID or a command object
* @param {function(FileEntry, FileEntry): number} compareFn The function that
* will be used inside JavaScript's sort function. The return a value
* should be >0 (sort a to a lower index than b), =0 (leaves a and b
* unchanged with respect to each other) or <0 (sort b to a lower index
* than a) and must always returns the same value when given a specific
* pair of elements a and b as its two arguments. Documentation at:
* https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/sort
* @param {?string} events One or more space-separated event types that
* DocumentManger uses. Each event passed will trigger the automatic
* sort. If no events are passed, the automatic sort will be disabled
* for that sort method.
* @return {?Sort}
*/
function register(commandID, compareFn, events, automaticFn) {
if (_sorts[commandID]) {
console.log("Attempting to register an already-registered sort: " + commandID);
function register(command, compareFn, events) {
var commandID = "";

if (!command || !compareFn) {
console.log("Attempting to register a Sort method with a missing required parameter: command or compare function");
return;
}
if (!commandID || !compareFn) {
console.log("Attempting to register a sort with a missing id, or compare function: " + commandID);
if (typeof command === "string") {
commandID = command;
} else {
commandID = command.getID();
}

if (_sorts[commandID]) {
console.log("Attempting to register an already-registered Sort method: " + command);
return;
}

// Adds ".sort" to the end of each event to make them specific for the automatic sort
// Adds ".sort" to the end of each event to make them specific for the automatic sort.
if (events) {
events = events.split(" ");
events.forEach(function (event, index) {
Expand All @@ -279,11 +262,7 @@ define(function (require, exports, module) {
events = events.join(" ");
}

automaticFn = automaticFn || function (event) {
_currentSort.sort();
};

var sort = new Sort(commandID, compareFn, events, automaticFn);
var sort = new Sort(commandID, compareFn, events);
_sorts[commandID] = sort;
return sort;
}
Expand Down Expand Up @@ -362,7 +341,7 @@ define(function (require, exports, module) {
_setCurrentSort(curSort);
}
if (autoSort) {
_enableAutomatic();
setAutomatic(true);
}
if (curSort && autoSort) {
curSort.sort();
Expand Down
9 changes: 5 additions & 4 deletions src/project/WorkingSetView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,16 @@ define(function (require, exports, module) {
} else {
CommandManager.execute(Commands.FILE_CLOSE, {file: $listItem.data(_FILE_KEY)});
}
} else if (moved) {

} else {
// Update the file selection
if (selected) {
// Update the file selection
_fireSelectionChanged();
ViewUtils.scrollElementIntoView($openFilesContainer, $listItem, false);
}

// Restore the shadow
if (addBottomShadow) {
// Restore the shadows
ViewUtils.addScrollerShadow($openFilesContainer[0], null, true);
}
}
Expand Down Expand Up @@ -544,7 +546,6 @@ define(function (require, exports, module) {
*/
function _handleWorkingSetSort() {
_rebuildWorkingSet(true);
_scrollSelectedDocIntoView();
}

/**
Expand Down

0 comments on commit 358b264

Please sign in to comment.