Fixing the adjust font restore scroll and adding unit tests

src/view/ViewCommandHandlers.js

@@ -76,24 +76,53 @@ define(function (require, exports, module) {
      * @type {boolean}
      */
     var _fontSizePrefsLoaded = false;
-
+
+
+    /**
+     * @private
+     * Refreshes the editor and restores the scroll position
+     */
+    function _refreshAndRestoreScroll() {
+        var editor     = EditorManager.getCurrentFullEditor(),
+            oldHeight  = editor.getTextHeight(),
+            oldWidth   = editor._codeMirror.defaultCharWidth();
+
+        editor.refreshAll();
+
+        // Since the height isnt updated after the font-size is changed, we just consider the rendered lines
+        // to calulate the new scroll position
+        var scrollPos   = editor.getScrollPos(),
+            viewportTop = $(".CodeMirror-lines", editor.getRootElement()).parent().position().top,
+            scrollTop   = scrollPos.y - viewportTop;
+
+        // Calcualte the new scroll based on the old font sizes and scroll position
+        var newWidth    = editor._codeMirror.defaultCharWidth(),
+            newHeight   = editor.getTextHeight(),
+            deltaX      = Math.round(scrollPos.x / oldWidth),
+            deltaY      = Math.round(scrollTop / oldHeight),
+            scrollPosX  = scrollPos.x + deltaX * (newWidth - oldWidth),
+            scrollPosY  = scrollPos.y + deltaY * (newHeight - oldHeight);
+
+        // Scroll the document back to its original position
+        editor.setScrollPos(scrollPosX, scrollPosY);
+    }
 
     /**
      * @private
      * Removes the styles used to update the font size and updates the editor if refresh is true
-     * @param {boolean} refresh - True to refresh the current full editor
+     * @param {!boolean} refresh - true to refresh the current full editor
      */
     function _removeDynamicFontSize(refresh) {
         $("#" + DYNAMIC_FONT_STYLE_ID).remove();
         if (refresh) {
-            EditorManager.getCurrentFullEditor().refreshAll();
+            _refreshAndRestoreScroll();
         }
     }
 
     /**
      * @private
      * Increases or decreases the editor's font size.
-     * @param {number} negative number to make the font smaller; positive number to make it bigger.
+     * @param {!number} adjustment - negative number to make the font smaller; positive number to make it bigger
      * @return {boolean} true if adjustment occurred, false if it did not occur 
      */
     function _adjustFontSize(adjustment) {
@@ -144,38 +173,26 @@ define(function (require, exports, module) {
                    "line-height: " + lhStr + " !important;}");
         $("head").append(style);
 
-        var editor = EditorManager.getCurrentFullEditor();
-        editor.refreshAll();
-
-        // Scroll the document back to its original position. This can only happen
-        // if the font size is specified in pixels (which it currently is).
-        if (fsUnits === "px") {
-            var scrollPos = editor.getScrollPos();
-            var scrollDeltaX = Math.round(scrollPos.x / lhOld);
-            var scrollDeltaY = Math.round(scrollPos.y / lhOld);
-
-            scrollDeltaX = (adjustment >= 0 ? scrollDeltaX : -scrollDeltaX);
-            scrollDeltaY = (adjustment >= 0 ? scrollDeltaY : -scrollDeltaY);
-
-            editor.setScrollPos((scrollPos.x + scrollDeltaX),
-                                (scrollPos.y + scrollDeltaY));
-        }
+        _refreshAndRestoreScroll();
 
         return true;
     }
 
+    /** Increses the font size by 1 */
     function _handleIncreaseFontSize() {
         if (_adjustFontSize(1)) {
             _prefs.setValue("fontSizeAdjustment", _prefs.getValue("fontSizeAdjustment") + 1);
         }
     }
-
+
+    /** Decreases the font size by 1 */
     function _handleDecreaseFontSize() {
         if (_adjustFontSize(-1)) {
             _prefs.setValue("fontSizeAdjustment", _prefs.getValue("fontSizeAdjustment") - 1);
         }
     }
 
+    /** Restores the font size to the original size */
     function _handleRestoreFontSize() {
         _removeDynamicFontSize(true);
         _prefs.setValue("fontSizeAdjustment", 0);
@@ -237,7 +254,7 @@ define(function (require, exports, module) {
     /**
      * @private
      * Scroll the viewport one line up or down.
-     * @param {number} -1 to scroll one line up; 1 to scroll one line down.
+     * @param {!number} -1 to scroll one line up; 1 to scroll one line down.
      */
     function _scrollLine(direction) {
         var editor        = EditorManager.getCurrentFullEditor(),
@@ -306,10 +323,12 @@ define(function (require, exports, module) {
         }
     }
 
+    /** Scrolls one line up */
     function _handleScrollLineUp() {
         _scrollLine(-1);
     }
 
+    /** Scrolls one line down */
     function _handleScrollLineDown() {
         _scrollLine(1);
     }

test/UnitTestSuite.js

@@ -24,7 +24,7 @@
 /*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */
 /*global define */
 define(function (require, exports, module) {
-    'use strict';
+    "use strict";
 
     require("spec/CodeHint-test");
     require("spec/CodeHintUtils-test");
@@ -57,6 +57,7 @@ define(function (require, exports, module) {
     require("spec/QuickOpen-test");
     require("spec/StringMatch-test");
     require("spec/UpdateNotification-test");
+    require("spec/ViewCommandHandlers-test");
     require("spec/ViewUtils-test");
     require("spec/WorkingSetView-test");
 });

test/spec/ViewCommandHandlers-test-files/test.css

@@ -0,0 +1,3 @@
+.testClass {
+  color: red;
+}

test/spec/ViewCommandHandlers-test-files/test.html

@@ -0,0 +1,11 @@
+<!doctype html>
+<html>
+<head>
+<title>Simple Test</title>
+<link rel="stylesheet" href="test.css">
+</head>
+
+<body>
+<p class="testClass">Brackets is awesome!</p>
+</body>
+</html>

test/spec/ViewCommandHandlers-test.js

@@ -0,0 +1,158 @@
+/*
+ * Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved.
+ *  
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"), 
+ * to deal in the Software without restriction, including without limitation 
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense, 
+ * and/or sell copies of the Software, and to permit persons to whom the 
+ * Software is furnished to do so, subject to the following conditions:
+ *  
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *  
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, 
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER 
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
+ * DEALINGS IN THE SOFTWARE.
+ * 
+ */
+
+/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
+/*global define, $, describe, beforeEach, afterEach, it, runs, waitsFor, expect, brackets, waitsForDone, document */
+
+define(function (require, exports, module) {
+    "use strict";
+
+    var CommandManager,      // loaded from brackets.test
+        Commands,            // loaded from brackets.test
+        EditorManager,       // loaded from brackets.test
+        DocumentManager,     // loaded from brackets.test
+        FileViewController,
+        SpecRunnerUtils     = require("spec/SpecRunnerUtils");
+
+    describe("ViewCommandHandlers", function () {
+        this.category = "integration";
+
+        var testPath = SpecRunnerUtils.getTestPath("/spec/ViewCommandHandlers-test-files"),
+            testWindow;
+
+        var CSS_FILE  = testPath + "/test.css",
+            HTML_FILE = testPath + "/test.html";
+
+        beforeEach(function () {
+            var promise;
+
+            // Create a new window that will be shared by ALL tests in this spec.
+            if (!testWindow) {
+                SpecRunnerUtils.createTestWindowAndRun(this, function (w) {
+                    testWindow = w;
+
+                    // Load module instances from brackets.test
+                    CommandManager      = testWindow.brackets.test.CommandManager;
+                    Commands            = testWindow.brackets.test.Commands;
+                    EditorManager       = testWindow.brackets.test.EditorManager;
+                    DocumentManager     = testWindow.brackets.test.DocumentManager;
+                    FileViewController  = testWindow.brackets.test.FileViewController;
+
+                    SpecRunnerUtils.loadProjectInTestWindow(testPath);
+                });
+
+                runs(function () {
+                    promise = CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, {fullPath: HTML_FILE});
+                    waitsForDone(promise, "Open into working set");
+                });
+
+                runs(function () {
+                    // Open inline editor onto test.css's ".testClass" rule
+                    promise = SpecRunnerUtils.toggleQuickEditAtOffset(EditorManager.getCurrentFullEditor(), {line: 8, ch: 11});
+                    waitsForDone(promise, "Open inline editor");
+                });
+            }
+        });
+
+        function getEditors() {
+            var editor = EditorManager.getCurrentFullEditor();
+            return {
+                editor: editor,
+                inline: editor.getInlineWidgets()[0].editors[0]
+            };
+        }
+
+
+        describe("Adjust the Font Size", function () {
+            it("should increase the font size in both editor and inline editor", function () {
+                runs(function () {
+                    var editors      = getEditors();
+                    var expectedSize = editors.editor.getTextHeight() + 2;
+
+                    CommandManager.execute(Commands.VIEW_INCREASE_FONT_SIZE);
+                    CommandManager.execute(Commands.VIEW_INCREASE_FONT_SIZE);
+
+                    expect(editors.editor.getTextHeight()).toBe(expectedSize);
+                    expect(editors.inline.getTextHeight()).toBe(expectedSize);
+                });
+            });
+
+            it("should decrease the font size in both editor and inline editor", function () {
+                runs(function () {
+                    var editors      = getEditors();
+                    var expectedSize = editors.editor.getTextHeight() - 2;
+
+                    CommandManager.execute(Commands.VIEW_DECREASE_FONT_SIZE);
+                    CommandManager.execute(Commands.VIEW_DECREASE_FONT_SIZE);
+
+                    expect(editors.editor.getTextHeight()).toBe(expectedSize);
+                    expect(editors.inline.getTextHeight()).toBe(expectedSize);
+                });
+            });
+
+            it("should restore the font size in both editor and inline editor", function () {
+                runs(function () {
+                    var editors      = getEditors();
+                    var expectedSize = editors.editor.getTextHeight();
+
+                    CommandManager.execute(Commands.VIEW_INCREASE_FONT_SIZE);
+                    CommandManager.execute(Commands.VIEW_INCREASE_FONT_SIZE);
+                    CommandManager.execute(Commands.VIEW_RESTORE_FONT_SIZE);
+
+                    expect(editors.editor.getTextHeight()).toBe(expectedSize);
+                    expect(editors.inline.getTextHeight()).toBe(expectedSize);
+                });
+            });
+
+            it("should keep the same font size when opening another document", function () {
+                var promise, expectedSize, editor;
+
+                runs(function () {
+                    editor       = EditorManager.getCurrentFullEditor();
+                    expectedSize = editor.getTextHeight() + 1;
+
+                    promise = CommandManager.execute(Commands.VIEW_INCREASE_FONT_SIZE);
+                    waitsForDone(promise, "Increase font size");
+                });
+
+                runs(function () {
+                    // Open another document and bring it to the front
+                    waitsForDone(FileViewController.openAndSelectDocument(CSS_FILE, FileViewController.PROJECT_MANAGER),
+                                 "FILE_OPEN on file timeout", 1000);
+                });
+
+                runs(function () {
+                    editor = EditorManager.getCurrentFullEditor();
+                    expect(editor.getTextHeight()).toBe(expectedSize);
+                });
+
+                // This must be in the last spec in the suite.
+                runs(function () {
+                    this.after(function () {
+                        SpecRunnerUtils.closeTestWindow();
+                    });
+                });
+            });
+        });
+    });
+});