(core) Fixing scrolly bug on mobile

Summary:
On mobile view not all rows are rendered when a section is expanded.
Scrolly component calculates height of the GridView too soon (before animation is
completed). With this change on mobile view we always take the screen height for
calculation.

A similar bug was on Card List, where cards were squeezed and their height was
calculated to soon.

Test Plan: Added test

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3748
This commit is contained in:
Jarosław Sadziński 2023-01-04 17:37:32 +01:00
parent cd72a54bbb
commit d107810127
4 changed files with 45 additions and 15 deletions

View File

@ -1,4 +1,4 @@
/* globals $ */
/* globals $, window */
const _ = require('underscore');
const ko = require('knockout');
@ -40,7 +40,7 @@ const {RowContextMenu} = require('../ui/RowContextMenu');
const {setPopupToCreateDom} = require('popweasel');
const {CellContextMenu} = require('app/client/ui/CellContextMenu');
const {testId} = require('app/client/ui2018/cssVars');
const {testId, isNarrowScreen} = require('app/client/ui2018/cssVars');
const {contextMenu} = require('app/client/ui/contextMenu');
const {mouseDragMatchElem} = require('app/client/ui/mouseDrag');
const {menuToggle} = require('app/client/ui/MenuToggle');
@ -1306,14 +1306,18 @@ GridView.prototype.buildDom = function() {
/** @inheritdoc */
GridView.prototype.onResize = function() {
const activeFieldBuilder = this.activeFieldBuilder();
let height = null;
if (isNarrowScreen()) {
height = window.outerHeight;
}
if (activeFieldBuilder && activeFieldBuilder.isEditorActive()) {
// When the editor is active, the common case for a resize is if the virtual keyboard is being
// shown on mobile device. In that case, we need to scroll active cell into view, and need to
// do it synchronously, to allow repositioning the editor to it in response to the same event.
this.scrolly.updateSize();
this.scrolly.updateSize(height);
this.scrolly.scrollRowIntoView(this.cursor.rowIndex.peek());
} else {
this.scrolly.scheduleUpdateSize();
this.scrolly.scheduleUpdateSize(height);
}
this.width(this.scrollPane.clientWidth)
};

View File

@ -15,13 +15,14 @@ import {reportError} from 'app/client/models/errors';
import {filterBar} from 'app/client/ui/FilterBar';
import {viewSectionMenu} from 'app/client/ui/ViewSectionMenu';
import {buildWidgetTitle} from 'app/client/ui/WidgetTitle';
import {colors, isNarrowScreenObs, mediaSmall, testId, theme} from 'app/client/ui2018/cssVars';
import {colors, isNarrowScreen, isNarrowScreenObs, mediaSmall, testId, theme} from 'app/client/ui2018/cssVars';
import {icon} from 'app/client/ui2018/icons';
import {DisposableWithEvents} from 'app/common/DisposableWithEvents';
import {mod} from 'app/common/gutil';
import {Observable} from 'grainjs';
import * as ko from 'knockout';
import * as _ from 'underscore';
import debounce from 'lodash/debounce';
import {computedArray, Disposable, dom, fromKo, Holder, IDomComponent, styled, subscribe} from 'grainjs';
// tslint:disable:no-console
@ -133,6 +134,23 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
// (See https://stackoverflow.com/questions/2381336/detect-click-into-iframe-using-javascript).
this.listenTo(this.gristDoc.app, 'clipboard_blur', this._maybeFocusInSection);
// On narrow screens (e.g. mobile), we need to resize the section after a transition.
// There will two transition events (one from section one from row), so we debounce them after a tick.
const handler = debounce((e: TransitionEvent) => {
// We work only on the transition of the flex-grow property, and only on narrow screens.
if (e.propertyName !== 'flex-grow' || !isNarrowScreen()) { return; }
// Make sure the view is still active.
if (this.viewModel.isDisposed() || !this.viewModel.activeSection) { return; }
const section = this.viewModel.activeSection.peek();
if (!section || section.isDisposed()) { return; }
const view = section.viewInstance.peek();
if (!view || view.isDisposed()) { return; }
// Make resize.
view.onResize();
}, 0);
this._layout.rootElem.addEventListener('transitionend', handler);
// Don't need to dispose the listener, as the rootElem is disposed with the layout.
const classActive = cssLayoutBox.className + '-active';
const classInactive = cssLayoutBox.className + '-inactive';
this.autoDispose(subscribe(fromKo(this.viewModel.activeSection), (use, section) => {
@ -140,6 +158,7 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
this._layout.forEachBox((box: {dom: Element}) => {
box.dom.classList.add(classInactive);
box.dom.classList.remove(classActive);
box.dom.classList.remove("transition");
});
let elem: Element|null = this._layout.getLeafBox(id)?.dom;
while (elem?.matches('.layout_box')) {
@ -147,7 +166,9 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
elem.classList.add(classActive);
elem = elem.parentElement;
}
if (!isNarrowScreen()) {
section.viewInstance.peek()?.onResize();
}
}));
const commandGroup = {
@ -406,7 +427,7 @@ const cssViewLeafInactive = styled('div', `
const cssLayoutBox = styled('div', `
@media screen and ${mediaSmall} {
&-active, &-inactive {
transition: flex-grow 0.4s;
transition: flex-grow var(--grist-layout-animation-duration, 0.4s); // Exposed for tests
}
&-active > &-inactive,
&-active > &-inactive.layout_hbox .layout_hbox,

View File

@ -274,9 +274,9 @@ Scrolly.prototype.addPane = function(containerElem, options, itemCreateFunc) {
/**
* Tells Scrolly to call updateSize after things have had a chance to render.
*/
Scrolly.prototype.scheduleUpdateSize = function() {
Scrolly.prototype.scheduleUpdateSize = function(overrideHeight) {
if (!this.isDisposed() && !this.delayedUpdateSize.isPending()) {
this.delayedUpdateSize.schedule(0, this.updateSize, this);
this.delayedUpdateSize.schedule(0, this.updateSize.bind(this, overrideHeight), this);
}
};
@ -284,15 +284,16 @@ Scrolly.prototype.scheduleUpdateSize = function() {
* Measures the size of the panes and adjusts Scrolly parameters for how many rows to render.
* This should be called as soon as all Scrolly panes have been attached to the Document, and any
* time their outer size changes.
* Pass in an overrideHeight to use instead of the current height of the panes.
*/
Scrolly.prototype.updateSize = function() {
Scrolly.prototype.updateSize = function(overrideHeight) {
this.resetHeights();
this.shownHeight = Math.max(0, Math.max.apply(null, this.panes.map(function(pane) {
return pane.container.clientHeight;
})));
// Update counts of rows that are shown.
var numVisible = Math.max(1, Math.ceil(this.shownHeight / this.minRowHeight));
var numVisible = Math.max(1, Math.ceil((overrideHeight ?? this.shownHeight) / this.minRowHeight));
this.numBuffered = 5;
this.numRendered = numVisible + 2 * this.numBuffered;

View File

@ -227,8 +227,13 @@ export function getSection(sectionOrTitle: string|WebElement): WebElement|WebEle
* Click into a section without disrupting cursor positions.
*/
export async function selectSectionByTitle(title: string) {
try {
// .test-viewsection is a special 1px width element added for tests only.
await driver.findContent(`.test-viewsection-title`, title).find(".test-viewsection-blank").click();
} catch (e) {
// We might be in mobile view.
await driver.findContent(`.test-viewsection-title`, title).findClosest(".view_leaf").click();
}
}
@ -2321,7 +2326,6 @@ export function bigScreen() {
*/
export function narrowScreen() {
resizeWindowForSuite(400, 750);
}
export async function addSupportUserIfPossible() {