Make Events Sheet more robust against changes in events outside the editor

newIDE/app/src/EventsSheet/EventsTree/EventHeightsCache.js

@@ -1,55 +1,22 @@
 // @flow
 
-type WatchedComponent = {
-  +onHeightsChanged: Function => void,
-};
+type OnHeightChangedCallback = (cb?: () => void) => void;
 
 /**
- * Store the height of events and notify a component whenever
- * heights have changed.
- * Needed for EventsTree as we need to tell it when heights have changed
- * so it can recompute the internal row heights of the react-virtualized List.
+ * Store the height of events.
+ * Needed for EventsTree as we need to tell the react-virtualized List
+ * the size of each event - which we only know after the event has been rendered.
  */
 export default class EventHeightsCache {
   eventHeights = {};
   updateTimeoutId: ?TimeoutID = null;
-  component: ?WatchedComponent = null;
+  onHeightsChanged: OnHeightChangedCallback;
 
-  constructor(component: WatchedComponent) {
-    this.component = component;
-  }
-
-  _notifyComponent() {
-    if (this.updateTimeoutId) {
-      return; // An update is already scheduled.
-    }
-
-    // Notify the component, on the next tick, that heights have changed
-    this.updateTimeoutId = setTimeout(() => {
-      if (this.component) {
-        this.component.onHeightsChanged(() => (this.updateTimeoutId = null));
-      } else {
-        this.updateTimeoutId = null;
-      }
-    }, 0);
+  constructor(onHeightsChanged: OnHeightChangedCallback) {
+    this.onHeightsChanged = onHeightsChanged;
   }
 
   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) {
-      // Notify the parent component that a height changed.
-      this._notifyComponent();
-    }
-
     this.eventHeights[event.ptr] = height;
   }
 

newIDE/app/src/EventsSheet/index.js

@@ -4,7 +4,7 @@ import { I18n } from '@lingui/react';
 import { type I18n as I18nType } from '@lingui/core';
 
 import * as React from 'react';
-import EventsTree from './EventsTree';
+import EventsTree, { type EventsTreeInterface } from './EventsTree';
 import { getInstructionMetadata } from './InstructionEditor/InstructionEditor';
 import InstructionEditorDialog from './InstructionEditor/InstructionEditorDialog';
 import InstructionEditorMenu from './InstructionEditor/InstructionEditorMenu';
@@ -232,7 +232,7 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
   ComponentProps,
   State
 > {
-  _eventsTree: ?EventsTree;
+  _eventsTree: ?EventsTreeInterface;
   _eventSearcher: ?EventsSearcher;
   _searchPanel: ?SearchPanelInterface;
   _containerDiv = React.createRef<HTMLDivElement>();
@@ -558,16 +558,16 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
       insertion.indexInList + 1
     );
 
