From 638c85549326d11e75a5189f38a4f890002678c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Sat, 30 Mar 2013 23:25:45 -0300 Subject: [PATCH 1/4] Fixing the adjust font restore scroll and adding unit tests --- src/view/ViewCommandHandlers.js | 63 ++++--- test/UnitTestSuite.js | 3 +- .../ViewCommandHandlers-test-files/test.css | 3 + .../ViewCommandHandlers-test-files/test.html | 11 ++ test/spec/ViewCommandHandlers-test.js | 158 ++++++++++++++++++ 5 files changed, 215 insertions(+), 23 deletions(-) create mode 100644 test/spec/ViewCommandHandlers-test-files/test.css create mode 100644 test/spec/ViewCommandHandlers-test-files/test.html create mode 100644 test/spec/ViewCommandHandlers-test.js diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js index 47a240fe832..e1031ae01a7 100644 --- a/src/view/ViewCommandHandlers.js +++ b/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); } diff --git a/test/UnitTestSuite.js b/test/UnitTestSuite.js index 8b120b02894..00e9727ad8a 100644 --- a/test/UnitTestSuite.js +++ b/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"); }); \ No newline at end of file diff --git a/test/spec/ViewCommandHandlers-test-files/test.css b/test/spec/ViewCommandHandlers-test-files/test.css new file mode 100644 index 00000000000..935fd719d01 --- /dev/null +++ b/test/spec/ViewCommandHandlers-test-files/test.css @@ -0,0 +1,3 @@ +.testClass { + color: red; +} diff --git a/test/spec/ViewCommandHandlers-test-files/test.html b/test/spec/ViewCommandHandlers-test-files/test.html new file mode 100644 index 00000000000..74234aaf331 --- /dev/null +++ b/test/spec/ViewCommandHandlers-test-files/test.html @@ -0,0 +1,11 @@ + + + +Simple Test + + + + +

Brackets is awesome!

