Merge pull request #3544 from adobe/randy/issue-3447

Position Hover Preview popover so it's not clipped
adobe · Apr 23, 2013 · 3cf285e · 3cf285e
2 parents 3292c87 + 3553640
commit 3cf285e
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 18 deletions.
diff --git a/src/extensions/default/HoverPreview/main.js b/src/extensions/default/HoverPreview/main.js
@@ -51,7 +51,8 @@ define(function (require, exports, module) {
         POSITION_OFFSET             = 38,   // Distance between the bottom of the line and the bottom of the preview container
         POINTER_LEFT_OFFSET         = 17,   // Half of the pointer width, used to find the center of the pointer
         POINTER_TOP_OFFSET          =  7,   // Pointer height, used to shift popover above pointer
-        POSITION_BELOW_OFFSET       = 16;   // Amount to adjust to top position when the preview bubble is below the text
+        POSITION_BELOW_OFFSET       = 16,   // Amount to adjust to top position when the preview bubble is below the text
+        POPOVER_HORZ_MARGIN         =  5;   // Horizontal margin
 
     /**
      * There are three states for this var:
@@ -103,22 +104,29 @@ define(function (require, exports, module) {
         popoverState = null;
     }
 
-    function positionPreview(xpos, ytop, ybot) {
-        var top = ytop - $previewContainer.height() - POSITION_OFFSET;
+    function positionPreview(xpos, ypos, ybot) {
+        var previewWidth  = $previewContainer.width(),
+            top           = ypos - $previewContainer.height() - POSITION_OFFSET,
+            left          = xpos - previewWidth / 2 - POINTER_LEFT_OFFSET,
+            $editorHolder = $("#editor-holder"),
+            editorLeft    = $editorHolder.offset().left;
+
+        left = Math.max(left, editorLeft + POPOVER_HORZ_MARGIN);
+        left = Math.min(left, editorLeft + $editorHolder.width() - previewWidth - POPOVER_HORZ_MARGIN);
 
         if (top < 0) {
             $previewContainer.removeClass("preview-bubble-above");
             $previewContainer.addClass("preview-bubble-below");
             top = ybot + POSITION_BELOW_OFFSET;
             $previewContainer.offset({
-                left: xpos - $previewContainer.width() / 2 - POINTER_LEFT_OFFSET,
+                left: left,
                 top: top
             });
         } else {
             $previewContainer.removeClass("preview-bubble-below");
             $previewContainer.addClass("preview-bubble-above");
             $previewContainer.offset({
-                left: xpos - $previewContainer.width() / 2 - POINTER_LEFT_OFFSET,
+                left: left,
                 top: top - POINTER_TOP_OFFSET
             });
         }

diff --git a/src/extensions/default/HoverPreview/unittests.js b/src/extensions/default/HoverPreview/unittests.js
@@ -280,11 +280,12 @@ define(function (require, exports, module) {
             }
 
             function boundsInsideWindow(object) {
-                var bounds = getBounds(object);
-                return bounds.left   >= 0                      &&
-                       bounds.right  <= $(testWindow).width()  &&
-                       bounds.top    >= 0                      &&
-                       bounds.bottom <= $(testWindow).height();
+                var bounds = getBounds(object),
+                    editorBounds = getBounds(testWindow.$("#editor-holder"));
+                return bounds.left   >= editorBounds.left   &&
+                       bounds.right  <= editorBounds.right  &&
+                       bounds.top    >= editorBounds.top    &&
+                       bounds.bottom <= editorBounds.bottom;
             }
 
             function toggleOption(commandID, text) {
@@ -314,23 +315,17 @@ define(function (require, exports, module) {
                 });
 
                 runs(function () {
-
-// Issue #3447 - fixes both of the following tests
-/*
                     // Popover should be inside right edge
                     showPopoverAtPos(81, 36);
                     expect(boundsInsideWindow($popover)).toBeTruthy();
-*/
 
-/*
                     // Popover should be inside left edge
-                    var scrollX = editor._codeMirror.defaultCharWidth()  * 120,
-                        scrollY = editor._codeMirror.defaultTextHeight() * 190;
+                    var scrollX = editor._codeMirror.defaultCharWidth()  * 80,
+                        scrollY = editor._codeMirror.defaultTextHeight() * 70;
 
                     editor.setScrollPos(scrollX, scrollY);      // Scroll right
                     showPopoverAtPos(82, 136);
                     expect(boundsInsideWindow($popover)).toBeTruthy();
-*/
 
                     // restore word wrap
                     toggleOption(Commands.TOGGLE_WORD_WRAP, "Toggle word-wrap");