feat(popover): do not remove the popover on every update

With popover usage now only passing in React Components, the
logic of removing the popover and recreating its html with
every update is not necessary. Instead allow React to update
the popover contents.

Because of this change, mouse event handlers are not recreated
on each update, so it is possible for mouseleave to fire after
the size of the popover shrinks when collapsing to hide more stats,
forcing the mouse out of the popover. To prevent this, padding has
been added to the top of the popover so on resize the mouse will
still be over the popover. The padding has the added bonus of
fixing an issue where the popover would not close until mouseenter
was triggered after size collapse, but it adds the drawback of
requiring more upward mouse travel to close the popover.

css/_jitsi_popover.scss

@@ -16,11 +16,31 @@
     white-space: normal;
     margin-top: -$popoverMenuPadding;
 
-    &__menu-padding {
+
-        height: $popoverMenuPadding;
+    &__menu-padding,
-        width: 100px;
+    &__menu-padding-top {
         position: absolute;
+        width: 100px;
+    }
+
+    /**
+     * Invisible padding is added to the bottom of the popover to extend its
+     * height so it does not close when moving the mouse from the trigger
+     * element towards the popover itself.
+     */
+    &__menu-padding {
         bottom: -$popoverMenuPadding;
+        height: $popoverMenuPadding;
+    }
+
+    /**
+     * Invisible padding is added to the top of the popover to extend its height
+     * so it does not close automatically when its height is shrunk from showing
+     * less video statistics.
+     */
+    &__menu-padding-top {
+        height: 20px;
+        top: -20px;
     }
 
     &__showmore {

modules/UI/util/JitsiPopover.js

@@ -37,16 +37,15 @@ const positionConfigurations = {
 
             $('.jitsipopover').css({
                 display: 'table',
-                left: position.left,
+                left: element.left,
-                top: position.top
+                top: element.top
             });
 
             // Move additional padding to the right edge of the popover and
             // allow css to take care of width. The padding is used to maintain
             // a hover state between the target and the popover.
-            $('.jitsipopover > .jitsipopover__menu-padding').css({
+            $('.jitsipopover > .jitsipopover__menu-padding')
-                left: element.width
+                .css({ left: element.width });
-            });
 
             // Find the distance from the top of the popover to the center of
             // the target and use that value to position the arrow to point to
@@ -67,13 +66,21 @@ const positionConfigurations = {
         at: "top",
         collision: "fit",
         using: function setPositionTop(position, elements) {
-            var calcLeft = elements.target.left - elements.element.left +
+            const { element, target } = elements;
-                elements.target.width/2;
+            const calcLeft = target.left - element.left + target.width / 2;
-            $(".jitsipopover").css(
+            const paddingLeftPosition = calcLeft - 50;
-                {top: position.top, left: position.left, display: "table"});
+            const $jistiPopover = $('.jitsipopover');
-            $(".jitsipopover > .arrow").css({left: calcLeft});
+
-            $(".jitsipopover > .jitsipopover__menu-padding").css(
+            $jistiPopover.css({
-                {left: calcLeft - 50});
+                display: 'table',
+                left: element.left,
+                top: element.top
+            });
+            $jistiPopover.find('.arrow').css({ left: calcLeft });
+            $jistiPopover.find('.jitsipopover__menu-padding')
+                .css({ left: paddingLeftPosition });
+            $jistiPopover.find('.jitsipopover__menu-padding-top')
+                .css({ left: paddingLeftPosition });
         }
     }
 };
@@ -93,8 +100,6 @@ export default (function () {
      * Constructs new JitsiPopover and attaches it to the element
      * @param element jquery selector
      * @param options the options for the popover.
-     *  - {Function} onBeforePosition - function executed just before
-     *      positioning the popover. Useful for translation.
      * @constructor
      */
     function JitsiPopover(element, options)
@@ -132,6 +137,7 @@ export default (function () {
 
         return  (
             `<div class="jitsipopover ${skin} ${position}">
+                <div class="jitsipopover__menu-padding-top"></div>
                 ${arrow}
                 <div class="jitsipopover__content"></div>
                 <div class="jitsipopover__menu-padding"></div>
@@ -176,65 +182,41 @@ export default (function () {
      * Creates the popover html.
      */
     JitsiPopover.prototype.createPopover = function () {
-        $("body").append(this.template);
+        let $popover = $('.jitsipopover');
-        let popoverElem = $(".jitsipopover > .jitsipopover__content");
 
-        const { content } = this.options;
+        if (!$popover.length) {
+            $('body').append(this.template);
 
-        if (React.isValidElement(content)) {
+            $popover = $('.jitsipopover');
-            /* jshint ignore:start */
-            ReactDOM.render(
-                <I18nextProvider i18n = { i18next }>
-                    { content }
-                </I18nextProvider>,
-                popoverElem.get(0),
-                () => {
-                    // FIXME There seems to be odd timing interaction when a
-                    // React Component is manually removed from the DOM and then
-                    // created again, as the ReactDOM callback will fire before
-                    // render is called on the React Component. Using a timeout
-                    // looks to bypass this behavior, maybe by creating
-                    // different execution context. JitsiPopover should be
-                    // rewritten into react soon anyway or at least rewritten
-                    // so the html isn't completely torn down with each update.
-                    setTimeout(() => this._popoverCreated());
-                });
-            /* jshint ignore:end */
-            return;
-        }
 
-        popoverElem.html(content);
+            $popover.on('mouseenter', () => {
-        this._popoverCreated();
-    };
-
-    /**
-     * Adds listeners and executes callbacks after the popover has been created
-     * and displayed.
-     *
-     * @private
-     * @returns {void}
-     */
-    JitsiPopover.prototype._popoverCreated = function () {
-        const { onBeforePosition } = this.options;
-
-        if (typeof onBeforePosition === 'function') {
-            onBeforePosition($(".jitsipopover"));
-        }
-
-        $('.jitsipopover').on('mouseenter', () => {
                 this.popoverIsHovered = true;
                 if (typeof this.onHoverPopover === 'function') {
                     this.onHoverPopover(this.popoverIsHovered);
                 }
-        }).on('mouseleave', () => {
+            });
+
+            $popover.on('mouseleave', () => {
                 this.popoverIsHovered = false;
                 this.hide();
                 if (typeof this.onHoverPopover === 'function') {
                     this.onHoverPopover(this.popoverIsHovered);
                 }
             });
+        }
 
+        const $popoverContent = $popover.find('.jitsipopover__content');
+
+        /* jshint ignore:start */
+        ReactDOM.render(
+            <I18nextProvider i18n = { i18next }>
+                { this.options.content }
+            </I18nextProvider>,
+            $popoverContent.get(0),
+            () => {
                 this.refreshPosition();
+            });
+        /* jshint ignore:end */
     };
 
     /**
@@ -264,9 +246,9 @@ export default (function () {
      */
     JitsiPopover.prototype.updateContent = function (content) {
         this.options.content = content;
-        if(!this.popoverShown)
+        if (!this.popoverShown) {
             return;
-        this.remove();
+        }
         this.createPopover();
     };
 
@@ -279,11 +261,9 @@ export default (function () {
     JitsiPopover.prototype.remove = function () {
         const $popover = $('.jitsipopover');
         const $popoverContent = $popover.find('.jitsipopover__content');
-        const attachedComponent = $popoverContent.get(0);
 
-        if (attachedComponent) {
+        if ($popoverContent.length) {
-            // ReactDOM will no-op if no React Component is found.
+            ReactDOM.unmountComponentAtNode($popoverContent.get(0));
-            ReactDOM.unmountComponentAtNode(attachedComponent);
         }
 
         $popover.off();

modules/UI/videolayout/ConnectionIndicator.js

@@ -1,4 +1,4 @@
-/* global $, APP, interfaceConfig, JitsiMeetJS */
+/* global $, interfaceConfig, JitsiMeetJS */
 /* jshint -W101 */
 
 /* eslint-disable no-unused-vars */
@@ -113,7 +113,6 @@ ConnectionIndicator.prototype.create = function () {
     this.popover = new JitsiPopover($(element), {
         content: popoverContent,
         skin: "black",
-        onBeforePosition: el => APP.translation.translateElement(el),
         position: interfaceConfig.VERTICAL_FILMSTRIP ? 'left' : 'top'
     });
 

modules/UI/videolayout/RemoteVideo.js

@@ -115,7 +115,6 @@ RemoteVideo.prototype._initPopupMenu = function (popupMenuElement) {
         content: popupMenuElement.outerHTML,
         skin: "black",
         hasArrow: false,
-        onBeforePosition: el => APP.translation.translateElement(el),
         position: interfaceConfig.VERTICAL_FILMSTRIP ? 'left' : 'top'
     };
     let element = $("#" + this.videoSpanId + " .remotevideomenu");