From f07e3bfe19bfef581cb33f791f1cd02718e36782 Mon Sep 17 00:00:00 2001 From: Florian Rival Date: Thu, 31 Aug 2023 10:09:47 +0200 Subject: [PATCH] Reduce "layout shifts" in Events Sheet when adding events or modifying them (#5612) * Browsing the events sheet and editing events should be a bit more comfortable now. --- .../EventsTree/EventHeightsCache.js | 13 +++++- .../app/src/EventsSheet/EventsTree/index.js | 9 ++++ .../app/src/EventsSheet/EventsTree/style.css | 7 +++ newIDE/app/src/EventsSheet/index.js | 45 ++++++++++++++++--- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/newIDE/app/src/EventsSheet/EventsTree/EventHeightsCache.js b/newIDE/app/src/EventsSheet/EventsTree/EventHeightsCache.js index a402236db1..0c509521a8 100644 --- a/newIDE/app/src/EventsSheet/EventsTree/EventHeightsCache.js +++ b/newIDE/app/src/EventsSheet/EventsTree/EventHeightsCache.js @@ -35,9 +35,18 @@ export default class EventHeightsCache { } setEventHeight(event: gdBaseEvent, height: number) { + if (height === 0) { + // Don't store the new height because it's 0, meaning a new rendering later + // will then render the proper height. In the meantime, we don't want to + // store this empty rendering BUT we still need to notify the parent that + // an update is needed. + this._notifyComponent(); + return; + } + const cachedHeight = this.eventHeights[event.ptr]; if (cachedHeight === undefined || cachedHeight !== height) { - // console.log(event.ptr, 'has a new height', height, 'old:', cachedHeight); + // Notify the parent component that a height changed. this._notifyComponent(); } @@ -45,6 +54,6 @@ export default class EventHeightsCache { } getEventHeight(event: gdBaseEvent): number { - return this.eventHeights[event.ptr] || 60; + return this.eventHeights[event.ptr] || 0; } } diff --git a/newIDE/app/src/EventsSheet/EventsTree/index.js b/newIDE/app/src/EventsSheet/EventsTree/index.js index a827af0f66..c22a482f08 100644 --- a/newIDE/app/src/EventsSheet/EventsTree/index.js +++ b/newIDE/app/src/EventsSheet/EventsTree/index.js @@ -128,6 +128,10 @@ type EventsContainerProps = {| idPrefix: string, |}; +const hiddenBecauseHeightNotComputedYetStyle = { + visibility: 'hidden', +}; + /** * The component containing an event. * It will report the rendered event height so that the EventsTree can @@ -178,6 +182,11 @@ const EventContainer = (props: EventsContainerProps) => { onClick={props.onEventClick} onContextMenu={_onEventContextMenu} {...longTouchForContextMenuProps} + style={ + eventsHeightsCache.getEventHeight(event) + ? undefined + : hiddenBecauseHeightNotComputedYetStyle + } > {!!EventComponent && (
diff --git a/newIDE/app/src/EventsSheet/EventsTree/style.css b/newIDE/app/src/EventsSheet/EventsTree/style.css index d404b5900b..4d0ae77b3e 100644 --- a/newIDE/app/src/EventsSheet/EventsTree/style.css +++ b/newIDE/app/src/EventsSheet/EventsTree/style.css @@ -64,6 +64,13 @@ border-radius: 0; padding: 0; flex: 1; + + /* + * Align the renderered event to the top, so that it does not move + * if the container is too large and resized down (for exemple, when deleting + * an action or a condition. + */ + align-items: flex-start; } .gd-events-sheet .rst__rowWrapper { diff --git a/newIDE/app/src/EventsSheet/index.js b/newIDE/app/src/EventsSheet/index.js index 6c205bf552..655769eea1 100644 --- a/newIDE/app/src/EventsSheet/index.js +++ b/newIDE/app/src/EventsSheet/index.js @@ -1317,6 +1317,8 @@ export class EventsSheetComponentWithoutHandle extends React.Component< // /!\ Events were changed, so any reference to an existing event can now // be invalid. Make sure to immediately trigger a forced update before // any re-render that could use a deleted/invalid event. + if (this._eventSearcher) this._eventSearcher.reset(); + const { _eventsTree: eventsTree } = this; if (!eventsTree) return; eventsTree.forceEventsUpdate(() => { @@ -1326,8 +1328,6 @@ export class EventsSheetComponentWithoutHandle extends React.Component< } = newEventsHistory.futureActions[ newEventsHistory.futureActions.length - 1 ]; - // Whether it is an ADD, EDIT or DELETE, scroll to the place where it was done. - eventsTree.scrollToRow(positions.positionsBeforeAction[0]); let newSelection: SelectionState = getInitialSelection(); // If it is a DELETE or EDIT, then the element will be present, so we can select them. @@ -1340,12 +1340,25 @@ export class EventsSheetComponentWithoutHandle extends React.Component< ); newSelection = selectEventsAfterHistoryChange(eventContexts); } + this.setState( { selection: newSelection, eventsHistory: newEventsHistory, }, - () => this.updateToolbar() + () => { + // Whether it is an ADD, EDIT or DELETE, scroll to the place where it was done. + eventsTree.scrollToRow(positions.positionsBeforeAction[0]); + // Hack: because of the virtualization and the undo/redo, we lose the heights of events + // (at least some, because they are different objects in memory). + // While they are recomputed when rendered, scroll again to be sure we don't end + // up at the very beginning (if everything was recomputed from 0) or at + // an offset too large. + setTimeout(() => { + eventsTree.scrollToRow(positions.positionsBeforeAction[0]); + }, 70); + this.updateToolbar(); + } ); }); }; @@ -1359,6 +1372,8 @@ export class EventsSheetComponentWithoutHandle extends React.Component< // /!\ Events were changed, so any reference to an existing event can now // be invalid. Make sure to immediately trigger a forced update before // any re-render that could use a deleted/invalid event. + if (this._eventSearcher) this._eventSearcher.reset(); + const { _eventsTree: eventsTree } = this; if (!eventsTree) return; eventsTree.forceEventsUpdate(() => { @@ -1368,8 +1383,7 @@ export class EventsSheetComponentWithoutHandle extends React.Component< } = newEventsHistory.previousActions[ newEventsHistory.previousActions.length - 1 ]; - // Whether it was an ADD, EDIT or DELETE, scroll to the place where it will happen. - eventsTree.scrollToRow(positions.positionAfterAction); + // If it is a ADD or EDIT, then the element will be present, so we can select them. // If it is a DELETE, then they will not be present, so we can't select them. let newSelection: SelectionState = getInitialSelection(); @@ -1387,7 +1401,19 @@ export class EventsSheetComponentWithoutHandle extends React.Component< selection: newSelection, eventsHistory: newEventsHistory, }, - () => this.updateToolbar() + () => { + // Whether it was an ADD, EDIT or DELETE, scroll to the place where it will happen. + eventsTree.scrollToRow(positions.positionsBeforeAction[0]); + // Hack: because of the virtualization and the undo/redo, we lose the heights of events + // (at least some, because they are different objects in memory). + // While they are recomputed when rendered, scroll again to be sure we don't end + // up at the very beginning (if everything was recomputed from 0) or at + // an offset too large. + setTimeout(() => { + eventsTree.scrollToRow(positions.positionsBeforeAction[0]); + }, 70); + this.updateToolbar(); + } ); }); }; @@ -1411,6 +1437,13 @@ export class EventsSheetComponentWithoutHandle extends React.Component< zoomLevel.max ) ); + + // Force a new rendering - not strictly necessary but otherwise a user input + // is needed for events to occupy their proper new height. + setTimeout(() => { + const { _eventsTree: eventsTree } = this; + if (eventsTree) eventsTree.forceEventsUpdate(); + }); }; };