Merge pull request #509 from vector-im/bwindels/fix-menupositioning

automatically position popups using a simpler algorithm

src/platform/web/ui/css/layout.css

@@ -118,8 +118,6 @@ the layout viewport up without resizing it when the keyboard shows */
     width: 100%;
     /* otherwise we don't get scrollbars and the content grows as large as it can */
     min-height: 0;
-    /* make popups relative to this element so changing the left panel width doesn't affect their position */
-    position: relative;
 }
 
 .middle {

src/platform/web/ui/css/main.css

@@ -16,6 +16,7 @@ limitations under the License.
 */
 @import url('font.css');
 @import url('layout.css');
+@import url('popup.css');
 @import url('login.css');
 @import url('left-panel.css');
 @import url('right-panel.css');

src/platform/web/ui/css/popup.css

@@ -0,0 +1,20 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+.popupContainer {
+    position: absolute;
+    white-space: nowrap;
+}

src/platform/web/ui/css/timeline.css

@@ -17,6 +17,7 @@ limitations under the License.
 .Timeline {
     display: flex;
     flex-direction: column;
+    /* needed to position the jump to bottom button */
     position: relative;
     min-height: 0;
 }

src/platform/web/ui/general/Popup.js

@@ -14,20 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-const HorizontalAxis = {
-    scrollOffset(el) {return el.scrollLeft;},
-    size(el) {return el.offsetWidth;},
-    offsetStart(el) {return el.offsetLeft;},
-    setStart(el, value) {el.style.left = `${value}px`;}, 
-    setEnd(el, value) {el.style.right = `${value}px`;},
-};
-const VerticalAxis = {
-    scrollOffset(el) {return el.scrollTop;},
-    size(el) {return el.offsetHeight;},
-    offsetStart(el) {return el.offsetTop;},
-    setStart(el, value) {el.style.top = `${value}px`;}, 
-    setEnd(el, value) {el.style.bottom = `${value}px`;},
-};
+import {tag} from "./html";
 
 export class Popup {
     constructor(view, closeCallback = null) {
@@ -40,27 +27,28 @@ export class Popup {
         this._closeCallback = closeCallback;
     }
 
+    _getPopupContainer() {
+        const appContainer = this._target.closest(".hydrogen");
+        let popupContainer = appContainer.querySelector(".popupContainer");
+        if (!popupContainer) {
+            popupContainer = tag.div({className: "popupContainer"});
+            appContainer.appendChild(popupContainer);
+        }
+        return popupContainer;
+    }
+
     trackInTemplateView(templateView) {
         this._trackingTemplateView = templateView;
         this._trackingTemplateView.addSubView(this);
     }
 
-    /**
-    @param {DOMElement}
-    @param {string} arrangement.relativeTo: whether top/left or bottom/right is used to position
-    @param {string} arrangement.align: how much of the popup axis size (start: 0, end: width or center: width/2)
-           is taken into account when positioning relative to the target
-    @param {number} arrangement.before extra padding to shift the final positioning with
-    @param {number} arrangement.after extra padding to shift the final positioning with
-    */
-    showRelativeTo(target, arrangement) {
+    showRelativeTo(target, verticalPadding = 0) {
         this._target = target;
-        this._arrangement = arrangement;
+        this._verticalPadding = verticalPadding;
         this._scroller = findScrollParent(this._target);
         this._view.mount();
-        this._target.offsetParent.appendChild(this._popup);
-        this._applyArrangementAxis(HorizontalAxis, this._arrangement.horizontal);
-        this._applyArrangementAxis(VerticalAxis, this._arrangement.vertical);
+        this._getPopupContainer().appendChild(this._popup);
+        this._position();
         if (this._scroller) {
             document.body.addEventListener("scroll", this, true);
         }
@@ -95,75 +83,48 @@ export class Popup {
 
     handleEvent(evt) {
         if (evt.type === "scroll") {
-            this._onScroll();
+            if(!this._position()) {
+                this.close();
+            }
         } else if (evt.type === "click") {
             this._onClick(evt);
         }
     }
 
-    _onScroll() {
-        if (this._scroller && !this._isVisibleInScrollParent(VerticalAxis)) {
-            this.close();
-        } else {
-            this._applyArrangementAxis(HorizontalAxis, this._arrangement.horizontal);
-            this._applyArrangementAxis(VerticalAxis, this._arrangement.vertical);
-        }
-    }
-
     _onClick() {
         this.close();
     }
 
-    _applyArrangementAxis(axis, {relativeTo, align, before, after}) {
-        // TODO: using {relativeTo: "end", align: "start"} to align the right edge of the popup
-        // with the right side of the target doens't make sense here, we'd expect align: "right"?
-        // see RoomView
-        if (relativeTo === "end") {
-            let end = axis.size(this._target.offsetParent) - axis.offsetStart(this._target);
-            if (align === "end") {
-                end -= axis.size(this._popup);
-            } else if (align === "center") {
-                end -= ((axis.size(this._popup) / 2) - (axis.size(this._target) / 2));
-            }
-            if (typeof before === "number") {
-                end += before;
-            } else if (typeof after === "number") {
-                end -= (axis.size(this._target) + after);
-            }
-            axis.setEnd(this._popup, end);
-        } else if (relativeTo === "start") {
-            let scrollOffset = this._scroller ? axis.scrollOffset(this._scroller) : 0;
-            let start = axis.offsetStart(this._target) - scrollOffset;
-            if (align === "start") {
-                start -= axis.size(this._popup);
-            } else if (align === "center") {
-                start -= ((axis.size(this._popup) / 2) - (axis.size(this._target) / 2));
-            }
-            if (typeof before === "number") {
-                start -= before;
-            } else if (typeof after === "number") {
-                start += (axis.size(this._target) + after);
-            }
-            axis.setStart(this._popup, start);
-        } else {
-            throw new Error("unknown relativeTo: " + relativeTo);
-        }
-    }
+    _position() {
+        const targetPosition = this._target.getBoundingClientRect();
+        const popupWidth = this._popup.clientWidth;
+        const popupHeight = this._popup.clientHeight;
+        const viewport = (this._scroller ? this._scroller : document.documentElement).getBoundingClientRect();
 
-    _isVisibleInScrollParent(axis) {
-        // clipped at start?
-        if ((axis.offsetStart(this._target) + axis.size(this._target)) < (
-            axis.offsetStart(this._scroller) + 
-            axis.scrollOffset(this._scroller)
-        )) {
+        if (
+            targetPosition.top > viewport.bottom ||
+            targetPosition.left > viewport.right ||
+            targetPosition.bottom < viewport.top ||
+            targetPosition.right < viewport.left
+        ) {
             return false;
         }
-        // clipped at end?
-        if (axis.offsetStart(this._target) > (
-            axis.offsetStart(this._scroller) + 
-            axis.size(this._scroller) + 
-            axis.scrollOffset(this._scroller)
-        )) {
+        if (viewport.bottom >= targetPosition.bottom + popupHeight) {
+            // show below
+            this._popup.style.top = `${targetPosition.bottom + this._verticalPadding}px`;
+        } else if (viewport.top <= targetPosition.top - popupHeight) {
+            // show top
+            this._popup.style.top = `${targetPosition.top - popupHeight - this._verticalPadding}px`;
+        } else {
+            return false;
+        }
+        if (viewport.right >= targetPosition.right + popupWidth) {
+            // show right
+            this._popup.style.left = `${targetPosition.left}px`;
+        } else if (viewport.left <= targetPosition.left - popupWidth) {
+            // show left
+            this._popup.style.left = `${targetPosition.right - popupWidth}px`;
+        } else {
             return false;
         }
         return true;
@@ -196,10 +157,10 @@ function findScrollParent(el) {
             // can cause the scrollHeight to be larger than the clientHeight in the parent
             // see button.link class
             const style = window.getComputedStyle(parent);
-            const {overflow} = style;
-            if (overflow === "auto" || overflow === "scroll") {
+            const overflowY = style.getPropertyValue("overflow-y");
+            if (overflowY === "auto" || overflowY === "scroll") {
                 return parent;
             }
         }
-    } while (parent !== el.offsetParent);
+    } while (parent !== document.body);
 }

src/platform/web/ui/session/room/MessageComposer.js

@@ -102,18 +102,7 @@ export class MessageComposer extends TemplateView {
                 Menu.option(vm.i18n`Send file`, () => vm.sendFile()).setIcon("file"),
             ]));
             this._attachmentPopup.trackInTemplateView(this);
-            this._attachmentPopup.showRelativeTo(evt.target, {
-                horizontal: {
-                    relativeTo: "end",
-                    align: "start",
-                    after: 0
-                },
-                vertical: {
-                    relativeTo: "end",
-                    align: "start",
-                    before: 8,
-                }
-            });
+            this._attachmentPopup.showRelativeTo(evt.target, 12);
         }
     }
 }

src/platform/web/ui/session/room/RoomView.js

@@ -83,18 +83,7 @@ export class RoomView extends TemplateView {
             }
             this._optionsPopup = new Popup(new Menu(options));
             this._optionsPopup.trackInTemplateView(this);
-            this._optionsPopup.showRelativeTo(evt.target, {
-                horizontal: {
-                    relativeTo: "end",
-                    align: "start",
-                    after: 0
-                },
-                vertical: {
-                    relativeTo: "start",
-                    align: "end",
-                    before: -32 - 4
-                }
-            });
+            this._optionsPopup.showRelativeTo(evt.target, 10);
         }
     }
 

src/platform/web/ui/session/room/timeline/BaseMessageView.js

@@ -98,18 +98,7 @@ export class BaseMessageView extends TemplateView {
             const onClose = () => this.root().classList.remove("menuOpen");
             this._menuPopup = new Popup(new Menu(options), onClose);
             this._menuPopup.trackInTemplateView(this);
-            this._menuPopup.showRelativeTo(button, {
-                horizontal: {
-                    relativeTo: "end",
-                    align: "start",
-                    after: 0
-                },
-                vertical: {
-                    relativeTo: "start",
-                    align: "end",
-                    before: -24
-                }
-            });
+            this._menuPopup.showRelativeTo(button, 2);
         }
     }