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

Replaced toggleClass with add/removeClass where appropriate #3689

Merged
merged 4 commits into from
May 13, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
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
13 changes: 4 additions & 9 deletions src/command/Menus.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ define(function (require, exports, module) {
KeyBindingManager = require("command/KeyBindingManager"),
StringUtils = require("utils/StringUtils"),
CommandManager = require("command/CommandManager"),
PopUpManager = require("widgets/PopUpManager");
PopUpManager = require("widgets/PopUpManager"),
ViewUtils = require("utils/ViewUtils");

/**
* Brackets Application Menu Constants
Expand Down Expand Up @@ -675,13 +676,7 @@ define(function (require, exports, module) {
}
});
} else {
// Note, checked can also be undefined, so we explicitly check
// for truthiness and don't use toggleClass().
if (checked) {
$(_getHTMLMenuItem(this.id)).addClass("checked");
} else {
$(_getHTMLMenuItem(this.id)).removeClass("checked");
}
ViewUtils.toggleClass($(_getHTMLMenuItem(this.id)), "checked", checked);
}
};

Expand All @@ -698,7 +693,7 @@ define(function (require, exports, module) {
}
Copy link
Member

Choose a reason for hiding this comment

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

You can also port over the calls just above this, in _checkedChanged().

});
} else {
$(_getHTMLMenuItem(this.id)).toggleClass("disabled", !this._command.getEnabled());
ViewUtils.toggleClass($(_getHTMLMenuItem(this.id)), "disabled", !this._command.getEnabled());
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/editor/MultiRangeInlineEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,14 @@ define(function (require, exports, module) {
var $previousItem = (this._selectedRangeIndex >= 0) ? this._ranges[this._selectedRangeIndex].$listItem : null;

if ($previousItem) {
$previousItem.toggleClass("selected", false);
$previousItem.removeClass("selected");
}

this._selectedRangeIndex = newIndex;

var $rangeItem = this._ranges[this._selectedRangeIndex].$listItem;

this._ranges[this._selectedRangeIndex].$listItem.toggleClass("selected", true);
this._ranges[this._selectedRangeIndex].$listItem.addClass("selected");

// Remove previous editors
this.editors.forEach(function (editor) {
Expand Down
8 changes: 3 additions & 5 deletions src/project/WorkingSetView.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,8 @@ define(function (require, exports, module) {

// Set icon's class
if ($fileStatusIcon) {
// cast to Boolean needed because toggleClass() distinguishes true/false from truthy/falsy
$fileStatusIcon.toggleClass("dirty", Boolean(isDirty));
$fileStatusIcon.toggleClass("can-close", Boolean(canClose));
ViewUtils.toggleClass($fileStatusIcon, "dirty", isDirty);
ViewUtils.toggleClass($fileStatusIcon, "can-close", canClose);
}
}

Expand All @@ -336,8 +335,7 @@ define(function (require, exports, module) {
function _updateListItemSelection(listItem, selectedDoc) {
var shouldBeSelected = (selectedDoc && $(listItem).data(_FILE_KEY).fullPath === selectedDoc.file.fullPath);

// cast to Boolean needed because toggleClass() distinguishes true/false from truthy/falsy
$(listItem).toggleClass("selected", Boolean(shouldBeSelected));
ViewUtils.toggleClass($(listItem), "selected", shouldBeSelected);
}

function isOpenAndDirty(file) {
Expand Down
5 changes: 3 additions & 2 deletions src/search/FindReplace.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ define(function (require, exports, module) {
StringUtils = require("utils/StringUtils"),
Editor = require("editor/Editor"),
EditorManager = require("editor/EditorManager"),
ModalBar = require("widgets/ModalBar").ModalBar;
ModalBar = require("widgets/ModalBar").ModalBar,
ViewUtils = require("utils/ViewUtils");

var modalBar,
isFindFirst = false;
Expand Down Expand Up @@ -221,7 +222,7 @@ define(function (require, exports, module) {
var foundAny = findNext(editor, rev);

if (modalBar) {
getDialogTextField().toggleClass("no-results", !foundAny);
ViewUtils.toggleClass(getDialogTextField(), "no-results", !foundAny);
}
});
isFindFirst = false;
Expand Down
8 changes: 5 additions & 3 deletions src/search/QuickOpen.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ define(function (require, exports, module) {
LanguageManager = require("language/LanguageManager"),
KeyEvent = require("utils/KeyEvent"),
ModalBar = require("widgets/ModalBar").ModalBar,
StringMatch = require("utils/StringMatch");
StringMatch = require("utils/StringMatch"),
ViewUtils = require("utils/ViewUtils");


/** @type Array.<QuickOpenPlugin> */
Expand Down Expand Up @@ -402,8 +403,9 @@ define(function (require, exports, module) {
QuickNavigateDialog.prototype._handleResultsReady = function (e, results) {
// Give visual clue when there are no results (unless we're in "Go To Line" mode, where there
// are never results, or we're in file search mode and waiting for the index to get rebuilt)
var isNoResults = (results.length === 0 && (fileList || currentPlugin) && !this._isValidLineNumberQuery(this.$searchField.val()));
this.$searchField.toggleClass("no-results", Boolean(isNoResults));
var hasNoResults = (results.length === 0 && (fileList || currentPlugin) && !this._isValidLineNumberQuery(this.$searchField.val()));

ViewUtils.toggleClass(this.$searchField, "no-results", hasNoResults);
};

/**
Expand Down
19 changes: 17 additions & 2 deletions src/utils/ViewUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,20 @@ define(function (require, exports, module) {
$displayElement.off("contentChanged.scroller-shadow");
}

/**
* Utility function to replace jQuery.toggleClass when used with the second argument, which needs to be a true boolean for jQuery
* @param {!jQueryObject} $domElement The jQueryObject to toggle the Class on
* @param {!string} className Class name or names (separated by spaces) to toggle
* @param {!boolean} addClass A truthy value to add the class and a falsy value to remove the class
*/
function toggleClass($domElement, className, addClass) {
if (addClass) {
$domElement.addClass(className);
} else {
$domElement.removeClass(className);
}
}

/**
* Within a scrolling DOMElement, creates and positions a styled selection
* div to align a single selected list item from a ul list element.
Expand Down Expand Up @@ -202,8 +216,8 @@ define(function (require, exports, module) {

$selectionTriangle.css("top", triangleTop);
$selectionTriangle.css("left", $sidebar.width() - $selectionTriangle.outerWidth());
$selectionTriangle.toggleClass("triangle-visible", showTriangle);

toggleClass($selectionTriangle, "triangle-visible", showTriangle);
var triangleClipOffsetYBy = Math.floor((selectionMarkerHeight - triangleHeight) / 2),
triangleBottom = triangleTop + triangleHeight + triangleClipOffsetYBy;

Expand Down Expand Up @@ -351,4 +365,5 @@ define(function (require, exports, module) {
exports.sidebarList = sidebarList;
exports.scrollElementIntoView = scrollElementIntoView;
exports.getFileEntryDisplay = getFileEntryDisplay;
exports.toggleClass = toggleClass;
Copy link
Member

Choose a reason for hiding this comment

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

ViewUtils seems like an odd place to put this, since the other code in here is so oriented around managing scrolling panels (mainly the sidebar). But short of creating a new DOMUtils module, there doesn't seem to be any better option. So we might as well leave it here for now...

Copy link
Member

Choose a reason for hiding this comment

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

ViewUtils seems like an odd place to put this, since the other code in here is so oriented around managing scrolling panels (mainly the sidebar). But short of creating a new DOMUtils module, there doesn't seem to be any better option. So we might as well leave it here for now...

});