-    const currentTree = this._eventsTree;
-    if (currentTree) {
-      currentTree.forceEventsUpdate(() => {
+    const eventsTree = this._eventsTree;
+    if (eventsTree) {
+      eventsTree.forceEventsUpdate(() => {
         const positions = this._getChangedEventRows([newEvent]);
         this._saveChangesToHistory(
           'ADD',
           { positionsBeforeAction: positions, positionAfterAction: positions },
           () => {
             if (!context && !selectedEventContext) {
-              currentTree.scrollToRow(currentTree.getEventRow(newEvent));
+              eventsTree.scrollToRow(eventsTree.getEventRow(newEvent));
             }
           }
         );
@@ -580,7 +580,7 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
           (type === 'BuiltinCommonInstructions::Comment' ||
             type === 'BuiltinCommonInstructions::Group')
         ) {
-          const rowIndex = currentTree.getEventRow(newEvent);
+          const rowIndex = eventsTree.getEventRow(newEvent);
           const clickableElement = document.querySelector(
             `[data-row-index="${rowIndex}"] [data-editable-text="true"]`
           );
@@ -1372,9 +1372,9 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
   };
 
   _getChangedEventRows = (events: Array<gdBaseEvent>) => {
-    const currentTree = this._eventsTree;
-    if (currentTree) {
-      return events.map(event => currentTree.getEventRow(event));
+    const eventsTree = this._eventsTree;
+    if (eventsTree) {
+      return events.map(event => eventsTree.getEventRow(event));
     }
     return [];
   };
@@ -1484,10 +1484,11 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
       // 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();
+      let eventContexts: Array<EventContext> = [];
       if (type === 'DELETE') {
         newSelection = clearSelection();
       } else {
-        const eventContexts = eventsTree.getEventContextAtRowIndexes(
+        eventContexts = eventsTree.getEventContextAtRowIndexes(
           positions.positionAfterAction
         );
         newSelection = selectEventsAfterHistoryChange(eventContexts);

newIDE/app/src/fixtures/TestProject.js

@@ -20,6 +20,7 @@ export type TestProject = {|
   spriteObjectWithoutBehaviors: gdObject,
   testSpriteObjectInstance: gdInitialInstance,
   testLayout: gdLayout,
+  testProjectScopedContainersAccessor: ProjectScopedContainersAccessor,
   testSceneProjectScopedContainersAccessor: ProjectScopedContainersAccessor,
   group1: gdObjectGroup,
   group2: gdObjectGroup,
@@ -902,6 +903,12 @@ export const makeTestProject = (gd /*: libGDevelop */) /*: TestProject */ => {
     'whatever-this-is-not-recognised'
   );
 
+  const testProjectScopedContainersAccessor = new ProjectScopedContainersAccessor(
+    {
+      project
+    }
+  );
+
   const testSceneProjectScopedContainersAccessor = new ProjectScopedContainersAccessor(
     {
       project,
@@ -942,6 +949,7 @@ export const makeTestProject = (gd /*: libGDevelop */) /*: TestProject */ => {
     spriteObjectWithBehaviors,
     spriteObjectWithoutBehaviors,
     testLayout,
+    testProjectScopedContainersAccessor,
     testSceneProjectScopedContainersAccessor,
     group1,
     group2,

newIDE/app/src/stories/componentStories/EventsSheet/EventsTree.stories.js

@@ -42,6 +42,9 @@ export const DefaultMediumScreenScopeInLayout = () => (
             project: testProject.project,
             layout: testProject.testLayout,
           }}
+          projectScopedContainersAccessor={
+            testProject.testSceneProjectScopedContainersAccessor
+          }
           globalObjectsContainer={testProject.project.getObjects()}
           objectsContainer={testProject.testLayout.getObjects()}
           selection={getInitialSelection()}
@@ -53,6 +56,10 @@ export const DefaultMediumScreenScopeInLayout = () => (
           onInstructionDoubleClick={action('instruction double click')}
           onInstructionContextMenu={action('instruction context menu')}
           onAddInstructionContextMenu={action('instruction list context menu')}
+          onVariableDeclarationDoubleClick={action(
+            'onVariableDeclarationDoubleClick'
+          )}
+          onVariableDeclarationClick={action('onVariableDeclarationClick')}
           onParameterClick={action('parameter click')}
           onEventClick={action('event click')}
           onEventContextMenu={action('event context menu')}
@@ -66,6 +73,8 @@ export const DefaultMediumScreenScopeInLayout = () => (
           screenType={'normal'}
           windowSize={'medium'}
           eventsSheetHeight={500}
+          eventsSheetWidth={500}
+          indentScale={1}
           preferences={initialPreferences}
           tutorials={eventsTreeTutorials}
           onEndEditingEvent={action('end editing event')}
@@ -86,6 +95,9 @@ export const DefaultSmallScreenScopeInLayout = () => (
             project: testProject.project,
             layout: testProject.testLayout,
           }}
+          projectScopedContainersAccessor={
+            testProject.testSceneProjectScopedContainersAccessor
+          }
           globalObjectsContainer={testProject.project.getObjects()}
           objectsContainer={testProject.testLayout.getObjects()}
           selection={getInitialSelection()}
@@ -97,6 +109,10 @@ export const DefaultSmallScreenScopeInLayout = () => (
           onInstructionDoubleClick={action('instruction double click')}
           onInstructionContextMenu={action('instruction context menu')}
           onAddInstructionContextMenu={action('instruction list context menu')}
+          onVariableDeclarationDoubleClick={action(
+            'onVariableDeclarationDoubleClick'
+          )}
+          onVariableDeclarationClick={action('onVariableDeclarationClick')}
           onParameterClick={action('parameter click')}
           onEventClick={action('event click')}
           onEventContextMenu={action('event context menu')}
@@ -110,6 +126,8 @@ export const DefaultSmallScreenScopeInLayout = () => (
           screenType={'normal'}
           windowSize={'small'}
           eventsSheetHeight={500}
+          eventsSheetWidth={500}
+          indentScale={1}
           preferences={initialPreferences}
           tutorials={eventsTreeTutorials}
           onEndEditingEvent={action('end editing event')}
@@ -127,6 +145,9 @@ export const DefaultMediumScreenScopeNotInLayout = () => (
           events={testProject.testLayout.getEvents()}
           project={testProject.project}
           scope={{ project: testProject.project }}
+          projectScopedContainersAccessor={
+            testProject.testProjectScopedContainersAccessor
+          }
           globalObjectsContainer={testProject.project.getObjects()}
           objectsContainer={testProject.testLayout.getObjects()}
           selection={getInitialSelection()}
@@ -138,6 +159,10 @@ export const DefaultMediumScreenScopeNotInLayout = () => (
           onInstructionDoubleClick={action('instruction double click')}
           onInstructionContextMenu={action('instruction context menu')}
           onAddInstructionContextMenu={action('instruction list context menu')}
+          onVariableDeclarationDoubleClick={action(
+            'onVariableDeclarationDoubleClick'
+          )}
+          onVariableDeclarationClick={action('onVariableDeclarationClick')}
           onParameterClick={action('parameter click')}
           onEventClick={action('event click')}
           onEventContextMenu={action('event context menu')}
@@ -151,6 +176,8 @@ export const DefaultMediumScreenScopeNotInLayout = () => (
           screenType={'normal'}
           windowSize={'medium'}
           eventsSheetHeight={500}
+          eventsSheetWidth={500}
+          indentScale={1}
           preferences={initialPreferences}
           tutorials={eventsTreeTutorials}
           onEndEditingEvent={action('end editing event')}
@@ -171,6 +198,9 @@ export const EmptySmallScreenScopeInALayout = () => (
             project: testProject.project,
             layout: testProject.testLayout,
           }}
+          projectScopedContainersAccessor={
+            testProject.testSceneProjectScopedContainersAccessor
+          }
           globalObjectsContainer={testProject.project.getObjects()}
           objectsContainer={testProject.testLayout.getObjects()}
           selection={getInitialSelection()}
@@ -182,6 +212,10 @@ export const EmptySmallScreenScopeInALayout = () => (
           onInstructionDoubleClick={action('instruction double click')}
           onInstructionContextMenu={action('instruction context menu')}
           onAddInstructionContextMenu={action('instruction list context menu')}
+          onVariableDeclarationDoubleClick={action(
+            'onVariableDeclarationDoubleClick'
+          )}
+          onVariableDeclarationClick={action('onVariableDeclarationClick')}
           onParameterClick={action('parameter click')}
           onEventClick={action('event click')}
           onEventContextMenu={action('event context menu')}
@@ -195,6 +229,8 @@ export const EmptySmallScreenScopeInALayout = () => (
           screenType={'normal'}
           windowSize={'small'}
           eventsSheetHeight={500}
+          eventsSheetWidth={500}
+          indentScale={1}
           preferences={initialPreferences}
           tutorials={eventsTreeTutorials}
           onEndEditingEvent={action('end editing event')}