From cd0ce908d730b1fd0ddf029c5545800df9307ca5 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Tue, 21 Jan 2014 17:34:55 -0800 Subject: [PATCH 01/17] display erorr message --- src/editor/Editor.js | 87 +++++++++++++++++++++++++++++++++++++ src/editor/EditorManager.js | 46 ++++++++++++++++---- src/nls/root/strings.js | 6 +++ src/styles/brackets.less | 39 +++++++++++++++++ 4 files changed, 169 insertions(+), 9 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 80c84037d95..a47eb06b404 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1250,6 +1250,93 @@ define(function (require, exports, module) { return this._inlineWidgets; }; + /** + * Display temporary popover message at current cursor position. Display message above + * cursor if space allows, otherwise below. + * + * @param {string} errorMsg Error message to display + */ + Editor.prototype.displayErrorMessageAtCursor = function (errorMsg) { + var $messagePopover, arrowBelow, cursorPos, cursorCoord, popoverRect, + top, left, clip, arrowLeft, + $editorHolder = $("#editor-holder"), + POPOVER_MARGIN = 10, + POPOVER_ARROW_HALF_WIDTH = 10; + + function _clearMessagePopover() { + $messagePopover.remove(); + $messagePopover = null; + } + + if ($messagePopover) { + window.clearTimeout(_clearMessagePopover); + _clearMessagePopover(); + } + + // Determine if arrow is above or below + cursorPos = this.getCursorPos(); + cursorCoord = this._codeMirror.charCoords(cursorPos); + + // Assume popover height is max of 2 lines + arrowBelow = (cursorCoord.top > 100); + + // Text is dynamic, so build popover first so we can measure final width + $messagePopover = $("
").addClass("popover-message").appendTo($("body")); + if (!arrowBelow) { + $("
").addClass("arrowAbove").appendTo($messagePopover); + } + $("
").addClass("text").appendTo($messagePopover).text(errorMsg); + if (arrowBelow) { + $("
").addClass("arrowBelow").appendTo($messagePopover); + } + + // Estimate where to position popover. + top = (arrowBelow) + ? cursorCoord.top - $messagePopover.height() - POPOVER_MARGIN + : cursorCoord.bottom + POPOVER_MARGIN; + left = cursorCoord.left - ($messagePopover.width() / 2); + + popoverRect = { + top: top, + left: left, + height: $messagePopover.height(), + width: $messagePopover.width() + }; + + // See if popover is clipped on any side + clip = ViewUtils.getElementClipSize($editorHolder, popoverRect); + + // Prevent horizontal clipping + if (clip.left > 0) { + left += clip.left; + } else if (clip.right > 0) { + left -= clip.right; + } + + // Popover text and arrow are positioned individually + $messagePopover.css({"visibility": "visible", "top": top, "left": left}); + + // Position popover arrow exactly centered over/under cursor + arrowLeft = cursorCoord.left - left - POPOVER_ARROW_HALF_WIDTH; + if (arrowBelow) { + $messagePopover.find(".arrowBelow").css({"margin-left": arrowLeft}); + } else { + $messagePopover.find(".arrowAbove").css({"margin-left": arrowLeft}); + } + +// AnimationUtils.animateUsingClass($messagePopover[0], "animating") +// .done(_clearMessagePopover); + + // TODO: + // - add shadow to "arrow" + // - css to fade out after 3-4 sec + // - clear message on scroll?, change editors?, click in page?, typing? + // - test conflicts with other popovers, menus, dropdowns + + // Destroy after 4 sec + window.setTimeout(_clearMessagePopover, 4000); + }; + /** * Returns the offset of the top of the virtual scroll area relative to the browser window (not the editor * itself). Mainly useful for calculations related to scrollIntoView(), where you're starting with the diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index 427b3fb4d85..fdf71d934ce 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -160,24 +160,48 @@ define(function (require, exports, module) { * @private * Finds an inline widget provider from the given list that can offer a widget for the current cursor * position, and once the widget has been created inserts it into the editor. + * * @param {!Editor} editor The host editor - * @param {!Array.<{priority:number, provider:function(!Editor, !{line:number, ch:number}):?$.Promise}>} prioritized providers + * @param {Array.<{priority:number, provider:function(...)}>} providers + * prioritized list of providers + * @param {string=} defaultErrorMsg Default error message to display if no initial provider found * @return {$.Promise} a promise that will be resolved when an InlineWidget * is created or rejected if no inline providers have offered one. */ - function _openInlineWidget(editor, providers) { + function _openInlineWidget(editor, providers, defaultErrorMsg) { PerfUtils.markStart(PerfUtils.INLINE_WIDGET_OPEN); // Run through inline-editor providers until one responds var pos = editor.getCursorPos(), inlinePromise, i, - result = new $.Deferred(); + result = new $.Deferred(), + errorMsg, + providerRet; + // Query each provider in priority order. Provider may return: + // 1. `null` to indicate it does not apply to current cursor position + // 2. promise that should resolve to an InlineWidget + // 3. string which indicates provider does apply to current cursor position, + // but reason it could not create InlineWidget + // + // Keep looping until a provider is found. If a provider is not found, + // display highest priority error message that was found, otherwise display + // default error message for (i = 0; i < providers.length && !inlinePromise; i++) { var provider = providers[i].provider; - inlinePromise = provider(editor, pos); + providerRet = provider(editor, pos); + if (providerRet) { + if (providerRet.hasOwnProperty("done")) { + inlinePromise = providerRet; + } else if (!errorMsg && typeof (providerRet) === "string") { + errorMsg = providerRet; + } + } } + + // Use default error message if none other provided + errorMsg = errorMsg || defaultErrorMsg; // If one of them will provide a widget, show it inline once ready if (inlinePromise) { @@ -189,11 +213,13 @@ define(function (require, exports, module) { }).fail(function () { // terminate timer that was started above PerfUtils.finalizeMeasurement(PerfUtils.INLINE_WIDGET_OPEN); + editor.displayErrorMessageAtCursor(errorMsg); result.reject(); }); } else { // terminate timer that was started above PerfUtils.finalizeMeasurement(PerfUtils.INLINE_WIDGET_OPEN); + editor.displayErrorMessageAtCursor(errorMsg); result.reject(); } @@ -911,12 +937,14 @@ define(function (require, exports, module) { /** * Closes any focused inline widget. Else, asynchronously asks providers to create one. * - * @param {Array.<{priority:number, provider:function(...)}>} prioritized providers + * @param {Array.<{priority:number, provider:function(...)}>} providers + * prioritized list of providers + * @param {string=} errorMsg Error message to display if no initial provider found * @return {!Promise} A promise resolved with true if an inline widget is opened or false * when closed. Rejected if there is neither an existing widget to close nor a provider * willing to create a widget (or if no editor is open). */ - function _toggleInlineWidget(providers) { + function _toggleInlineWidget(providers, errorMsg) { var result = new $.Deferred(); if (_currentEditor) { @@ -932,7 +960,7 @@ define(function (require, exports, module) { }); } else { // main editor has focus, so create an inline editor - _openInlineWidget(_currentEditor, providers).done(function () { + _openInlineWidget(_currentEditor, providers, errorMsg).done(function () { result.resolve(true); }).fail(function () { result.reject(); @@ -1000,10 +1028,10 @@ define(function (require, exports, module) { // Initialize: command handlers CommandManager.register(Strings.CMD_TOGGLE_QUICK_EDIT, Commands.TOGGLE_QUICK_EDIT, function () { - return _toggleInlineWidget(_inlineEditProviders); + return _toggleInlineWidget(_inlineEditProviders, Strings.ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND); }); CommandManager.register(Strings.CMD_TOGGLE_QUICK_DOCS, Commands.TOGGLE_QUICK_DOCS, function () { - return _toggleInlineWidget(_inlineDocsProviders); + return _toggleInlineWidget(_inlineDocsProviders, Strings.ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND); }); CommandManager.register(Strings.CMD_JUMPTO_DEFINITION, Commands.NAVIGATE_JUMPTO_DEFINITION, _doJumpToDef); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 39591c369bb..12d2738a890 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -166,6 +166,12 @@ define({ "ERROR_FETCHING_UPDATE_INFO_TITLE" : "Error getting update info", "ERROR_FETCHING_UPDATE_INFO_MSG" : "There was a problem getting the latest update information from the server. Please make sure you are connected to the internet and try again.", + // Quick Edit + "ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND" : "No Quick Edit provider found for current cursor position", + + // Quick Docs + "ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND" : "No Quick Docs provider found for current cursor position", + /** * ProjectManager */ diff --git a/src/styles/brackets.less b/src/styles/brackets.less index eb440678bb1..181a83f22a6 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -1382,3 +1382,42 @@ label input { .install-extension-dialog .spinner { margin-left: 10px; } + +/* Quick Edit, Quick Docs */ +.popover-message { + position: absolute; + top: 0; + left: 0; + z-index: 1000; // TODO: put in @var + visibility: hidden; + opacity: 1.0; + + .text { + padding: 5px 10px; + background-color: #f74687; // TODO: put in @var + border-radius: 3px; + color: @bc-white; + box-shadow: @tc-dropdown-shadow; + white-space: nowrap; + } + .arrowAbove { + height: 0; + width: 0; + border-width: 0 10px 10px 10px; + border-color: transparent transparent #f74687 transparent; // TODO: put in @var + border-style: solid; + } + .arrowBelow { + height: 0; + width: 0; + border-width: 10px 10px 0 10px; + border-color: #f74687 transparent transparent transparent; // TODO: put in @var + border-style: solid; + } + &.animating { + // Make the animation use the GPU--especially important for retina. + -webkit-transform: translateZ(0); + transform: translateZ(0); + transition: 1s 3s opacity ease-out; + } +} From 03dd575278a7d6f03428f8601218112beadf7e45 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Tue, 21 Jan 2014 19:31:28 -0800 Subject: [PATCH 02/17] only 1 message at a time; add opacity transition --- src/editor/Editor.js | 44 ++++++++++++++++++++-------------------- src/styles/brackets.less | 3 ++- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index a47eb06b404..cc0c38922b2 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -348,6 +348,7 @@ define(function (require, exports, module) { // (if makeMasterEditor, we attach the Doc back to ourselves below once we're fully initialized) this._inlineWidgets = []; + this._$messagePopover = null; // Editor supplies some standard keyboard behavior extensions of its own var codeMirrorKeyMap = { @@ -1257,19 +1258,22 @@ define(function (require, exports, module) { * @param {string} errorMsg Error message to display */ Editor.prototype.displayErrorMessageAtCursor = function (errorMsg) { - var $messagePopover, arrowBelow, cursorPos, cursorCoord, popoverRect, + var arrowBelow, cursorPos, cursorCoord, popoverRect, top, left, clip, arrowLeft, + self = this, $editorHolder = $("#editor-holder"), POPOVER_MARGIN = 10, POPOVER_ARROW_HALF_WIDTH = 10; function _clearMessagePopover() { - $messagePopover.remove(); - $messagePopover = null; + if (self._$messagePopover) { + self._$messagePopover.remove(); + self._$messagePopover = null; + } } - if ($messagePopover) { - window.clearTimeout(_clearMessagePopover); + // Only 1 message at a time + if (this._$messagePopover) { _clearMessagePopover(); } @@ -1281,26 +1285,26 @@ define(function (require, exports, module) { arrowBelow = (cursorCoord.top > 100); // Text is dynamic, so build popover first so we can measure final width - $messagePopover = $("
").addClass("popover-message").appendTo($("body")); + this._$messagePopover = $("
").addClass("popover-message").appendTo($("body")); if (!arrowBelow) { - $("
").addClass("arrowAbove").appendTo($messagePopover); + $("
").addClass("arrowAbove").appendTo(this._$messagePopover); } - $("
").addClass("text").appendTo($messagePopover).text(errorMsg); + $("
").addClass("text").appendTo(this._$messagePopover).text(errorMsg); if (arrowBelow) { - $("
").addClass("arrowBelow").appendTo($messagePopover); + $("
").addClass("arrowBelow").appendTo(this._$messagePopover); } // Estimate where to position popover. top = (arrowBelow) - ? cursorCoord.top - $messagePopover.height() - POPOVER_MARGIN + ? cursorCoord.top - this._$messagePopover.height() - POPOVER_MARGIN : cursorCoord.bottom + POPOVER_MARGIN; - left = cursorCoord.left - ($messagePopover.width() / 2); + left = cursorCoord.left - (this._$messagePopover.width() / 2); popoverRect = { top: top, left: left, - height: $messagePopover.height(), - width: $messagePopover.width() + height: this._$messagePopover.height(), + width: this._$messagePopover.width() }; // See if popover is clipped on any side @@ -1314,27 +1318,23 @@ define(function (require, exports, module) { } // Popover text and arrow are positioned individually - $messagePopover.css({"visibility": "visible", "top": top, "left": left}); + this._$messagePopover.css({"visibility": "visible", "top": top, "left": left}); // Position popover arrow exactly centered over/under cursor arrowLeft = cursorCoord.left - left - POPOVER_ARROW_HALF_WIDTH; if (arrowBelow) { - $messagePopover.find(".arrowBelow").css({"margin-left": arrowLeft}); + this._$messagePopover.find(".arrowBelow").css({"margin-left": arrowLeft}); } else { - $messagePopover.find(".arrowAbove").css({"margin-left": arrowLeft}); + this._$messagePopover.find(".arrowAbove").css({"margin-left": arrowLeft}); } -// AnimationUtils.animateUsingClass($messagePopover[0], "animating") -// .done(_clearMessagePopover); + AnimationUtils.animateUsingClass(this._$messagePopover[0], "animating") + .done(_clearMessagePopover); // TODO: // - add shadow to "arrow" - // - css to fade out after 3-4 sec // - clear message on scroll?, change editors?, click in page?, typing? // - test conflicts with other popovers, menus, dropdowns - - // Destroy after 4 sec - window.setTimeout(_clearMessagePopover, 4000); }; /** diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 181a83f22a6..e0e09749e19 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -1415,9 +1415,10 @@ label input { border-style: solid; } &.animating { + opacity: 0.0; // Make the animation use the GPU--especially important for retina. -webkit-transform: translateZ(0); transform: translateZ(0); - transition: 1s 3s opacity ease-out; + transition: 500ms 3s opacity ease-in; } } From 86de431500b72687da66c87ff65ac74b50a3193c Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 22 Jan 2014 08:23:45 -0800 Subject: [PATCH 03/17] support for css quick edit --- src/editor/CSSInlineEditor.js | 26 +++++++++++++++++++++----- src/editor/Editor.js | 2 +- src/nls/root/strings.js | 3 +++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index 89550074b3f..b271dfc9522 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -59,11 +59,14 @@ define(function (require, exports, module) { * Given a position in an HTML editor, returns the relevant selector for the attribute/tag * surrounding that position, or "" if none is found. * @param {!Editor} editor + * @param {!{line:Number, ch:Number}} pos + * @return {selectorName: {string}, reason: {string}} * @private */ function _getSelectorName(editor, pos) { var tagInfo = HTMLUtils.getTagInfo(editor, pos), - selectorName = ""; + selectorName = "", + reason; if (tagInfo.position.tokenType === HTMLUtils.TAG_NAME || tagInfo.position.tokenType === HTMLUtils.CLOSING_TAG) { // Type selector @@ -90,16 +93,27 @@ define(function (require, exports, module) { if (selectorName === ".") { selectorName = ""; } + + if (selectorName === "") { + reason = Strings.ERROR_CSSQUICKEDIT_CLASSNOTFOUND; + } } else if (tagInfo.attr.name === "id") { // ID selector var trimmedVal = tagInfo.attr.value.trim(); if (trimmedVal) { selectorName = "#" + trimmedVal; + } else { + reason = Strings.ERROR_CSSQUICKEDIT_IDNOTFOUND; } + } else { + reason = Strings.ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR; } } - return selectorName; + return { + selectorName: selectorName, + reason: reason + }; } /** @@ -172,10 +186,12 @@ define(function (require, exports, module) { // Always use the selection start for determining selector name. The pos // parameter is usually the selection end. - var selectorName = _getSelectorName(hostEditor, sel.start); - if (selectorName === "") { - return null; + var selectorResult = _getSelectorName(hostEditor, sel.start); + if (selectorResult.selectorName === "") { + return selectorResult.reason || null; } + + var selectorName = selectorResult.selectorName; var result = new $.Deferred(), cssInlineEditor, diff --git a/src/editor/Editor.js b/src/editor/Editor.js index cc0c38922b2..3464dc8be9a 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1289,7 +1289,7 @@ define(function (require, exports, module) { if (!arrowBelow) { $("
").addClass("arrowAbove").appendTo(this._$messagePopover); } - $("
").addClass("text").appendTo(this._$messagePopover).text(errorMsg); + $("
").addClass("text").appendTo(this._$messagePopover).html(errorMsg); if (arrowBelow) { $("
").addClass("arrowBelow").appendTo(this._$messagePopover); } diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 12d2738a890..4a840d83f5c 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -168,6 +168,9 @@ define({ // Quick Edit "ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND" : "No Quick Edit provider found for current cursor position", + "ERROR_CSSQUICKEDIT_CLASSNOTFOUND" : "CSS Quick Edit: place cursor in class name", + "ERROR_CSSQUICKEDIT_IDNOTFOUND" : "CSS Quick Edit: place cursor in id name", + "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag name, class name, or id name", // Quick Docs "ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND" : "No Quick Docs provider found for current cursor position", From 26086ad7cf7beab218ea3b0b9360bd68afdab331 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 22 Jan 2014 08:52:25 -0800 Subject: [PATCH 04/17] support for timing function quick edit --- .../default/InlineTimingFunctionEditor/main.js | 16 +++++++--------- src/nls/root/strings.js | 1 + 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/extensions/default/InlineTimingFunctionEditor/main.js b/src/extensions/default/InlineTimingFunctionEditor/main.js index ab319e118aa..f76a794537d 100644 --- a/src/extensions/default/InlineTimingFunctionEditor/main.js +++ b/src/extensions/default/InlineTimingFunctionEditor/main.js @@ -65,7 +65,7 @@ define(function (require, exports, module) { * * @param {Editor} hostEditor * @param {{line:Number, ch:Number}} pos - * @return {?{color:String, start:TextMarker, end:TextMarker}} + * @return {timingFunction:{?string}, reason:{?string}, start:{?TextMarker}, end:{?TextMarker}} */ function prepareEditorForProvider(hostEditor, pos) { var cursorLine, sel, startPos, endPos, startBookmark, endBookmark, currentMatch, @@ -73,7 +73,7 @@ define(function (require, exports, module) { sel = hostEditor.getSelection(); if (sel.start.line !== sel.end.line) { - return null; + return {timingFunction: null, reason: null}; } cursorLine = hostEditor.document.getLine(pos.line); @@ -81,12 +81,12 @@ define(function (require, exports, module) { // code runs several matches complicated patterns, multiple times, so // first do a quick, simple check to see make sure we may have a match if (!cursorLine.match(/cubic-bezier|linear|ease|step/)) { - return null; + return {timingFunction: null, reason: null}; } currentMatch = TimingFunctionUtils.timingFunctionMatch(cursorLine, false); if (!currentMatch) { - return null; + return {timingFunction: null, reason: Strings.ERROR_TIMINGQUICKEDIT_INVALIDSYNTAX}; } // check for subsequent matches, and use first match after pos @@ -129,15 +129,15 @@ define(function (require, exports, module) { * @param {!Editor} hostEditor * @param {!{line:Number, ch:Number}} pos * @return {?$.Promise} synchronously resolved with an InlineWidget, or null if there's - * no color at pos. + * no timing function at pos. */ function inlineTimingFunctionEditorProvider(hostEditor, pos) { var context = prepareEditorForProvider(hostEditor, pos), inlineTimingFunctionEditor, result; - if (!context) { - return null; + if (!context.timingFunction) { + return context.reason || null; } else { inlineTimingFunctionEditor = new InlineTimingFunctionEditor(context.timingFunction, context.start, context.end); inlineTimingFunctionEditor.load(hostEditor); @@ -161,8 +161,6 @@ define(function (require, exports, module) { init(); - // for use by other InlineColorEditors - exports.prepareEditorForProvider = prepareEditorForProvider; // for unit tests only exports.inlineTimingFunctionEditorProvider = inlineTimingFunctionEditorProvider; diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 4a840d83f5c..c51152df3f3 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -171,6 +171,7 @@ define({ "ERROR_CSSQUICKEDIT_CLASSNOTFOUND" : "CSS Quick Edit: place cursor in class name", "ERROR_CSSQUICKEDIT_IDNOTFOUND" : "CSS Quick Edit: place cursor in id name", "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag name, class name, or id name", + "ERROR_TIMINGQUICKEDIT_INVALIDSYNTAX" : "CSS Timing Function Quick Edit: invalid syntax", // Quick Docs "ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND" : "No Quick Docs provider found for current cursor position", From d105d47c3b98ab4ec223af2f6f400607a42ed46c Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 22 Jan 2014 08:55:18 -0800 Subject: [PATCH 05/17] update comment for timing function quick edit --- src/extensions/default/InlineTimingFunctionEditor/main.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/extensions/default/InlineTimingFunctionEditor/main.js b/src/extensions/default/InlineTimingFunctionEditor/main.js index f76a794537d..86b24824d06 100644 --- a/src/extensions/default/InlineTimingFunctionEditor/main.js +++ b/src/extensions/default/InlineTimingFunctionEditor/main.js @@ -128,8 +128,9 @@ define(function (require, exports, module) { * * @param {!Editor} hostEditor * @param {!{line:Number, ch:Number}} pos - * @return {?$.Promise} synchronously resolved with an InlineWidget, or null if there's - * no timing function at pos. + * @return {?$.Promise} synchronously resolved with an InlineWidget, or + * {string} if timing function with invalid syntax is detected at pos, or + * null if there's no timing function at pos. */ function inlineTimingFunctionEditorProvider(hostEditor, pos) { var context = prepareEditorForProvider(hostEditor, pos), From 90405d2a3414d29eafbb0d6981fe2ad20d32b375 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 22 Jan 2014 09:12:06 -0800 Subject: [PATCH 06/17] support for js quick edit --- .../default/JavaScriptQuickEdit/main.js | 23 ++++++++++++------- src/nls/root/strings.js | 1 + 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/extensions/default/JavaScriptQuickEdit/main.js b/src/extensions/default/JavaScriptQuickEdit/main.js index 0634c8bc6c5..722805c272b 100644 --- a/src/extensions/default/JavaScriptQuickEdit/main.js +++ b/src/extensions/default/JavaScriptQuickEdit/main.js @@ -34,14 +34,15 @@ define(function (require, exports, module) { DocumentManager = brackets.getModule("document/DocumentManager"), JSUtils = brackets.getModule("language/JSUtils"), PerfUtils = brackets.getModule("utils/PerfUtils"), - ProjectManager = brackets.getModule("project/ProjectManager"); + ProjectManager = brackets.getModule("project/ProjectManager"), + Strings = brackets.getModule("strings"); /** * Return the token string that is at the specified position. * * @param hostEditor {!Editor} editor * @param {!{line:Number, ch:Number}} pos - * @return {String} token string at the specified position + * @return {functionName: {string}, reason: {string}} */ function _getFunctionName(hostEditor, pos) { var token = hostEditor._codeMirror.getTokenAt(pos, true); @@ -56,10 +57,16 @@ define(function (require, exports, module) { if (!((token.type === "variable") || (token.type === "variable-2") || (token.type === "property"))) { - return null; + return { + functionName: null, + reason: Strings.ERROR_JSQUICKEDIT_FUNCTIONNOTFOUND + }; } - return token.string; + return { + functionName: token.string, + reason: null + }; } /** @@ -196,12 +203,12 @@ define(function (require, exports, module) { // Always use the selection start for determining the function name. The pos // parameter is usually the selection end. - var functionName = _getFunctionName(hostEditor, sel.start); - if (!functionName) { - return null; + var functionResult = _getFunctionName(hostEditor, sel.start); + if (!functionResult.functionName) { + return functionResult.reason || null; } - return _createInlineEditor(hostEditor, functionName); + return _createInlineEditor(hostEditor, functionResult.functionName); } // init diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index c51152df3f3..d7a1713f1a7 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -172,6 +172,7 @@ define({ "ERROR_CSSQUICKEDIT_IDNOTFOUND" : "CSS Quick Edit: place cursor in id name", "ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR" : "CSS Quick Edit: place cursor in tag name, class name, or id name", "ERROR_TIMINGQUICKEDIT_INVALIDSYNTAX" : "CSS Timing Function Quick Edit: invalid syntax", + "ERROR_JSQUICKEDIT_FUNCTIONNOTFOUND" : "JS Quick Edit: place cursor in function name", // Quick Docs "ERROR_QUICK_DOCS_PROVIDER_NOT_FOUND" : "No Quick Docs provider found for current cursor position", From 769cbc4b7942d5f364dbba8496b54fa4293998a3 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 22 Jan 2014 09:22:36 -0800 Subject: [PATCH 07/17] update comments --- src/editor/CSSInlineEditor.js | 5 +++-- src/extensions/default/JavaScriptQuickEdit/main.js | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/editor/CSSInlineEditor.js b/src/editor/CSSInlineEditor.js index b271dfc9522..35ef1269ed8 100644 --- a/src/editor/CSSInlineEditor.js +++ b/src/editor/CSSInlineEditor.js @@ -168,8 +168,9 @@ define(function (require, exports, module) { * * @param {!Editor} editor * @param {!{line:Number, ch:Number}} pos - * @return {$.Promise} a promise that will be resolved with an InlineWidget - * or null if we're not going to provide anything. + * @return {?$.Promise} synchronously resolved with an InlineWidget, or + * {string} if pos is in tag but not in tag name, class attr, or id attr, or + * null if we're not going to provide anything. */ function htmlToCSSProvider(hostEditor, pos) { diff --git a/src/extensions/default/JavaScriptQuickEdit/main.js b/src/extensions/default/JavaScriptQuickEdit/main.js index 722805c272b..f93bd45e0dd 100644 --- a/src/extensions/default/JavaScriptQuickEdit/main.js +++ b/src/extensions/default/JavaScriptQuickEdit/main.js @@ -107,8 +107,9 @@ define(function (require, exports, module) { * * @param {!Editor} hostEditor * @param {!string} functionName - * @return {$.Promise} a promise that will be resolved with an InlineWidget - * or null if we're not going to provide anything. + * @return {?$.Promise} synchronously resolved with an InlineWidget, or + * {string} if js other than function is detected at pos, or + * null if we're not going to provide anything. */ function _createInlineEditor(hostEditor, functionName) { // Use Tern jump-to-definition helper, if it's available, to find InlineEditor target. From 8d52f02224634d48ece99de18015b9e65626845c Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Tue, 11 Feb 2014 11:30:08 -0800 Subject: [PATCH 08/17] increase message display time to 5 sec --- src/styles/brackets.less | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/styles/brackets.less b/src/styles/brackets.less index e0e09749e19..bc0cdee327e 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -1419,6 +1419,6 @@ label input { // Make the animation use the GPU--especially important for retina. -webkit-transform: translateZ(0); transform: translateZ(0); - transition: 500ms 3s opacity ease-in; + transition: 500ms 5s opacity ease-in; } } From 1b3c3352607674d0e1441437fec97db8378596f3 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Tue, 18 Feb 2014 14:33:47 -0800 Subject: [PATCH 09/17] cleanup TODOs --- src/editor/Editor.js | 5 ----- src/styles/brackets.less | 10 +++++----- src/styles/brackets_colors.less | 2 ++ src/styles/brackets_variables.less | 1 + 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 3f977bcd09c..1294e2dfa02 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1340,11 +1340,6 @@ define(function (require, exports, module) { AnimationUtils.animateUsingClass(this._$messagePopover[0], "animating") .done(_clearMessagePopover); - - // TODO: - // - add shadow to "arrow" - // - clear message on scroll?, change editors?, click in page?, typing? - // - test conflicts with other popovers, menus, dropdowns }; /** diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 32eb51b38bb..95abfc50225 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -1077,7 +1077,7 @@ a, img { top: 24px; min-width: 291px + 2px; // to align with search field above it - background-color: #f74687; + background-color: @bc-error; color: @bc-white; font-size: 12px; padding: 3px 8px; @@ -1400,13 +1400,13 @@ label input { position: absolute; top: 0; left: 0; - z-index: 1000; // TODO: put in @var + z-index: @z-index-brackets-inline-editor-error; visibility: hidden; opacity: 1.0; .text { padding: 5px 10px; - background-color: #f74687; // TODO: put in @var + background-color: @bc-error; border-radius: 3px; color: @bc-white; box-shadow: @tc-dropdown-shadow; @@ -1416,14 +1416,14 @@ label input { height: 0; width: 0; border-width: 0 10px 10px 10px; - border-color: transparent transparent #f74687 transparent; // TODO: put in @var + border-color: transparent transparent @bc-error transparent; border-style: solid; } .arrowBelow { height: 0; width: 0; border-width: 10px 10px 0 10px; - border-color: #f74687 transparent transparent transparent; // TODO: put in @var + border-color: @bc-error transparent transparent transparent; border-style: solid; } &.animating { diff --git a/src/styles/brackets_colors.less b/src/styles/brackets_colors.less index 194c4a7c8f4..f4cc78253e9 100644 --- a/src/styles/brackets_colors.less +++ b/src/styles/brackets_colors.less @@ -65,6 +65,8 @@ @bc-cyan: #2aa198; @bc-green: #859900; +@bc-error: #f74687; + // TopCoat colors and styles, putting them here for now; let me know if there's a better place for these. @tc-icon-down: 0.5; diff --git a/src/styles/brackets_variables.less b/src/styles/brackets_variables.less index 42776246aec..724e83ec38d 100644 --- a/src/styles/brackets_variables.less +++ b/src/styles/brackets_variables.less @@ -58,3 +58,4 @@ @z-index-brackets-context-menu-base: 1000; @z-index-brackets-stylesheet-menu: 1000; +@z-index-brackets-inline-editor-error: 1000; From a3c81c625daca5a48e10752b8847c7f8f0d047fc Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 26 Feb 2014 14:33:33 -0800 Subject: [PATCH 10/17] add transition on open --- src/editor/Editor.js | 9 ++++++--- src/styles/brackets.less | 23 ++++++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 056bfd6f47e..c83004724b3 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1331,7 +1331,7 @@ define(function (require, exports, module) { } // Popover text and arrow are positioned individually - this._$messagePopover.css({"visibility": "visible", "top": top, "left": left}); + this._$messagePopover.css({"top": top, "left": left}); // Position popover arrow exactly centered over/under cursor arrowLeft = cursorCoord.left - left - POPOVER_ARROW_HALF_WIDTH; @@ -1341,8 +1341,11 @@ define(function (require, exports, module) { this._$messagePopover.find(".arrowAbove").css({"margin-left": arrowLeft}); } - AnimationUtils.animateUsingClass(this._$messagePopover[0], "animating") - .done(_clearMessagePopover); + AnimationUtils.animateUsingClass(this._$messagePopover[0], "animateOpen").done(function () { + self._$messagePopover.addClass("open"); + AnimationUtils.animateUsingClass(self._$messagePopover[0], "animateClose") + .done(_clearMessagePopover); + }); }; /** diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 62ede1deaec..5a5e3602b70 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -1403,9 +1403,11 @@ label input { top: 0; left: 0; z-index: @z-index-brackets-inline-editor-error; - visibility: hidden; opacity: 1.0; - + + -webkit-transform: scale(0); + transform: scale(0); + .text { padding: 5px 10px; background-color: @bc-error; @@ -1428,11 +1430,22 @@ label input { border-color: @bc-error transparent transparent transparent; border-style: solid; } - &.animating { - opacity: 0.0; + &.animateOpen { + -webkit-transition: -webkit-transform 125ms; + transition: transform 125ms; + -webkit-transform: scale(1); + transform: scale(1); + } + &.animateClose { // Make the animation use the GPU--especially important for retina. -webkit-transform: translateZ(0); transform: translateZ(0); - transition: 500ms 5s opacity ease-in; + -webkit-transition: opacity 500ms 5s ease-in, -webkit-transform 500ms 5s; + transition: opacity 500ms 5s ease-in, transform 500ms 5s; + opacity: 0.0; + } + &.open { + -webkit-transform: scale(1); + transform: scale(1); } } From 09dcbc1adeec1aa830e5518b8b32383181b51d99 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 26 Feb 2014 16:51:25 -0800 Subject: [PATCH 11/17] dismiss message box on scroll, edit, change editors, click in page --- src/editor/Editor.js | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index c83004724b3..351644fed17 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, CodeMirror, window */ +/*global define, $, CodeMirror, window, _clearMessagePopover */ /** * Editor is a 1-to-1 wrapper for a CodeMirror editor instance. It layers on Brackets-specific @@ -1277,14 +1277,31 @@ define(function (require, exports, module) { $editorHolder = $("#editor-holder"), POPOVER_MARGIN = 10, POPOVER_ARROW_HALF_WIDTH = 10; - + + function _removeListeners() { + $(self) + .off("change", _clearMessagePopover) + .off("scroll", _clearMessagePopover) + .off("update", _clearMessagePopover); + self._codeMirror.off("blur", _clearMessagePopover); + } + function _clearMessagePopover() { - if (self._$messagePopover) { + if (self._$messagePopover.length > 0) { self._$messagePopover.remove(); self._$messagePopover = null; } + _removeListeners(); } + function _addListeners() { + $(self) + .on("change", _clearMessagePopover) + .on("scroll", _clearMessagePopover) + .on("update", _clearMessagePopover); + self._codeMirror.on("blur", _clearMessagePopover); + } + // Only 1 message at a time if (this._$messagePopover) { _clearMessagePopover(); @@ -1346,6 +1363,8 @@ define(function (require, exports, module) { AnimationUtils.animateUsingClass(self._$messagePopover[0], "animateClose") .done(_clearMessagePopover); }); + + _addListeners(); }; /** From 4fa14c36d854d0664493c12014342485c1ce28b8 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 26 Feb 2014 17:01:39 -0800 Subject: [PATCH 12/17] fix jshint error --- src/editor/Editor.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 351644fed17..1cbdd4ee78b 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1325,9 +1325,8 @@ define(function (require, exports, module) { } // Estimate where to position popover. - top = (arrowBelow) - ? cursorCoord.top - this._$messagePopover.height() - POPOVER_MARGIN - : cursorCoord.bottom + POPOVER_MARGIN; + top = (arrowBelow) ? cursorCoord.top - this._$messagePopover.height() - POPOVER_MARGIN + : cursorCoord.bottom + POPOVER_MARGIN; left = cursorCoord.left - (this._$messagePopover.width() / 2); popoverRect = { From 1f3b3dbd831f8724c9b115835ac140e83320a9e9 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Wed, 26 Feb 2014 17:49:40 -0800 Subject: [PATCH 13/17] cleanup events; add cursor activity --- src/editor/Editor.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 1cbdd4ee78b..933130a74e4 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, CodeMirror, window, _clearMessagePopover */ +/*global define, $, CodeMirror, window */ /** * Editor is a 1-to-1 wrapper for a CodeMirror editor instance. It layers on Brackets-specific @@ -706,7 +706,7 @@ define(function (require, exports, module) { this._codeMirror.on("blur", function () { self._focused = false; - // EditorManager only cares about other Editors gaining focus, so we don't notify it of anything here + $(self).triggerHandler("blur", [self]); }); this._codeMirror.on("update", function (instance) { @@ -1279,11 +1279,7 @@ define(function (require, exports, module) { POPOVER_ARROW_HALF_WIDTH = 10; function _removeListeners() { - $(self) - .off("change", _clearMessagePopover) - .off("scroll", _clearMessagePopover) - .off("update", _clearMessagePopover); - self._codeMirror.off("blur", _clearMessagePopover); + $(self).off(".msgbox"); } function _clearMessagePopover() { @@ -1296,10 +1292,11 @@ define(function (require, exports, module) { function _addListeners() { $(self) - .on("change", _clearMessagePopover) - .on("scroll", _clearMessagePopover) - .on("update", _clearMessagePopover); - self._codeMirror.on("blur", _clearMessagePopover); + .on("blur.msgbox", _clearMessagePopover) + .on("change.msgbox", _clearMessagePopover) + .on("cursorActivity.msgbox", _clearMessagePopover) + .on("scroll.msgbox", _clearMessagePopover) + .on("update.msgbox", _clearMessagePopover); } // Only 1 message at a time From 986b8ede296d25bddd24dbd2ee6e1437ed7f741e Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Sat, 1 Mar 2014 13:32:15 -0800 Subject: [PATCH 14/17] initial tests for displaying inline editor errors --- .../test1.html | 2 +- test/spec/InlineEditorProviders-test.js | 116 ++++++++++++++++-- 2 files changed, 109 insertions(+), 9 deletions(-) diff --git a/test/spec/InlineEditorProviders-test-files/test1.html b/test/spec/InlineEditorProviders-test-files/test1.html index 3fdad04e694..8468186c4a7 100644 --- a/test/spec/InlineEditorProviders-test-files/test1.html +++ b/test/spec/InlineEditorProviders-test-files/test1.html @@ -14,6 +14,6 @@
-
+
diff --git a/test/spec/InlineEditorProviders-test.js b/test/spec/InlineEditorProviders-test.js index ecb7a2fcff9..92a3abff600 100644 --- a/test/spec/InlineEditorProviders-test.js +++ b/test/spec/InlineEditorProviders-test.js @@ -37,7 +37,8 @@ define(function (require, exports, module) { Dialogs = require("widgets/Dialogs"), KeyEvent = require("utils/KeyEvent"), FileUtils = require("file/FileUtils"), - SpecRunnerUtils = require("spec/SpecRunnerUtils"); + SpecRunnerUtils = require("spec/SpecRunnerUtils"), + Strings = require("strings"); describe("InlineEditorProviders", function () { @@ -78,13 +79,11 @@ define(function (require, exports, module) { * * @param {string} openFile Project relative file path to open in a main editor. * @param {number} openOffset The offset index location within openFile to open an inline editor. - * @param {?boolean} expectInline Use false to verify that an inline editor should not be opened. Omit otherwise. + * @param {boolean=} expectInline Use false to verify that an inline editor should not be opened. Omit otherwise. + * @param {Array<{string}>=} workingSet Optional array of files to open in working set */ function initInlineTest(openFile, openOffset, expectInline, workingSet) { - var allFiles, - editor, - hostOpened = false, - err = false; + var editor; workingSet = workingSet || []; @@ -145,6 +144,14 @@ define(function (require, exports, module) { return false; } + function expectPopoverMessageWithText(text) { + var $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(1); + + var popoverText = $(".text", $popover).html(); + expect(popoverText).toEqual(text); + } + /* * Note that the bulk of selector matching tests are in CSSutils-test.js. * These tests are primarily focused on the InlineEditorProvider module. @@ -405,8 +412,33 @@ define(function (require, exports, module) { initInlineTest("test1.html", 3, false); runs(function () { - // verify no inline open + // verify no inline editor open + expect(EditorManager.getCurrentFullEditor().getInlineWidgets().length).toBe(0); + + // verify popover message is displayed with correct string + expectPopoverMessageWithText(Strings.ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND); + }); + }); + + it("should not open an inline editor when positioned on title attribute", function () { + initInlineTest("test1.html", 12, false); + + runs(function () { + // verify no inline editor open expect(EditorManager.getCurrentFullEditor().getInlineWidgets().length).toBe(0); + + // verify popover message is displayed with correct string + expectPopoverMessageWithText(Strings.ERROR_CSSQUICKEDIT_UNSUPPORTEDATTR); + + // verify popover message is automatically dismissed after short wait + // current delay is 5 sec + 0.5 sec fade-out transition + waits(6000); + }); + + runs(function () { + // verify no popover message + var $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(0); }); }); @@ -414,11 +446,79 @@ define(function (require, exports, module) { initInlineTest("test1.html", 4, false); runs(function () { - // verify no inline open + // verify no inline editor open expect(EditorManager.getCurrentFullEditor().getInlineWidgets().length).toBe(0); }); }); + it("should close first popover message before opening another one", function () { + var editor, + openFile = "test1.html"; + + runs(function () { + waitsForDone(SpecRunnerUtils.openProjectFiles([openFile]), "FILE_OPEN timeout", 1000); + }); + + runs(function () { + editor = EditorManager.getCurrentFullEditor(); + + // attempt to open inline editor at specified offset index + var inlineEditorResult = SpecRunnerUtils.toggleQuickEditAtOffset( + editor, + infos[openFile].offsets[3] + ); + + waitsForFail(inlineEditorResult, "inline editor not opened", 1000); + }); + + runs(function () { + // attempt to open another inline editor at a different offset index + var inlineEditorResult = SpecRunnerUtils.toggleQuickEditAtOffset( + editor, + infos[openFile].offsets[12] + ); + + waitsForFail(inlineEditorResult, "inline editor not opened", 1000); + }); + + runs(function () { + // verify only 1 popover message + var $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(1); + }); + }); + + it("should close popover message on selection change", function () { + var editor, + openFile = "test1.html"; + + runs(function () { + waitsForDone(SpecRunnerUtils.openProjectFiles([openFile]), "FILE_OPEN timeout", 1000); + }); + + runs(function () { + editor = EditorManager.getCurrentFullEditor(); + + // attempt to open inline editor at specified offset index + var inlineEditorResult = SpecRunnerUtils.toggleQuickEditAtOffset( + editor, + infos[openFile].offsets[3] + ); + + waitsForFail(inlineEditorResult, "inline editor not opened", 1000); + }); + + runs(function () { + // change selection + var offset = infos[openFile].offsets[12]; + editor.setCursorPos(offset.line, offset.ch); + + // verify no popover message + var $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(0); + }); + }); + it("should increase size based on content", function () { initInlineTest("test1.html", 1); From e442e4b243687f2b20bfe819c69562c9d14443bf Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Sat, 1 Mar 2014 14:57:20 -0800 Subject: [PATCH 15/17] scroll cursor into view --- src/editor/Editor.js | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 933130a74e4..93a4263b58a 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1283,7 +1283,7 @@ define(function (require, exports, module) { } function _clearMessagePopover() { - if (self._$messagePopover.length > 0) { + if (self._$messagePopover && self._$messagePopover.length > 0) { self._$messagePopover.remove(); self._$messagePopover = null; } @@ -1295,7 +1295,6 @@ define(function (require, exports, module) { .on("blur.msgbox", _clearMessagePopover) .on("change.msgbox", _clearMessagePopover) .on("cursorActivity.msgbox", _clearMessagePopover) - .on("scroll.msgbox", _clearMessagePopover) .on("update.msgbox", _clearMessagePopover); } @@ -1304,8 +1303,11 @@ define(function (require, exports, module) { _clearMessagePopover(); } + // Make sure cursor is in view + cursorPos = this.getCursorPos(); + this._codeMirror.scrollIntoView(cursorPos); + // Determine if arrow is above or below - cursorPos = this.getCursorPos(); cursorCoord = this._codeMirror.charCoords(cursorPos); // Assume popover height is max of 2 lines @@ -1353,14 +1355,25 @@ define(function (require, exports, module) { } else { this._$messagePopover.find(".arrowAbove").css({"margin-left": arrowLeft}); } - - AnimationUtils.animateUsingClass(this._$messagePopover[0], "animateOpen").done(function () { - self._$messagePopover.addClass("open"); - AnimationUtils.animateUsingClass(self._$messagePopover[0], "animateClose") - .done(_clearMessagePopover); - }); + // Add listeners _addListeners(); + + // Animate open + AnimationUtils.animateUsingClass(this._$messagePopover[0], "animateOpen").done(function () { + // Make sure we still have a popover + if (self._$messagePopover && self._$messagePopover.length > 0) { + self._$messagePopover.addClass("open"); + + // Don't add scroll listeners until open so we don't get event + // from scrolling cursor into view + $(self).on("scroll.msgbox", _clearMessagePopover); + + // Animate closed -- which includes delay to show message + AnimationUtils.animateUsingClass(self._$messagePopover[0], "animateClose") + .done(_clearMessagePopover); + } + }); }; /** From 8a5191453e9e0408c34e828af1142ad8d7ebd690 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Sat, 1 Mar 2014 16:10:16 -0800 Subject: [PATCH 16/17] test popover message positioning --- .../test1.html | 1 + test/spec/InlineEditorProviders-test.js | 106 +++++++++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/test/spec/InlineEditorProviders-test-files/test1.html b/test/spec/InlineEditorProviders-test-files/test1.html index 8468186c4a7..3a6d3370c24 100644 --- a/test/spec/InlineEditorProviders-test-files/test1.html +++ b/test/spec/InlineEditorProviders-test-files/test1.html @@ -15,5 +15,6 @@
+
This is a long line of text so that cursor position at end of line will have to be scrolled into view
{{13}} diff --git a/test/spec/InlineEditorProviders-test.js b/test/spec/InlineEditorProviders-test.js index 92a3abff600..b7e5aed434a 100644 --- a/test/spec/InlineEditorProviders-test.js +++ b/test/spec/InlineEditorProviders-test.js @@ -28,7 +28,8 @@ define(function (require, exports, module) { 'use strict'; - var Commands, // loaded from brackets.test + var CommandManager, // loaded from brackets.test + Commands, // loaded from brackets.test EditorManager, // loaded from brackets.test FileSyncManager, // loaded from brackets.test DocumentManager, // loaded from brackets.test @@ -152,6 +153,37 @@ define(function (require, exports, module) { expect(popoverText).toEqual(text); } + function getBounds(object, useOffset) { + var left = (useOffset ? object.offset().left : parseInt(object.css("left"), 10)), + top = (useOffset ? object.offset().top : parseInt(object.css("top"), 10)); + return { + left: left, + top: top, + right: left + object.outerWidth(), + bottom: top + object.outerHeight() + }; + } + + function boundsInsideWindow(object) { + // For the popover, we can't use offset(), because jQuery gets confused by the + // scale factor and transform origin that the animation uses. Instead, we rely on + // the fact that its offset parent is body, and just test its explicit left/top values. + var bounds = getBounds(object, false), + editorBounds = getBounds(testWindow.$("#editor-holder"), true); + + return bounds.left >= editorBounds.left && + bounds.right <= editorBounds.right && + bounds.top >= editorBounds.top && + bounds.bottom <= editorBounds.bottom; + } + + function toggleOption(commandID, text) { + runs(function () { + var promise = CommandManager.execute(commandID); + waitsForDone(promise, text); + }); + } + /* * Note that the bulk of selector matching tests are in CSSutils-test.js. * These tests are primarily focused on the InlineEditorProvider module. @@ -171,6 +203,7 @@ define(function (require, exports, module) { testWindow = w; // Load module instances from brackets.test + CommandManager = testWindow.brackets.test.CommandManager; Commands = testWindow.brackets.test.Commands; EditorManager = testWindow.brackets.test.EditorManager; FileSyncManager = testWindow.brackets.test.FileSyncManager; @@ -519,6 +552,77 @@ define(function (require, exports, module) { }); }); + it("should position message popover inside left edge of window", function () { + var $popover; + initInlineTest("test1.html", 11, false); + + runs(function () { + $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(1); + }); + + runs(function () { + expect(boundsInsideWindow($popover)).toBeTruthy(); + }); + }); + + it("should position message popover inside top edge of window", function () { + var $popover; + initInlineTest("test1.html", 3, false); + + runs(function () { + $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(1); + }); + + runs(function () { + expect(boundsInsideWindow($popover)).toBeTruthy(); + }); + }); + + it("should scroll cursor into view and position message popover inside right edge of window", function () { + var $popover, scrollPos, editor, + openFile = "test1.html"; + + runs(function () { + // Turn off word wrap for next tests + toggleOption(Commands.TOGGLE_WORD_WRAP, "Toggle word-wrap"); + }); + + runs(function () { + waitsForDone(SpecRunnerUtils.openProjectFiles([openFile]), "FILE_OPEN timeout", 1000); + }); + + runs(function () { + editor = EditorManager.getCurrentFullEditor(); + expect(editor.getScrollPos().x).toEqual(0); + + // attempt to open inline editor at specified offset index + var inlineEditorResult = SpecRunnerUtils.toggleQuickEditAtOffset( + editor, + infos[openFile].offsets[13] + ); + + waitsForFail(inlineEditorResult, "inline editor not opened", 1000); + }); + + runs(function () { + $popover = testWindow.$(".popover-message"); + expect($popover.length).toEqual(1); + }); + + runs(function () { + // Popover should be inside right edge + expect(boundsInsideWindow($popover)).toBeTruthy(); + + // verify that page scrolled left + expect(editor.getScrollPos().x).toBeGreaterThan(0); + + // restore word wrap + toggleOption(Commands.TOGGLE_WORD_WRAP, "Toggle word-wrap"); + }); + }); + it("should increase size based on content", function () { initInlineTest("test1.html", 1); From 85952461346d78757ba2f1f7dca8ec5eec06b268 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Tue, 11 Mar 2014 09:10:28 -0700 Subject: [PATCH 17/17] add popover message to PopUpManager --- src/editor/Editor.js | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 93a4263b58a..d620aa58153 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -66,6 +66,7 @@ define(function (require, exports, module) { var Menus = require("command/Menus"), PerfUtils = require("utils/PerfUtils"), + PopUpManager = require("widgets/PopUpManager"), PreferencesManager = require("preferences/PreferencesManager"), Strings = require("strings"), TextRange = require("document/TextRange").TextRange, @@ -1282,25 +1283,32 @@ define(function (require, exports, module) { $(self).off(".msgbox"); } + // PopUpManager.removePopUp() callback function _clearMessagePopover() { if (self._$messagePopover && self._$messagePopover.length > 0) { - self._$messagePopover.remove(); + // self._$messagePopover.remove() is done by PopUpManager self._$messagePopover = null; } _removeListeners(); } + // PopUpManager.removePopUp() is called either directly by this closure, or by + // PopUpManager as a result of another popup being invoked. + function _removeMessagePopover() { + PopUpManager.removePopUp(self._$messagePopover); + } + function _addListeners() { $(self) - .on("blur.msgbox", _clearMessagePopover) - .on("change.msgbox", _clearMessagePopover) - .on("cursorActivity.msgbox", _clearMessagePopover) - .on("update.msgbox", _clearMessagePopover); + .on("blur.msgbox", _removeMessagePopover) + .on("change.msgbox", _removeMessagePopover) + .on("cursorActivity.msgbox", _removeMessagePopover) + .on("update.msgbox", _removeMessagePopover); } // Only 1 message at a time if (this._$messagePopover) { - _clearMessagePopover(); + _removeMessagePopover(); } // Make sure cursor is in view @@ -1357,6 +1365,7 @@ define(function (require, exports, module) { } // Add listeners + PopUpManager.addPopUp(this._$messagePopover, _clearMessagePopover, true); _addListeners(); // Animate open @@ -1367,11 +1376,11 @@ define(function (require, exports, module) { // Don't add scroll listeners until open so we don't get event // from scrolling cursor into view - $(self).on("scroll.msgbox", _clearMessagePopover); + $(self).on("scroll.msgbox", _removeMessagePopover); // Animate closed -- which includes delay to show message AnimationUtils.animateUsingClass(self._$messagePopover[0], "animateClose") - .done(_clearMessagePopover); + .done(_removeMessagePopover); } }); };