From 8135b1935c6785f500e7dc467c2cc74e6c1962fa Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Wed, 1 May 2013 09:01:32 +0200 Subject: [PATCH 1/3] Replaced toggleClass with add/removeClass where appropriate --- src/command/Menus.js | 6 +++++- src/editor/MultiRangeInlineEditor.js | 4 ++-- src/project/WorkingSetView.js | 20 +++++++++++++++----- src/search/FindReplace.js | 6 +++++- src/search/QuickOpen.js | 8 ++++++-- src/utils/ViewUtils.js | 8 ++++++-- 6 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 455aecb6151..16fdc209506 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -698,7 +698,11 @@ define(function (require, exports, module) { } }); } else { - $(_getHTMLMenuItem(this.id)).toggleClass("disabled", !this._command.getEnabled()); + if (this._command.getEnabled()) { + $(_getHTMLMenuItem(this.id)).removeClass("disabled"); + } else { + $(_getHTMLMenuItem(this.id)).addClass("disabled"); + } } }; diff --git a/src/editor/MultiRangeInlineEditor.js b/src/editor/MultiRangeInlineEditor.js index 30a3ea3f58b..5eafc18733b 100644 --- a/src/editor/MultiRangeInlineEditor.js +++ b/src/editor/MultiRangeInlineEditor.js @@ -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) { diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index d11af5104b6..5b5442badbe 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -321,9 +321,16 @@ 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)); + if (isDirty) { + $fileStatusIcon.addClass("dirty"); + } else { + $fileStatusIcon.removeClass("dirty"); + } + if (canClose) { + $fileStatusIcon.addClass("can-close"); + } else { + $fileStatusIcon.removeClass("can-close"); + } } } @@ -336,8 +343,11 @@ 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)); + if (shouldBeSelected) { + $(listItem).addClass("selected"); + } else { + $(listItem).removeClass("selected"); + } } function isOpenAndDirty(file) { diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index 1d5c1f7295f..cf377d15f6d 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -221,7 +221,11 @@ define(function (require, exports, module) { var foundAny = findNext(editor, rev); if (modalBar) { - getDialogTextField().toggleClass("no-results", !foundAny); + if (!foundAny) { + getDialogTextField().addClass("no-results"); + } else { + getDialogTextField().removeClass("no-results"); + } } }); isFindFirst = false; diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index 81376f4d6b2..bbe18f8a1fe 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -402,8 +402,12 @@ 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())); + if (hasNoResults) { + this.$searchField.addClass("no-results"); + } else { + this.$searchField.removeClass("no-results"); + } }; /** diff --git a/src/utils/ViewUtils.js b/src/utils/ViewUtils.js index 5026d32f1f0..1b0001aee68 100644 --- a/src/utils/ViewUtils.js +++ b/src/utils/ViewUtils.js @@ -202,8 +202,12 @@ define(function (require, exports, module) { $selectionTriangle.css("top", triangleTop); $selectionTriangle.css("left", $sidebar.width() - $selectionTriangle.outerWidth()); - $selectionTriangle.toggleClass("triangle-visible", showTriangle); - + if (showTriangle) { + $selectionTriangle.addClass("triangle-visible"); + } else { + $selectionTriangle.removeClass("triangle-visible"); + } + var triangleClipOffsetYBy = Math.floor((selectionMarkerHeight - triangleHeight) / 2), triangleBottom = triangleTop + triangleHeight + triangleClipOffsetYBy; From 74fc7331f7e486a318c8e8364f135038188ef037 Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Fri, 3 May 2013 21:06:45 +0200 Subject: [PATCH 2/3] Added a util function toggleClass --- src/command/Menus.js | 9 +++------ src/project/WorkingSetView.js | 18 +++--------------- src/search/FindReplace.js | 9 +++------ src/search/QuickOpen.js | 10 ++++------ src/utils/ViewUtils.js | 21 ++++++++++++++++----- 5 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 16fdc209506..4c69e375c09 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -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 @@ -698,11 +699,7 @@ define(function (require, exports, module) { } }); } else { - if (this._command.getEnabled()) { - $(_getHTMLMenuItem(this.id)).removeClass("disabled"); - } else { - $(_getHTMLMenuItem(this.id)).addClass("disabled"); - } + ViewUtils.toggleClass($(_getHTMLMenuItem(this.id)), "disabled", this._command.getEnabled()); } }; diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index 5b5442badbe..2676f7aef08 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -321,16 +321,8 @@ define(function (require, exports, module) { // Set icon's class if ($fileStatusIcon) { - if (isDirty) { - $fileStatusIcon.addClass("dirty"); - } else { - $fileStatusIcon.removeClass("dirty"); - } - if (canClose) { - $fileStatusIcon.addClass("can-close"); - } else { - $fileStatusIcon.removeClass("can-close"); - } + ViewUtils.toggleClass($fileStatusIcon, "dirty", isDirty); + ViewUtils.toggleClass($fileStatusIcon, "can-close", canClose); } } @@ -343,11 +335,7 @@ define(function (require, exports, module) { function _updateListItemSelection(listItem, selectedDoc) { var shouldBeSelected = (selectedDoc && $(listItem).data(_FILE_KEY).fullPath === selectedDoc.file.fullPath); - if (shouldBeSelected) { - $(listItem).addClass("selected"); - } else { - $(listItem).removeClass("selected"); - } + ViewUtils.toggleClass($(listItem), "selected", shouldBeSelected); } function isOpenAndDirty(file) { diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index cf377d15f6d..817d9d76c88 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -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; @@ -221,11 +222,7 @@ define(function (require, exports, module) { var foundAny = findNext(editor, rev); if (modalBar) { - if (!foundAny) { - getDialogTextField().addClass("no-results"); - } else { - getDialogTextField().removeClass("no-results"); - } + ViewUtils.toggleClass(getDialogTextField(), "no-results", !foundAny); } }); isFindFirst = false; diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index bbe18f8a1fe..4f7243ba4d4 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -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. */ @@ -403,11 +404,8 @@ define(function (require, exports, module) { // 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 hasNoResults = (results.length === 0 && (fileList || currentPlugin) && !this._isValidLineNumberQuery(this.$searchField.val())); - if (hasNoResults) { - this.$searchField.addClass("no-results"); - } else { - this.$searchField.removeClass("no-results"); - } + + ViewUtils.toggleClass(this.$searchField, "no-results", hasNoResults); }; /** diff --git a/src/utils/ViewUtils.js b/src/utils/ViewUtils.js index 1b0001aee68..a30c57ad7d4 100644 --- a/src/utils/ViewUtils.js +++ b/src/utils/ViewUtils.js @@ -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} switcher A truthy value to add the class and a falsy value to remove the class + */ + function toggleClass($DomElement, className, switcher) { + if (switcher) { + $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. @@ -202,11 +216,7 @@ define(function (require, exports, module) { $selectionTriangle.css("top", triangleTop); $selectionTriangle.css("left", $sidebar.width() - $selectionTriangle.outerWidth()); - if (showTriangle) { - $selectionTriangle.addClass("triangle-visible"); - } else { - $selectionTriangle.removeClass("triangle-visible"); - } + toggleClass($selectionTriangle, "triangle-visible", showTriangle); var triangleClipOffsetYBy = Math.floor((selectionMarkerHeight - triangleHeight) / 2), triangleBottom = triangleTop + triangleHeight + triangleClipOffsetYBy; @@ -355,4 +365,5 @@ define(function (require, exports, module) { exports.sidebarList = sidebarList; exports.scrollElementIntoView = scrollElementIntoView; exports.getFileEntryDisplay = getFileEntryDisplay; + exports.toggleClass = toggleClass; }); From dbc5164f4a942ce642e39a9c487b2f53a6f61e36 Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Sat, 11 May 2013 16:17:31 +0200 Subject: [PATCH 3/3] Fixes after review --- src/command/Menus.js | 10 ++-------- src/utils/ViewUtils.js | 12 ++++++------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 4c69e375c09..a9a08299f31 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -676,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); } }; @@ -699,7 +693,7 @@ define(function (require, exports, module) { } }); } else { - ViewUtils.toggleClass($(_getHTMLMenuItem(this.id)), "disabled", this._command.getEnabled()); + ViewUtils.toggleClass($(_getHTMLMenuItem(this.id)), "disabled", !this._command.getEnabled()); } }; diff --git a/src/utils/ViewUtils.js b/src/utils/ViewUtils.js index a30c57ad7d4..6ea760c872c 100644 --- a/src/utils/ViewUtils.js +++ b/src/utils/ViewUtils.js @@ -156,15 +156,15 @@ define(function (require, exports, module) { /** * 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 {!jQueryObject} $domElement The jQueryObject to toggle the Class on * @param {!string} className Class name or names (separated by spaces) to toggle - * @param {!boolean} switcher A truthy value to add the class and a falsy value to remove the class + * @param {!boolean} addClass A truthy value to add the class and a falsy value to remove the class */ - function toggleClass($DomElement, className, switcher) { - if (switcher) { - $DomElement.addClass(className); + function toggleClass($domElement, className, addClass) { + if (addClass) { + $domElement.addClass(className); } else { - $DomElement.removeClass(className); + $domElement.removeClass(className); } }