Merge pull request #6916 from adobe/randy/display-inline-editor-errors

Display Inline Editor Errors

Author: JeffryBooher

src/editor/CSSInlineEditor.js

@@ -54,11 +54,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
@@ -85,16 +88,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
+        };
     }
 
     /**
@@ -146,8 +160,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) {
 
@@ -164,10 +179,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,

src/editor/Editor.js

@@ -69,6 +69,7 @@ define(function (require, exports, module) {
         CodeMirror         = require("thirdparty/CodeMirror2/lib/codemirror"),
         Menus              = require("command/Menus"),
         PerfUtils          = require("utils/PerfUtils"),
+        PopUpManager       = require("widgets/PopUpManager"),
         PreferencesManager = require("preferences/PreferencesManager"),
         Strings            = require("strings"),
         TextRange          = require("document/TextRange").TextRange,
@@ -198,6 +199,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 = {
@@ -811,7 +813,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) {
@@ -1518,6 +1520,126 @@ 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 arrowBelow, cursorPos, cursorCoord, popoverRect,
+            top, left, clip, arrowLeft,
+            self = this,
+            $editorHolder = $("#editor-holder"),
+            POPOVER_MARGIN = 10,
+            POPOVER_ARROW_HALF_WIDTH = 10;
+
+        function _removeListeners() {
+            $(self).off(".msgbox");
+        }
+
+        // PopUpManager.removePopUp() callback
+        function _clearMessagePopover() {
+            if (self._$messagePopover && self._$messagePopover.length > 0) {
+                // 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",           _removeMessagePopover)
+                .on("change.msgbox",         _removeMessagePopover)
+                .on("cursorActivity.msgbox", _removeMessagePopover)
+                .on("update.msgbox",         _removeMessagePopover);
+        }
+
+        // Only 1 message at a time
+        if (this._$messagePopover) {
+            _removeMessagePopover();
+        }
+        
+        // Make sure cursor is in view
+        cursorPos = this.getCursorPos();
+        this._codeMirror.scrollIntoView(cursorPos);
+        
+        // Determine if arrow is above or below
+        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
+        this._$messagePopover = $("<div/>").addClass("popover-message").appendTo($("body"));
+        if (!arrowBelow) {
+            $("<div/>").addClass("arrowAbove").appendTo(this._$messagePopover);
+        }
+        $("<div/>").addClass("text").appendTo(this._$messagePopover).html(errorMsg);
+        if (arrowBelow) {
+            $("<div/>").addClass("arrowBelow").appendTo(this._$messagePopover);
+        }
+        
+        // Estimate where to position popover.
+        top = (arrowBelow) ? cursorCoord.top - this._$messagePopover.height() - POPOVER_MARGIN
+                           : cursorCoord.bottom + POPOVER_MARGIN;
+        left = cursorCoord.left - (this._$messagePopover.width() / 2);
+        
+        popoverRect = {
+            top:    top,
+            left:   left,
+            height: this._$messagePopover.height(),
+            width:  this._$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
+        this._$messagePopover.css({"top": top, "left": left});
+        
+        // Position popover arrow exactly centered over/under cursor
+        arrowLeft = cursorCoord.left - left - POPOVER_ARROW_HALF_WIDTH;
+        if (arrowBelow) {
+            this._$messagePopover.find(".arrowBelow").css({"margin-left": arrowLeft});
+        } else {
+            this._$messagePopover.find(".arrowAbove").css({"margin-left": arrowLeft});
+        }
+
+        // Add listeners
+        PopUpManager.addPopUp(this._$messagePopover, _clearMessagePopover, true);
+        _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", _removeMessagePopover);
+
+                // Animate closed -- which includes delay to show message
+                AnimationUtils.animateUsingClass(self._$messagePopover[0], "animateClose")
+                    .done(_removeMessagePopover);
+            }
+        });
+    };
+    
     /**
      * 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

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();
         }
 
@@ -918,12 +944,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) {
@@ -939,7 +967,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();
@@ -1008,10 +1036,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);
 

src/extensions/default/InlineTimingFunctionEditor/main.js

@@ -65,28 +65,28 @@ 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,
             cm = hostEditor._codeMirror;
 
         sel = hostEditor.getSelection();
         if (sel.start.line !== sel.end.line) {
-            return null;
+            return {timingFunction: null, reason: null};
         }
 
         cursorLine = hostEditor.document.getLine(pos.line);
 
         // 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,16 +129,17 @@ 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.
+     * @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),
             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);
@@ -162,8 +163,6 @@ define(function (require, exports, module) {
 
     init();
 
-    // for use by other InlineColorEditors
-    exports.prepareEditorForProvider = prepareEditorForProvider;
 
     // for unit tests only
     exports.inlineTimingFunctionEditorProvider = inlineTimingFunctionEditorProvider;