+ + \ No newline at end of file diff --git a/test/spec/ViewCommandHandlers-test.js b/test/spec/ViewCommandHandlers-test.js new file mode 100644 index 00000000000..fb2f77d4ec2 --- /dev/null +++ b/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(); + }); + }); + }); + }); + }); +}); From e178a8f32c0823ba2b5e8fe07312463a8ecf7294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Mon, 1 Apr 2013 21:22:20 -0300 Subject: [PATCH 2/4] Fixes after first review --- src/view/ViewCommandHandlers.js | 48 ++++++++++++++++----------- test/spec/ViewCommandHandlers-test.js | 6 ++-- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js index e1031ae01a7..c4e6a81120b 100644 --- a/src/view/ViewCommandHandlers.js +++ b/src/view/ViewCommandHandlers.js @@ -84,24 +84,24 @@ define(function (require, exports, module) { */ function _refreshAndRestoreScroll() { var editor = EditorManager.getCurrentFullEditor(), - oldHeight = editor.getTextHeight(), - oldWidth = editor._codeMirror.defaultCharWidth(); + oldWidth = editor._codeMirror.defaultCharWidth(), + oldHeight = editor.getTextHeight(); - 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 + // Since the height isn't updated after the font-size is changed, we just consider the rendered lines + // to calculate 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 + editor.refreshAll(); + + // Calculate 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); + deltaX = scrollPos.x / oldWidth, + deltaY = scrollTop / oldHeight, + scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)), + scrollPosY = scrollPos.y + Math.round(deltaY * (newHeight - oldHeight)); // Scroll the document back to its original position editor.setScrollPos(scrollPosX, scrollPosY); @@ -110,9 +110,17 @@ define(function (require, exports, module) { /** * @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) { + // This makes CodeMirror calculate the font sizes before the styles are removed so that we can + // get the correct old font size before applying the refresh + if (refresh) { + var editor = EditorManager.getCurrentFullEditor(); + editor._codeMirror.defaultCharWidth(); + editor.getTextHeight(); + } + $("#" + DYNAMIC_FONT_STYLE_ID).remove(); if (refresh) { _refreshAndRestoreScroll(); @@ -122,7 +130,7 @@ define(function (require, exports, module) { /** * @private * Increases or decreases the editor's font size. - * @param {!number} adjustment - 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) { @@ -178,7 +186,7 @@ define(function (require, exports, module) { return true; } - /** Increses the font size by 1 */ + /** Increases the font size by 1 */ function _handleIncreaseFontSize() { if (_adjustFontSize(1)) { _prefs.setValue("fontSizeAdjustment", _prefs.getValue("fontSizeAdjustment") + 1); @@ -233,10 +241,10 @@ define(function (require, exports, module) { /** * @private * Calculates the first and last visible lines of the focused editor - * @param {!number} textHeight - * @param {!number} scrollTop - * @param {!number} editorHeight - * @param {!number} viewportFrom + * @param {number} textHeight + * @param {number} scrollTop + * @param {number} editorHeight + * @param {number} viewportFrom * @return {{first: number, last: number}} */ function _getLinesInView(textHeight, scrollTop, editorHeight, viewportFrom) { @@ -254,7 +262,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} direction -1 to scroll one line up; 1 to scroll one line down. */ function _scrollLine(direction) { var editor = EditorManager.getCurrentFullEditor(), @@ -345,7 +353,7 @@ define(function (require, exports, module) { KeyBindingManager.addBinding(Commands.VIEW_SCROLL_LINE_UP); KeyBindingManager.addBinding(Commands.VIEW_SCROLL_LINE_DOWN); - // Init PreferenceStorage + // Initialize the PreferenceStorage _prefs = PreferencesManager.getPreferenceStorage(module, _defaultPrefs); // Update UI when opening or closing a document diff --git a/test/spec/ViewCommandHandlers-test.js b/test/spec/ViewCommandHandlers-test.js index fb2f77d4ec2..4e525b9b537 100644 --- a/test/spec/ViewCommandHandlers-test.js +++ b/test/spec/ViewCommandHandlers-test.js @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved. + * Copyright (c) 2013 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"), @@ -22,7 +22,7 @@ */ /*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 */ +/*global define, describe, beforeEach, it, runs, expect, waitsForDone */ define(function (require, exports, module) { "use strict"; @@ -30,7 +30,6 @@ define(function (require, exports, module) { 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"); @@ -55,7 +54,6 @@ define(function (require, exports, module) { 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); From 0d9c7d95077d2e57c937dbfbfd3b38fc63341b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Tue, 2 Apr 2013 19:54:29 -0300 Subject: [PATCH 3/4] Changes to the restore scroll --- src/view/ViewCommandHandlers.js | 60 ++++++++++++++------------------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js index c4e6a81120b..2e73598657e 100644 --- a/src/view/ViewCommandHandlers.js +++ b/src/view/ViewCommandHandlers.js @@ -80,9 +80,19 @@ define(function (require, exports, module) { /** * @private - * Refreshes the editor and restores the scroll position + * Removes the styles used to update the font size */ - function _refreshAndRestoreScroll() { + function _removeDynamicFontSize() { + $("#" + DYNAMIC_FONT_STYLE_ID).remove(); + } + + /** + * @private + * Sets the font size and restores the scroll position as best as possible + * @param {string} fsStr A string with the font size and the size unit + * @param {string} lhStr A string with the line height and a the size unit + */ + function _setSizeAndRestoreScroll(fsStr, lhStr) { var editor = EditorManager.getCurrentFullEditor(), oldWidth = editor._codeMirror.defaultCharWidth(), oldHeight = editor.getTextHeight(); @@ -93,6 +103,14 @@ define(function (require, exports, module) { viewportTop = $(".CodeMirror-lines", editor.getRootElement()).parent().position().top, scrollTop = scrollPos.y - viewportTop; + // It's necessary to inject a new rule to address all editors. + _removeDynamicFontSize(); + var style = $("").attr("id", DYNAMIC_FONT_STYLE_ID); + style.html(".CodeMirror {" + + "font-size: " + fsStr + " !important;" + + "line-height: " + lhStr + " !important;}"); + $("head").append(style); + editor.refreshAll(); // Calculate the new scroll based on the old font sizes and scroll position @@ -103,27 +121,9 @@ define(function (require, exports, module) { scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)), scrollPosY = scrollPos.y + Math.round(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 - */ - function _removeDynamicFontSize(refresh) { - // This makes CodeMirror calculate the font sizes before the styles are removed so that we can - // get the correct old font size before applying the refresh - if (refresh) { - var editor = EditorManager.getCurrentFullEditor(); - editor._codeMirror.defaultCharWidth(); - editor.getTextHeight(); - } - - $("#" + DYNAMIC_FONT_STYLE_ID).remove(); - if (refresh) { - _refreshAndRestoreScroll(); + // Scroll the document back to its original position, but not on the first load + if (_fontSizePrefsLoaded) { + editor.setScrollPos(scrollPosX, scrollPosY); } } @@ -173,15 +173,7 @@ define(function (require, exports, module) { return false; } - // It's necessary to inject a new rule to address all editors. - _removeDynamicFontSize(false); - var style = $("").attr("id", DYNAMIC_FONT_STYLE_ID); - style.html(".CodeMirror {" + - "font-size: " + fsStr + " !important;" + - "line-height: " + lhStr + " !important;}"); - $("head").append(style); - - _refreshAndRestoreScroll(); + _setSizeAndRestoreScroll(fsStr, lhStr); return true; } @@ -202,7 +194,7 @@ define(function (require, exports, module) { /** Restores the font size to the original size */ function _handleRestoreFontSize() { - _removeDynamicFontSize(true); + _adjustFontSize(-_prefs.getValue("fontSizeAdjustment")); _prefs.setValue("fontSizeAdjustment", 0); } @@ -223,7 +215,7 @@ define(function (require, exports, module) { // Font Size preferences only need to be loaded one time if (!_fontSizePrefsLoaded) { - _removeDynamicFontSize(false); + _removeDynamicFontSize(); _adjustFontSize(_prefs.getValue("fontSizeAdjustment")); _fontSizePrefsLoaded = true; } From c35ff520b675e1daa8424c5d54eef185d6a7f1fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Wed, 3 Apr 2013 20:29:41 -0300 Subject: [PATCH 4/4] Doc fixes --- src/view/ViewCommandHandlers.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js index 2e73598657e..47dd53eb18b 100644 --- a/src/view/ViewCommandHandlers.js +++ b/src/view/ViewCommandHandlers.js @@ -88,18 +88,16 @@ define(function (require, exports, module) { /** * @private - * Sets the font size and restores the scroll position as best as possible - * @param {string} fsStr A string with the font size and the size unit - * @param {string} lhStr A string with the line height and a the size unit + * Sets the font size and restores the scroll position as best as possible. + * TODO: Remove the viewportTop hack and direclty use scrollPos.y once #3115 is fixed. + * @param {string} fontSizeStyle A string with the font size and the size unit + * @param {string} lineHeightStyle A string with the line height and a the size unit */ - function _setSizeAndRestoreScroll(fsStr, lhStr) { - var editor = EditorManager.getCurrentFullEditor(), - oldWidth = editor._codeMirror.defaultCharWidth(), - oldHeight = editor.getTextHeight(); - - // Since the height isn't updated after the font-size is changed, we just consider the rendered lines - // to calculate the new scroll position - var scrollPos = editor.getScrollPos(), + function _setSizeAndRestoreScroll(fontSizeStyle, lineHeightStyle) { + var editor = EditorManager.getCurrentFullEditor(), + oldWidth = editor._codeMirror.defaultCharWidth(), + oldHeight = editor.getTextHeight(), + scrollPos = editor.getScrollPos(), viewportTop = $(".CodeMirror-lines", editor.getRootElement()).parent().position().top, scrollTop = scrollPos.y - viewportTop; @@ -107,8 +105,8 @@ define(function (require, exports, module) { _removeDynamicFontSize(); var style = $("").attr("id", DYNAMIC_FONT_STYLE_ID); style.html(".CodeMirror {" + - "font-size: " + fsStr + " !important;" + - "line-height: " + lhStr + " !important;}"); + "font-size: " + fontSizeStyle + " !important;" + + "line-height: " + lineHeightStyle + " !important;}"); $("head").append(style); editor.refreshAll(); @@ -121,7 +119,8 @@ define(function (require, exports, module) { scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth - oldWidth)), scrollPosY = scrollPos.y + Math.round(deltaY * (newHeight - oldHeight)); - // Scroll the document back to its original position, but not on the first load + // Scroll the document back to its original position, but not on the first load since the position + // was saved with the new height and already been restored. if (_fontSizePrefsLoaded) { editor.setScrollPos(scrollPosX, scrollPosY); }