From 7d427b87439c577d11e987d80dc45cbef4e50fab Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Thu, 6 Oct 2022 20:38:20 +0100 Subject: [PATCH] Use simpler transient cache, move update requests --- packages/widgets/src/contextmenu.ts | 2 +- packages/widgets/src/menu.ts | 77 ++++++++++++----------------- packages/widgets/src/menubar.ts | 5 +- review/api/widgets.api.md | 2 +- 4 files changed, 36 insertions(+), 50 deletions(-) diff --git a/packages/widgets/src/contextmenu.ts b/packages/widgets/src/contextmenu.ts index a04a0e8e4..b6c7e313a 100644 --- a/packages/widgets/src/contextmenu.ts +++ b/packages/widgets/src/contextmenu.ts @@ -79,7 +79,7 @@ export class ContextMenu { */ open(event: MouseEvent): boolean { // Prior to any DOM modifications update the window data. - Menu.updateWindowData(); + Menu.saveWindowData(); // Clear the current contents of the context menu. this.menu.clearItems(); diff --git a/packages/widgets/src/menu.ts b/packages/widgets/src/menu.ts index fec4e84a7..e91570cf0 100644 --- a/packages/widgets/src/menu.ts +++ b/packages/widgets/src/menu.ts @@ -833,6 +833,9 @@ export class Menu extends Widget { return; } + // Prior to any DOM modifications save window data + Menu.saveWindowData(); + // Ensure the current child menu is closed. this._closeChildMenu(); @@ -916,14 +919,16 @@ export class Menu extends Widget { } /** - * Update window data used for menu positioning in window data cache. + * Save window data used for menu positioning in transient 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). + * + * The transient cache will be released upon `open()` call. */ - static updateWindowData(): void { - Private.updateWindowData(); + static saveWindowData(): void { + Private.saveWindowData(); } private _childIndex = -1; @@ -1390,48 +1395,30 @@ 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; + let transientWindowDataCache: IWindowData | null = null; + let transientCacheCounter: number = 0; - function getLatestWindowData(): IWindowData | null { - const currentTimestamp = Date.now(); - if (isWindowDataCacheValid(currentTimestamp)) { - return latestWindowData; + function getWindowData(): IWindowData { + // if transient cache is in use, take one from it + if (transientCacheCounter > 0) { + transientCacheCounter--; + return transientWindowDataCache!; } - return null; - } - - function isWindowDataCacheValid(timestamp: number) { - return timestamp - latestWindowDataTimestamp < WINDOW_DATA_CACHE_DURATION; + return _getWindowData(); } /** - * Update window data cache. + * Store window data in transient cache. + * + * The transient cache will be released upon `getWindowData()` call. + * If this function is called multiple times, the cache will be + * retained until as many calls to `getWindowData()` were made. * * 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; - } + export function saveWindowData(): void { + transientWindowDataCache = _getWindowData(); + transientCacheCounter++; } /** @@ -1552,16 +1539,16 @@ namespace Private { forceX: boolean, forceY: boolean ): void { - // Ensure the menu is updated before attaching and measuring. - MessageLoop.sendMessage(menu, Widget.Msg.UpdateRequest); - // Get the current position and size of the main viewport. - const windowData = getLatestWindowData() || _getWindowData(); + const windowData = getWindowData(); let px = windowData.pageXOffset; let py = windowData.pageYOffset; let cw = windowData.clientWidth; let ch = windowData.clientHeight; + // Ensure the menu is updated before attaching and measuring. + MessageLoop.sendMessage(menu, Widget.Msg.UpdateRequest); + // Compute the maximum allowed height for the menu. let maxHeight = ch - (forceY ? y : 0); @@ -1604,16 +1591,16 @@ namespace Private { * Open a menu as a submenu using an item node for positioning. */ export function openSubmenu(submenu: Menu, itemNode: HTMLElement): void { - // Ensure the menu is updated before opening. - MessageLoop.sendMessage(submenu, Widget.Msg.UpdateRequest); - // Get the current position and size of the main viewport. - const windowData = getLatestWindowData() || _getWindowData(); + const windowData = getWindowData(); let px = windowData.pageXOffset; let py = windowData.pageYOffset; let cw = windowData.clientWidth; let ch = windowData.clientHeight; + // Ensure the menu is updated before opening. + MessageLoop.sendMessage(submenu, Widget.Msg.UpdateRequest); + // Compute the maximum allowed height for the menu. let maxHeight = ch; diff --git a/packages/widgets/src/menubar.ts b/packages/widgets/src/menubar.ts index bd289e000..fbf9d066b 100644 --- a/packages/widgets/src/menubar.ts +++ b/packages/widgets/src/menubar.ts @@ -520,7 +520,7 @@ export class MenuBar extends Widget { this.activeIndex = index; } else { const position = this._positionForMenu(index); - Menu.updateWindowData(); + Menu.saveWindowData(); // Begin DOM modifications. this.activeIndex = index; this._openChildMenu(position); @@ -552,7 +552,7 @@ export class MenuBar extends Widget { const position = this._positionForMenu(index); // Before any modification, update window data. - Menu.updateWindowData(); + Menu.saveWindowData(); // Begin DOM modifications. @@ -629,7 +629,6 @@ export class MenuBar extends Widget { if (typeof left === 'undefined' || typeof top === 'undefined') { ({ left, top } = this._positionForMenu(this._activeIndex)); } - // Begin DOM modifications if (!oldMenu) { diff --git a/review/api/widgets.api.md b/review/api/widgets.api.md index e191f52c5..54f23ffac 100644 --- a/review/api/widgets.api.md +++ b/review/api/widgets.api.md @@ -678,8 +678,8 @@ export class Menu extends Widget { removeItemAt(index: number): void; readonly renderer: Menu.IRenderer; get rootMenu(): Menu; + static saveWindowData(): void; triggerActiveItem(): void; - static updateWindowData(): void; } // @public