(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
This commit is contained in:
Cyprien P 2022-09-01 10:07:15 +02:00
parent b722fb3aaa
commit 193d5360ad
3 changed files with 45 additions and 5 deletions

View File

@ -6,7 +6,7 @@ import {watchElementForBlur} from 'app/client/lib/FocusLayer';
import {urlState} from "app/client/models/gristUrlState"; import {urlState} from "app/client/models/gristUrlState";
import {resizeFlexVHandle} from 'app/client/ui/resizeHandle'; import {resizeFlexVHandle} from 'app/client/ui/resizeHandle';
import {transition, TransitionWatcher} from 'app/client/ui/transitions'; 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 {isNarrowScreenObs} from 'app/client/ui2018/cssVars';
import {icon} from 'app/client/ui2018/icons'; import {icon} from 'app/client/ui2018/icons';
import {dom, DomArg, MultiHolder, noTestId, Observable, styled, subscribe, TestId} from "grainjs"; 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 onResize = page.onResize || (() => null);
const leftOverlap = Observable.create(null, false); const leftOverlap = Observable.create(null, false);
const dragResizer = Observable.create(null, false); const dragResizer = Observable.create(null, false);
const isScreenResizingObs = isScreenResizing();
let lastLeftOpen = left.panelOpen.get(); let lastLeftOpen = left.panelOpen.get();
let lastRightOpen = right?.panelOpen.get() || false; let lastRightOpen = right?.panelOpen.get() || false;
@ -60,11 +61,14 @@ export function pagePanels(page: PageContents) {
// last desktop state. // last desktop state.
const sub1 = subscribe(isNarrowScreenObs(), (use, narrow) => { const sub1 = subscribe(isNarrowScreenObs(), (use, narrow) => {
if (narrow) { if (narrow) {
lastLeftOpen = left.panelOpen.get(); lastLeftOpen = leftOverlap.get() ? false : left.panelOpen.get();
lastRightOpen = right?.panelOpen.get() || false; lastRightOpen = right?.panelOpen.get() || false;
} }
left.panelOpen.set(narrow ? false : lastLeftOpen); left.panelOpen.set(narrow ? false : lastLeftOpen);
right?.panelOpen.set(narrow ? false : lastRightOpen); 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 // 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 // opening left panel on over
dom.on('mouseenter', (_ev, elem) => { dom.on('mouseenter', (evt1, elem) => {
if (left.panelOpen.get()) { return; }
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 isMouseInsideLeftPane = true;
let isFocusInsideLeftPane = false; let isFocusInsideLeftPane = false;
@ -161,7 +175,6 @@ export function pagePanels(page: PageContents) {
// updates isFocusInsideLeftPane and starts watch for blur on activeElement. // updates isFocusInsideLeftPane and starts watch for blur on activeElement.
const watchBlur = debounce(() => { const watchBlur = debounce(() => {
if (owner.isDisposed()) { return; } if (owner.isDisposed()) { return; }
// console.warn('watchBlur', document.activeElement);
isFocusInsideLeftPane = Boolean(leftPaneDom.contains(document.activeElement) || isFocusInsideLeftPane = Boolean(leftPaneDom.contains(document.activeElement) ||
document.activeElement?.closest('.grist-floating-menu')); document.activeElement?.closest('.grist-floating-menu'));
maybeStartCollapse(); maybeStartCollapse();
@ -189,6 +202,16 @@ export function pagePanels(page: PageContents) {
owner.autoDispose(dom.onElem(document, 'mousemove', onMouseEvt)); owner.autoDispose(dom.onElem(document, 'mousemove', onMouseEvt));
owner.autoDispose(dom.onElem(document, 'mouseup', 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 // schedule start of expansion
const timeoutId = setTimeout(startExpansion, AUTO_EXPAND_TIMEOUT_MS); const timeoutId = setTimeout(startExpansion, AUTO_EXPAND_TIMEOUT_MS);
}), }),

View File

@ -9,6 +9,7 @@
import {urlState} from 'app/client/models/gristUrlState'; import {urlState} from 'app/client/models/gristUrlState';
import {getTheme, ProductFlavor} from 'app/client/ui/CustomThemes'; import {getTheme, ProductFlavor} from 'app/client/ui/CustomThemes';
import {dom, makeTestId, Observable, styled, TestId} from 'grainjs'; import {dom, makeTestId, Observable, styled, TestId} from 'grainjs';
import debounce = require('lodash/debounce');
import values = require('lodash/values'); import values = require('lodash/values');
const VAR_PREFIX = 'grist'; const VAR_PREFIX = 'grist';
@ -222,6 +223,20 @@ export const cssHideForNarrowScreen = styled('div', `
} }
`); `);
let _isScreenResizingObs: Observable<boolean>|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<boolean> {
if (!_isScreenResizingObs) {
const obs = Observable.create<boolean>(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. * Attaches the global css properties to the document's root to them available in the page.
*/ */

View File

@ -20,6 +20,8 @@ export interface IOptionFull<T> {
let _lastOpenedController: weasel.IOpenController|null = null; let _lastOpenedController: weasel.IOpenController|null = null;
// Close opened menu if any, otherwise do nothing. // 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() { export function closeRegisteredMenu() {
if (_lastOpenedController) { _lastOpenedController.close(); } if (_lastOpenedController) { _lastOpenedController.close(); }
} }