From 193d5360adea055367d6ed59b7b907a046cbe77e Mon Sep 17 00:00:00 2001 From: Cyprien P Date: Thu, 1 Sep 2022 10:07:15 +0200 Subject: [PATCH] (core) Fix prevent auto-expansion when user is resizing browser window Summary: Diff fixes an annoying issue when the left panel would sometimes auto-expand when the user is resizing the window. Alghouth it is quite easy to reproduce the issue still would happen a bit randomly. Here are what was done to fix it: 1) The most annoying manifestation of the issue happens with doc menu page. Indeed the panel is not meant to be collapsing hence triggering overlapping expansion messes UI a great deal. This was fix by asserting that the panel was collapsible before triggering expansion `if (left.hideOpener) { return; }`. 2) To prevent issue to happen also with doc page, we first test whether the user is actually dragging using `ev1.buttons !== 0` and also we've added a `isScreeResizingObs` observable. Although this seems to have fixed the problem for me, other developers still reports occurence of the issue on there machine but at a lesser frequence. It is unknown what this solution does not fully work, still situation seems acceptable now (most annoying part was 1st item). Diff also brings another small improvement when using Grist in a split screen setup when Grist is on the right. Moving cursor back and forth between the two windows would frequently leave the left panel inadvertandly expanded. Diff added a fix to allow panel to collapse when cursor leave window. Test Plan: Tested manually as it is hard to test when selenium. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3562 --- app/client/ui/PagePanels.ts | 33 ++++++++++++++++++++++++++++----- app/client/ui2018/cssVars.ts | 15 +++++++++++++++ app/client/ui2018/menus.ts | 2 ++ 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/app/client/ui/PagePanels.ts b/app/client/ui/PagePanels.ts index 8e39a00f..f00b9146 100644 --- a/app/client/ui/PagePanels.ts +++ b/app/client/ui/PagePanels.ts @@ -6,7 +6,7 @@ import {watchElementForBlur} from 'app/client/lib/FocusLayer'; import {urlState} from "app/client/models/gristUrlState"; import {resizeFlexVHandle} from 'app/client/ui/resizeHandle'; import {transition, TransitionWatcher} from 'app/client/ui/transitions'; -import {colors, cssHideForNarrowScreen, mediaNotSmall, mediaSmall} from 'app/client/ui2018/cssVars'; +import {colors, cssHideForNarrowScreen, isScreenResizing, mediaNotSmall, mediaSmall} from 'app/client/ui2018/cssVars'; import {isNarrowScreenObs} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; import {dom, DomArg, MultiHolder, noTestId, Observable, styled, subscribe, TestId} from "grainjs"; @@ -50,6 +50,7 @@ export function pagePanels(page: PageContents) { const onResize = page.onResize || (() => null); const leftOverlap = Observable.create(null, false); const dragResizer = Observable.create(null, false); + const isScreenResizingObs = isScreenResizing(); let lastLeftOpen = left.panelOpen.get(); let lastRightOpen = right?.panelOpen.get() || false; @@ -60,11 +61,14 @@ export function pagePanels(page: PageContents) { // last desktop state. const sub1 = subscribe(isNarrowScreenObs(), (use, narrow) => { if (narrow) { - lastLeftOpen = left.panelOpen.get(); + lastLeftOpen = leftOverlap.get() ? false : left.panelOpen.get(); lastRightOpen = right?.panelOpen.get() || false; } left.panelOpen.set(narrow ? false : lastLeftOpen); right?.panelOpen.set(narrow ? false : lastRightOpen); + + // overlap should always be OFF when switching screen mode + leftOverlap.set(false); }); // When url changes, we must have navigated; close the left panel since if it were open, it was @@ -129,8 +133,18 @@ export function pagePanels(page: PageContents) { }), // opening left panel on over - dom.on('mouseenter', (_ev, elem) => { - if (left.panelOpen.get()) { return; } + dom.on('mouseenter', (evt1, elem) => { + + + if (left.panelOpen.get() + + // when no opener should not auto-expand + || left.hideOpener + + // if user is resizing the window, don't expand. + || isScreenResizingObs.get()) { return; } + + let isMouseInsideLeftPane = true; let isFocusInsideLeftPane = false; @@ -161,7 +175,6 @@ export function pagePanels(page: PageContents) { // updates isFocusInsideLeftPane and starts watch for blur on activeElement. const watchBlur = debounce(() => { if (owner.isDisposed()) { return; } - // console.warn('watchBlur', document.activeElement); isFocusInsideLeftPane = Boolean(leftPaneDom.contains(document.activeElement) || document.activeElement?.closest('.grist-floating-menu')); maybeStartCollapse(); @@ -189,6 +202,16 @@ export function pagePanels(page: PageContents) { owner.autoDispose(dom.onElem(document, 'mousemove', onMouseEvt)); owner.autoDispose(dom.onElem(document, 'mouseup', onMouseEvt)); + // Enables collapsing when the cursor leaves the window. This comes handy in a split + // screen setup, especially when Grist is on the right side: moving the cursor back and + // forth between the 2 windows, the cursor is likely to hover the left pane and expand it + // inadvertendly. This line collapses it back. + const onMouseLeave = () => { + isMouseInsideLeftPane = false; + maybeStartCollapse(); + }; + owner.autoDispose(dom.onElem(document.body, 'mouseleave', onMouseLeave)); + // schedule start of expansion const timeoutId = setTimeout(startExpansion, AUTO_EXPAND_TIMEOUT_MS); }), diff --git a/app/client/ui2018/cssVars.ts b/app/client/ui2018/cssVars.ts index 6ee87cc0..d9d7bc60 100644 --- a/app/client/ui2018/cssVars.ts +++ b/app/client/ui2018/cssVars.ts @@ -9,6 +9,7 @@ import {urlState} from 'app/client/models/gristUrlState'; import {getTheme, ProductFlavor} from 'app/client/ui/CustomThemes'; import {dom, makeTestId, Observable, styled, TestId} from 'grainjs'; +import debounce = require('lodash/debounce'); import values = require('lodash/values'); const VAR_PREFIX = 'grist'; @@ -222,6 +223,20 @@ export const cssHideForNarrowScreen = styled('div', ` } `); +let _isScreenResizingObs: Observable|undefined; + +// Returns a singleton observable for whether user is currently resizing the window. (listen to +// `resize` events and uses a timer of 1000ms). +export function isScreenResizing(): Observable { + if (!_isScreenResizingObs) { + const obs = Observable.create(null, false); + const ping = debounce(() => obs.set(false), 1000); + window.addEventListener('resize', () => { obs.set(true); ping(); }); + _isScreenResizingObs = obs; + } + return _isScreenResizingObs; +} + /** * Attaches the global css properties to the document's root to them available in the page. */ diff --git a/app/client/ui2018/menus.ts b/app/client/ui2018/menus.ts index 9aff3a4c..0f2d36a1 100644 --- a/app/client/ui2018/menus.ts +++ b/app/client/ui2018/menus.ts @@ -20,6 +20,8 @@ export interface IOptionFull { let _lastOpenedController: weasel.IOpenController|null = null; // Close opened menu if any, otherwise do nothing. +// WARN: current implementation does not handle submenus correctly. Does not seem a problem as of +// today though, as there is no submenu in UI. export function closeRegisteredMenu() { if (_lastOpenedController) { _lastOpenedController.close(); } }