Avoid menu layout trashing by moving DOM queries

packages/virtualdom/src/index.ts

@@ -1349,7 +1349,10 @@ namespace Private {
 
       // Handle the simplest case of in-place text update first.
       if (oldVNode.type === 'text' && newVNode.type === 'text') {
-        currElem!.textContent = newVNode.content;
+        // Avoid spurious updates for performance.
+        if (currElem!.textContent !== newVNode.content) {
+          currElem!.textContent = newVNode.content;
+        }
         currElem = currElem!.nextSibling;
         continue;
       }

packages/widgets/src/contextmenu.ts

@@ -78,6 +78,9 @@ export class ContextMenu {
    * position indicated by the event.
    */
   open(event: MouseEvent): boolean {
+    // Prior to any DOM modifications update the window data.
+    Menu.updateWindowData();
+
     // Clear the current contents of the context menu.
     this.menu.clearItems();
 

packages/widgets/src/menu.ts

@@ -32,6 +32,13 @@ import {
 
 import { Widget } from './widget';
 
+interface IWindowData {
+  pageXOffset: number;
+  pageYOffset: number;
+  clientWidth: number;
+  clientHeight: number;
+}
+
 /**
  * A widget which displays items as a canonical menu.
  */
@@ -908,6 +915,17 @@ export class Menu extends Widget {
     }
   }
 
+  /**
+   * Update window data used for menu positioning in window data cache.
+   *
+   * In order to avoid layout trashing it is recommended to invoke this
+   * method immediately prior to opening the menu and any DOM modifications
+   * (like closing previously visible menu, or adding a class to menu widget).
+   */
+  static updateWindowData(): void {
+    Private.updateWindowData();
+  }
+
   private _childIndex = -1;
   private _activeIndex = -1;
   private _openTimerID = 0;
@@ -1372,6 +1390,50 @@ namespace Private {
    */
   export const SUBMENU_OVERLAP = 3;
 
+  /**
+   * How long the window data cache should be kept (ms).
+   */
+  export const WINDOW_DATA_CACHE_DURATION = 1000;
+
+  let latestWindowData: IWindowData | null = null;
+  let latestWindowDataTimestamp: number;
+
+  function getLatestWindowData(): IWindowData | null {
+    const currentTimestamp = Date.now();
+    if (isWindowDataCacheValid(currentTimestamp)) {
+      return latestWindowData;
+    }
+    return null;
+  }
+
+  function isWindowDataCacheValid(timestamp: number) {
+    return timestamp - latestWindowDataTimestamp < WINDOW_DATA_CACHE_DURATION;
+  }
+
+  /**
+   * Update window data cache.
+   *
+   * Note: should be called before any DOM modifications.
+   */
+  export function updateWindowData(): void {
+    const currentTimestamp = Date.now();
+    if (isWindowDataCacheValid(currentTimestamp)) {
+      // If cache is still valid, try again as soon as it expires and layout is be ready,
+      // which drastically reduces the chance of cache timeout occurring just as we start
+      // showing or switching the menu.
+      setTimeout(() => {
+        requestAnimationFrame(() => {
+          latestWindowData = _getWindowData();
+          latestWindowDataTimestamp = currentTimestamp;
+        });
+      }, WINDOW_DATA_CACHE_DURATION - (currentTimestamp - latestWindowDataTimestamp));
+    } else {
+      // Update immediately
+      latestWindowData = _getWindowData();
+      latestWindowDataTimestamp = currentTimestamp;
+    }
+  }
+
   /**
    * Create the DOM node for a menu.
    */
@@ -1471,6 +1533,15 @@ namespace Private {
     return result;
   }
 
+  function _getWindowData(): IWindowData {
+    return {
+      pageXOffset: window.pageXOffset,
+      pageYOffset: window.pageYOffset,
+      clientWidth: document.documentElement.clientWidth,
+      clientHeight: document.documentElement.clientHeight
+    };
+  }
+
   /**
    * Open a menu as a root menu at the target location.
    */
@@ -1485,10 +1556,11 @@ namespace Private {
     MessageLoop.sendMessage(menu, Widget.Msg.UpdateRequest);
 
     // Get the current position and size of the main viewport.
-    let px = window.pageXOffset;
-    let py = window.pageYOffset;
-    let cw = document.documentElement.clientWidth;
-    let ch = document.documentElement.clientHeight;
+    const windowData = getLatestWindowData() || _getWindowData();
+    let px = windowData.pageXOffset;
+    let py = windowData.pageYOffset;
+    let cw = windowData.clientWidth;
+    let ch = windowData.clientHeight;
 
     // Compute the maximum allowed height for the menu.
     let maxHeight = ch - (forceY ? y : 0);
@@ -1498,11 +1570,7 @@ namespace Private {
     let style = node.style;
 
     // Clear the menu geometry and prepare it for measuring.
-    style.top = '';
-    style.left = '';
-    style.width = '';
-    style.height = '';
-    style.visibility = 'hidden';
+    style.opacity = '0';
     style.maxHeight = `${maxHeight}px`;
 
     // Attach the menu to the document.
@@ -1526,11 +1594,10 @@ namespace Private {
     }
 
     // Update the position of the menu to the computed position.
-    style.top = `${Math.max(0, y)}px`;
-    style.left = `${Math.max(0, x)}px`;
+    style.transform = `translate(${Math.max(0, x)}px, ${Math.max(0, y)}px`;
 
     // Finally, make the menu visible on the screen.
-    style.visibility = '';
+    style.opacity = '1';
   }
 
   /**
@@ -1541,10 +1608,11 @@ namespace Private {
     MessageLoop.sendMessage(submenu, Widget.Msg.UpdateRequest);
 
     // Get the current position and size of the main viewport.
-    let px = window.pageXOffset;
-    let py = window.pageYOffset;
-    let cw = document.documentElement.clientWidth;
-    let ch = document.documentElement.clientHeight;
+    const windowData = getLatestWindowData() || _getWindowData();
+    let px = windowData.pageXOffset;
+    let py = windowData.pageYOffset;
+    let cw = windowData.clientWidth;
+    let ch = windowData.clientHeight;
 
     // Compute the maximum allowed height for the menu.
     let maxHeight = ch;
@@ -1554,11 +1622,7 @@ namespace Private {
     let style = node.style;
 
     // Clear the menu geometry and prepare it for measuring.
-    style.top = '';
-    style.left = '';
-    style.width = '';
-    style.height = '';
-    style.visibility = 'hidden';
+    style.opacity = '0';
     style.maxHeight = `${maxHeight}px`;
 
     // Attach the menu to the document.
@@ -1590,11 +1654,10 @@ namespace Private {
     }
 
     // Update the position of the menu to the computed position.
-    style.top = `${Math.max(0, y)}px`;
-    style.left = `${Math.max(0, x)}px`;
+    style.transform = `translate(${Math.max(0, x)}px, ${Math.max(0, y)}px`;
 
     // Finally, make the menu visible on the screen.
-    style.visibility = '';
+    style.opacity = '1';
   }
 
   /**

packages/widgets/src/menubar.ts

@@ -519,8 +519,11 @@ export class MenuBar extends Widget {
       this._closeChildMenu();
       this.activeIndex = index;
     } else {
+      const position = this._positionForMenu(index);
+      Menu.updateWindowData();
+      // Begin DOM modifications.
       this.activeIndex = index;
-      this._openChildMenu();
+      this._openChildMenu(position);
     }
   }
 
@@ -545,15 +548,39 @@ export class MenuBar extends Widget {
       return;
     }
 
+    // Get position for the new menu >before< updating active index.
+    const position = this._positionForMenu(index);
+
+    // Before any modification, update window data.
+    Menu.updateWindowData();
+
+    // Begin DOM modifications.
+
     // Update the active index to the hovered item.
     this.activeIndex = index;
 
     // Open the new menu if a menu is already open.
     if (this._childMenu) {
-      this._openChildMenu();
+      this._openChildMenu(position);
     }
   }
 
+  /**
+   * Find initial position for the menu based on menubar item position.
+   *
+   * NOTE: this should be called before updating active index to avoid
+   * an additional layout and style invalidation as changing active
+   * index modifies DOM.
+   */
+  private _positionForMenu(index: number): Private.IPosition {
+    let itemNode = this.contentNode.children[index];
+    let { left, bottom } = (itemNode as HTMLElement).getBoundingClientRect();
+    return {
+      top: bottom,
+      left
+    };
+  }
+
   /**
    * Handle the `'mouseleave'` event for the menu bar.
    */
@@ -570,7 +597,7 @@ export class MenuBar extends Widget {
    * If a different child menu is already open, it will be closed,
    * even if there is no active menu.
    */
-  private _openChildMenu(): void {
+  private _openChildMenu(options: { left?: number; top?: number } = {}): void {
     // If there is no active menu, close the current menu.
     let newMenu = this.activeMenu;
     if (!newMenu) {
@@ -591,20 +618,28 @@ export class MenuBar extends Widget {
     if (oldMenu) {
       oldMenu.close();
     } else {
-      this.addClass('lm-mod-active');
       document.addEventListener('mousedown', this, true);
     }
 
     // Ensure the menu bar is updated and look up the item node.
     MessageLoop.sendMessage(this, Widget.Msg.UpdateRequest);
-    let itemNode = this.contentNode.children[this._activeIndex];
 
     // Get the positioning data for the new menu.
-    let { left, bottom } = (itemNode as HTMLElement).getBoundingClientRect();
+    let { left, top } = options;
+    if (typeof left === 'undefined' || typeof top === 'undefined') {
+      ({ left, top } = this._positionForMenu(this._activeIndex));
+    }
+
+    // Begin DOM modifications
+
+    if (!oldMenu) {
+      // Continue setup for new menu
+      this.addClass('lm-mod-active');
+    }
 
     // Open the new menu at the computed location.
     if (newMenu.items.length > 0) {
-      newMenu.open(left, bottom, this._forceItemsPosition);
+      newMenu.open(left, top, this._forceItemsPosition);
     }
   }
 
@@ -912,6 +947,20 @@ namespace Private {
     return node;
   }
 
+  /**
+   * Position for the menu relative to top-left screen corner.
+   */
+  export interface IPosition {
+    /**
+     * Pixels right from screen origin.
+     */
+    left: number;
+    /**
+     * Pixels down from screen origin.
+     */
+    top: number;
+  }
+
   /**
    * The results of a mnemonic search.
    */

packages/widgets/tests/src/menu.spec.ts

@@ -582,30 +582,28 @@ describe('@lumino/widgets', () => {
       it('should open the menu at the specified location', () => {
         menu.addItem({ command: 'test' });
         menu.open(10, 10);
-        expect(menu.node.style.left).to.equal('10px');
-        expect(menu.node.style.top).to.equal('10px');
+        expect(menu.node.style.transform).to.equal('translate(10px, 10px)');
       });
 
       it('should be adjusted to fit naturally on the screen', () => {
         menu.addItem({ command: 'test' });
         menu.open(-10, 10000);
-        expect(menu.node.style.left).to.equal('0px');
-        expect(menu.node.style.top).to.not.equal('10000px');
+        expect(menu.node.style.transform).to.equal('translate(0px, 10000px)');
       });
 
       it('should accept flags to force the location', () => {
         menu.addItem({ command: 'test' });
         menu.open(10000, 10000, { forceX: true, forceY: true });
-        expect(menu.node.style.left).to.equal('10000px');
-        expect(menu.node.style.top).to.equal('10000px');
+        expect(menu.node.style.transform).to.equal(
+          'translate(10000px, 10000px)'
+        );
       });
 
       it('should bail if already attached', () => {
         menu.addItem({ command: 'test' });
         menu.open(10, 10);
         menu.open(100, 100);
-        expect(menu.node.style.left).to.equal('10px');
-        expect(menu.node.style.top).to.equal('10px');
+        expect(menu.node.style.transform).to.equal('translate(10px, 10px)');
       });
     });
 

review/api/widgets.api.md

@@ -679,6 +679,7 @@ export class Menu extends Widget {
     readonly renderer: Menu.IRenderer;
     get rootMenu(): Menu;
     triggerActiveItem(): void;
+    static updateWindowData(): void;
 }
 
 // @public