From 859119fdfc05a6811239605a613fc7cb17fc2c25 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Sun, 15 Sep 2013 12:56:14 +0200 Subject: [PATCH] Code review: cleaner way to check for inline editors, hoist out a const, tweak comments, narrower tickmarks on Mac (per Larz), go back to window.* references in Async. --- src/editor/Editor.js | 5 +++++ src/search/FindReplace.js | 2 +- src/search/ScrollTrackMarkers.js | 9 ++++++--- src/styles/brackets.less | 1 + src/utils/Async.js | 13 +++++++------ 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index b4369836ecc..9dc28b7afab 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -506,6 +506,11 @@ define(function (require, exports, module) { }); }; + /** @return {boolean} True if editor is not showing the entire text of the document (i.e. an inline editor) */ + Editor.prototype.isTextSubset = function () { + return Boolean(this._visibleRange); + }; + /** * Ensures that the lines that are actually hidden in the inline editor correspond to * the desired visible range. diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index 1adb3f8fdee..0813ed2ff0f 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -333,7 +333,7 @@ define(function (require, exports, module) { // Clear highlights but leave search state in place so Find Next/Previous work after closing clearHighlights(cm, state); - // As soon as focus goes back to the editor, restore normal selection color + // Dispose highlighting UI (important to restore normal selection color as soon as focus goes back to the editor) toggleHighlighting(editor, false); }); diff --git a/src/search/ScrollTrackMarkers.js b/src/search/ScrollTrackMarkers.js index 2ae90d2b158..6e742236b39 100644 --- a/src/search/ScrollTrackMarkers.js +++ b/src/search/ScrollTrackMarkers.js @@ -38,6 +38,9 @@ define(function (require, exports, module) { Async = require("utils/Async"); + /** @const @type {number} Height (and width) or scrollbar up/down arrow button on Win */ + var WIN_ARROW_HT = 17; + /** @type {?Editor} Editor the markers are currently shown for, or null if not shown */ var editor; @@ -61,9 +64,9 @@ define(function (require, exports, module) { if (trackHt > 0) { // Scrollbar visible: determine offset of track from top of scrollbar if (brackets.platform === "win") { - trackOffset = 17; // Up arrow pushes down track + trackOffset = WIN_ARROW_HT; // Up arrow pushes down track } else { - trackOffset = 0; // No arrows + trackOffset = 0; // No arrows } } else { @@ -109,7 +112,7 @@ define(function (require, exports, module) { editor = curEditor; // Don't support inline editors yet - search inside them is pretty screwy anyway (#2110) - if (editor.getFirstVisibleLine() > 0 || editor.getLastVisibleLine() < editor.lineCount() - 1) { + if (editor.isTextSubset()) { return; } diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 68ad144ed3b..b8b22009ef4 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -938,6 +938,7 @@ a, img { .tickmark { position: absolute; width: 16px; + body.platform-mac & { width: 15px; } height: 1px; background-color: #eddd23; diff --git a/src/utils/Async.js b/src/utils/Async.js index f2ce2448c38..95b96669f97 100644 --- a/src/utils/Async.js +++ b/src/utils/Async.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, setTimeout, clearTimeout */ +/*global define, $, window */ /** * Utilities for working with Deferred, Promise, and other asynchronous processes. @@ -225,7 +225,7 @@ define(function (require, exports, module) { // if we've exhausted our maxBlockingTime if ((new Date()).getTime() - sliceStartTime >= maxBlockingTime) { //yield - setTimeout(function () { + window.setTimeout(function () { sliceStartTime = (new Date()).getTime(); result.resolve(); }, idleTime); @@ -301,11 +301,11 @@ define(function (require, exports, module) { function withTimeout(promise, timeout) { var wrapper = new $.Deferred(); - var timer = setTimeout(function () { + var timer = window.setTimeout(function () { wrapper.reject(ERROR_TIMEOUT); }, timeout); promise.always(function () { - clearTimeout(timer); + window.clearTimeout(timer); }); // If the wrapper was already rejected due to timeout, the Promise's calls to resolve/reject @@ -429,14 +429,15 @@ define(function (require, exports, module) { * * @param {number} idleDelay Minimum delay (ms) before invoking callback. * @param {!function()} callback + * @return {!function()} */ function whenIdle(idleDelay, callback) { var timer; return function () { if (timer) { - clearTimeout(timer); + window.clearTimeout(timer); } - timer = setTimeout(function () { + timer = window.setTimeout(function () { timer = null; callback(); }, idleDelay);