jenkins-bot has submitted this change and it was merged.

Change subject: Limit the height of desktop-context popups
......................................................................


Limit the height of desktop-context popups

Popups can currently grow to be taller than the page, at which point
they can't always be scrolled properly into view. This mostly happens
with the MW title search link inspector.

So, limit the height of the inspectors to the current size of the
viewport.

Bug: T114614
Change-Id: I534ce003d763f7986153d9b03afbe564162ad240
---
M src/ce/ve.ce.Surface.js
M src/ui/contexts/ve.ui.DesktopContext.js
M src/ui/ve.ui.Surface.js
3 files changed, 33 insertions(+), 5 deletions(-)

Approvals:
  Esanders: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 0f227b5..26cbb2c 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -2912,10 +2912,13 @@
        var surface = this,
                documentModel = this.getModel().getDocument(),
                data = documentModel.data,
-               surfaceRect = this.getSurface().getBoundingClientRect(),
+               dimensions = this.surface.getViewportDimensions(),
+               // We want a little padding when finding the range, because 
this is
+               // generally used for things like find/replace, where scrolling 
to see
+               // context is important.
                padding = 50,
-               top = Math.max( this.surface.toolbarHeight - surfaceRect.top - 
padding, 0 ),
-               bottom = top + this.$window.height() - 
this.surface.toolbarHeight + ( padding * 2 ),
+               top = Math.max( 0, dimensions.top - padding ),
+               bottom = dimensions.top + ( padding * 2 ),
                documentRange = new ve.Range( 0, 
this.getModel().getDocument().getInternalList().getListNode().getOuterRange().start
 );
 
        function highestIgnoreChildrenNode( childNode ) {
diff --git a/src/ui/contexts/ve.ui.DesktopContext.js 
b/src/ui/contexts/ve.ui.DesktopContext.js
index e8410af..3514ade 100644
--- a/src/ui/contexts/ve.ui.DesktopContext.js
+++ b/src/ui/contexts/ve.ui.DesktopContext.js
@@ -292,15 +292,24 @@
  * Resize the popup to match the size of its contents (menu or inspector).
  */
 ve.ui.DesktopContext.prototype.setPopupSize = function () {
-       var $container = this.inspector ? this.inspector.$frame : this.$group;
+       var surface = this.surface,
+               viewport = surface.getViewportDimensions(),
+               $container = this.inspector ? this.inspector.$frame : 
this.$group;
 
        // PopupWidget normally is clippable, suppress that to be able to 
resize and scroll it into view.
        // Needs to be repeated before every call, as it resets itself when the 
popup is shown or hidden.
        this.popup.toggleClipping( false );
 
+       // We want to stop the popup from possibly being bigger than the 
viewport,
+       // as that can result in situations where it's impossible to reach parts
+       // of the popup. Limiting it to the window height would ignore toolbars
+       // and the find-replace dialog and suchlike. Therefore we set its max
+       // height to the surface's estimation of the actual viewport available 
to
+       // it. It's okay if the inspector goes off the edge of the viewport, so
+       // long as it's possible to scroll and get it all in view.
        this.popup.setSize(
                $container.outerWidth( true ),
-               $container.outerHeight( true )
+               Math.min( $container.outerHeight( true ), viewport.height )
        );
 
        this.popup.scrollElementIntoView();
diff --git a/src/ui/ve.ui.Surface.js b/src/ui/ve.ui.Surface.js
index 401c715..dafd816 100644
--- a/src/ui/ve.ui.Surface.js
+++ b/src/ui/ve.ui.Surface.js
@@ -246,6 +246,22 @@
 };
 
 /**
+ * Get measurements of the visible area of the surface viewport
+ *
+ * @return {Object} Object with top, bottom, and height properties
+ */
+ve.ui.Surface.prototype.getViewportDimensions = function () {
+       var rect = this.getBoundingClientRect(),
+               top = Math.max( this.toolbarHeight - rect.top, 0 ),
+               bottom = top + $( this.getElementWindow() ).height() - 
this.toolbarHeight;
+       return {
+               top: top,
+               bottom: bottom,
+               height: bottom - top
+       };
+};
+
+/**
  * Check if editing is enabled.
  *
  * @method

-- 
To view, visit https://gerrit.wikimedia.org/r/254195
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I534ce003d763f7986153d9b03afbe564162ad240
Gerrit-PatchSet: 6
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: DLynch <dly...@wikimedia.org>
Gerrit-Reviewer: DLynch <dly...@wikimedia.org>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: TheDJ <hartman.w...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to