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.
This commit is contained in:
Florian Rival
2023-08-31 10:09:47 +02:00
committed by GitHub
parent ef9a82ee33
commit f07e3bfe19
4 changed files with 66 additions and 8 deletions
@@ -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;
}
}
@@ -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 && (
<div style={styles.eventComponentContainer}>
@@ -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 {
+39 -6
View File
@@ -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();
});
};
};