From 71bd797dd4367f11c2cfabd13cfe1ceb873becaf Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 24 Sep 2021 18:28:06 +0200 Subject: [PATCH] automatically position popups using a simpler algorithm --- src/platform/web/ui/css/layout.css | 2 - src/platform/web/ui/css/main.css | 1 + src/platform/web/ui/css/popup.css | 20 +++ src/platform/web/ui/css/timeline.css | 1 + src/platform/web/ui/general/Popup.js | 135 +++++++----------- .../web/ui/session/room/MessageComposer.js | 13 +- src/platform/web/ui/session/room/RoomView.js | 13 +- .../session/room/timeline/BaseMessageView.js | 13 +- 8 files changed, 73 insertions(+), 125 deletions(-) create mode 100644 src/platform/web/ui/css/popup.css diff --git a/src/platform/web/ui/css/layout.css b/src/platform/web/ui/css/layout.css index 3fb23a27..a85ab109 100644 --- a/src/platform/web/ui/css/layout.css +++ b/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 { diff --git a/src/platform/web/ui/css/main.css b/src/platform/web/ui/css/main.css index 82b849c8..94752e4e 100644 --- a/src/platform/web/ui/css/main.css +++ b/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'); diff --git a/src/platform/web/ui/css/popup.css b/src/platform/web/ui/css/popup.css new file mode 100644 index 00000000..e71e47ab --- /dev/null +++ b/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; +} diff --git a/src/platform/web/ui/css/timeline.css b/src/platform/web/ui/css/timeline.css index f214c98f..10999ddf 100644 --- a/src/platform/web/ui/css/timeline.css +++ b/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; } diff --git a/src/platform/web/ui/general/Popup.js b/src/platform/web/ui/general/Popup.js index e630d398..c72f99f7 100644 --- a/src/platform/web/ui/general/Popup.js +++ b/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); } diff --git a/src/platform/web/ui/session/room/MessageComposer.js b/src/platform/web/ui/session/room/MessageComposer.js index b82e0ffd..0e38216f 100644 --- a/src/platform/web/ui/session/room/MessageComposer.js +++ b/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); } } } diff --git a/src/platform/web/ui/session/room/RoomView.js b/src/platform/web/ui/session/room/RoomView.js index f100d796..c172766a 100644 --- a/src/platform/web/ui/session/room/RoomView.js +++ b/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); } } diff --git a/src/platform/web/ui/session/room/timeline/BaseMessageView.js b/src/platform/web/ui/session/room/timeline/BaseMessageView.js index 424587a8..1fa14841 100644 --- a/src/platform/web/ui/session/room/timeline/BaseMessageView.js +++ b/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); } }