[1851] Fix an issue where the explorer could be expanded while not sy…

…nchronized Bug: #1851 Signed-off-by: Stéphane Bégaudeau <[email protected]>
eclipse-sirius · Mar 16, 2023 · 3af2f18 · 3af2f18
1 parent 23f2948
commit 3af2f18
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 48 deletions.
diff --git a/packages/trees/frontend/sirius-components-trees/src/toolbar/TreeToolBar.tsx b/packages/trees/frontend/sirius-components-trees/src/toolbar/TreeToolBar.tsx
@@ -51,7 +51,7 @@ export const TreeToolBar = ({ editingContextId, onSynchronizedClick, synchronize
           color="inherit"
           aria-label="New model"
           title="New model"
-          onClick={() => openNewDocumentModal()}
+          onClick={openNewDocumentModal}
           data-testid="new-model">
           <AddIcon />
         </IconButton>
@@ -61,7 +61,7 @@ export const TreeToolBar = ({ editingContextId, onSynchronizedClick, synchronize
           color="inherit"
           aria-label="Upload model"
           title="Upload model"
-          onClick={() => openUploadDocumentModal()}
+          onClick={openUploadDocumentModal}
           data-testid="upload-document">
           <PublishIcon />
         </IconButton>
@@ -70,7 +70,7 @@ export const TreeToolBar = ({ editingContextId, onSynchronizedClick, synchronize
           size="small"
           aria-label={preferenceButtonSynchroniseTitle}
           title={preferenceButtonSynchroniseTitle}
-          onClick={() => onSynchronizedClick()}
+          onClick={onSynchronizedClick}
           data-testid="tree-synchronise">
           <SwapHorizIcon color={synchronized ? 'inherit' : 'disabled'} />
         </IconButton>

diff --git a/packages/trees/frontend/sirius-components-trees/src/views/ExplorerView.tsx b/packages/trees/frontend/sirius-components-trees/src/views/ExplorerView.tsx
@@ -28,6 +28,7 @@ import {
   GQLGetTreePathVariables,
 } from './ExplorerView.types';
 import {
+  AutoExpandToRevealSelectionEvent,
   ExplorerViewContext,
   ExplorerViewEvent,
   explorerViewMachine,
@@ -38,8 +39,7 @@ import {
   HideToastEvent,
   SchemaValue,
   ShowToastEvent,
-  SynchronizeSelectionEvent,
-  SynchronizeWithRepresentationEvent,
+  SynchronizeWithSelectionEvent,
 } from './ExplorerViewMachine';
 import { getTreeEventSubscription } from './getTreeEventSubscription';
 const getTreePathQuery = gql`
@@ -66,25 +66,24 @@ export const ExplorerView = ({ editingContextId, selection, setSelection, readOn
 
   const [{ value, context }, dispatch] = useMachine<ExplorerViewContext, ExplorerViewEvent>(explorerViewMachine);
   const { toast, explorerView } = value as SchemaValue;
-  const { id, tree, expanded, maxDepth, synchronizedSelection, synchronizedWithRepresentation, message } = context;
+  const { id, tree, expanded, maxDepth, autoExpandToRevealSelection, synchronizedWithSelection, message } = context;
 
   const [getTreePath, { loading: treePathLoading, data: treePathData, error: treePathError }] = useLazyQuery<
     GQLGetTreePathData,
     GQLGetTreePathVariables
   >(getTreePathQuery);
 
+  // If we should auto-expand to reveal the selection, we need to compute the tree path to expand
   useEffect(() => {
-    if (tree && synchronizedSelection) {
+    if (tree && autoExpandToRevealSelection) {
       const variables: GQLGetTreePathVariables = {
         editingContextId,
         treeId: tree.id,
-        selectionEntryIds: synchronizedSelection ? selection.entries.map((entry) => entry.id) : [],
+        selectionEntryIds: selection.entries.map((entry) => entry.id),
       };
-      getTreePath({
-        variables,
-      });
+      getTreePath({ variables });
     }
-  }, [editingContextId, tree, selection, synchronizedSelection, getTreePath]);
+  }, [editingContextId, tree, selection, autoExpandToRevealSelection, getTreePath]);
 
   useEffect(() => {
     if (!treePathLoading) {
@@ -134,13 +133,11 @@ export const ExplorerView = ({ editingContextId, selection, setSelection, readOn
   }, [error, dispatch]);
 
   useEffect(() => {
-    if (synchronizedWithRepresentation) {
-      const synchronizeSelectionEvent: SynchronizeSelectionEvent = {
-        type: 'SYNCHRONIZE_SELECTION',
-        synchronizedSelection: true,
-      };
-      dispatch(synchronizeSelectionEvent);
-    }
+    const autoExpandToRevealSelectionEvent: AutoExpandToRevealSelectionEvent = {
+      type: 'AUTO_EXPAND_TO_REVEAL_SELECTION',
+      autoExpandToRevealSelection: true,
+    };
+    dispatch(autoExpandToRevealSelectionEvent);
   }, [selection]);
 
   const onExpand = (id: string, depth: number) => {
@@ -149,19 +146,19 @@ export const ExplorerView = ({ editingContextId, selection, setSelection, readOn
   };
 
   const onSynchronizedClick = () => {
-    const synchronizeWithRepresentationEvent: SynchronizeWithRepresentationEvent = {
-      type: 'SYNCHRONISE_WITH_REPRESENTATION',
-      synchronizedWithRepresentation: !synchronizedWithRepresentation,
+    const synchronizeWithSelectionEvent: SynchronizeWithSelectionEvent = {
+      type: 'SYNCHRONISE_WITH_SELECTION',
+      synchronizedWithSelection: !synchronizedWithSelection,
     };
-    dispatch(synchronizeWithRepresentationEvent);
+    dispatch(synchronizeWithSelectionEvent);
   };
   return (
     <>
       <div className="explorerToolBar">
         <TreeToolBar
           editingContextId={editingContextId}
           onSynchronizedClick={onSynchronizedClick}
-          synchronized={synchronizedWithRepresentation}
+          synchronized={synchronizedWithSelection}
           readOnly={readOnly}></TreeToolBar>
       </div>
       <div className={styles.explorerView} data-testid="explorer">

diff --git a/packages/trees/frontend/sirius-components-trees/src/views/ExplorerViewMachine.ts b/packages/trees/frontend/sirius-components-trees/src/views/ExplorerViewMachine.ts
@@ -49,8 +49,8 @@ export interface ExplorerViewContext {
   tree: GQLTree | null;
   expanded: string[];
   maxDepth: number;
-  synchronizedSelection: boolean;
-  synchronizedWithRepresentation: boolean;
+  autoExpandToRevealSelection: boolean;
+  synchronizedWithSelection: boolean;
   message: string | null;
 }
 
@@ -61,10 +61,13 @@ export type HandleSubscriptionResultEvent = {
   result: SubscriptionResult<GQLExplorerEventData>;
 };
 export type HandleCompleteEvent = { type: 'HANDLE_COMPLETE' };
-export type SynchronizeSelectionEvent = { type: 'SYNCHRONIZE_SELECTION'; synchronizedSelection: boolean };
-export type SynchronizeWithRepresentationEvent = {
-  type: 'SYNCHRONISE_WITH_REPRESENTATION';
-  synchronizedWithRepresentation: boolean;
+export type AutoExpandToRevealSelectionEvent = {
+  type: 'AUTO_EXPAND_TO_REVEAL_SELECTION';
+  autoExpandToRevealSelection: boolean;
+};
+export type SynchronizeWithSelectionEvent = {
+  type: 'SYNCHRONISE_WITH_SELECTION';
+  synchronizedWithSelection: boolean;
 };
 export type HandleExpandedEvent = { type: 'HANDLE_EXPANDED'; id: string; depth: number };
 export type HandleTreePathEvent = { type: 'HANDLE_TREE_PATH'; treePathData: GQLGetTreePathData };
@@ -73,10 +76,10 @@ export type ExplorerViewEvent =
   | HandleCompleteEvent
   | ShowToastEvent
   | HideToastEvent
-  | SynchronizeSelectionEvent
+  | AutoExpandToRevealSelectionEvent
   | HandleExpandedEvent
   | HandleTreePathEvent
-  | SynchronizeWithRepresentationEvent;
+  | SynchronizeWithSelectionEvent;
 
 const isTreeRefreshedEventPayload = (payload: GQLTreeEventPayload): payload is GQLTreeRefreshedEventPayload =>
   payload.__typename === 'TreeRefreshedEventPayload';
@@ -89,8 +92,8 @@ export const explorerViewMachine = Machine<ExplorerViewContext, ExplorerViewStat
       tree: null,
       expanded: [],
       maxDepth: 1,
-      synchronizedSelection: true,
-      synchronizedWithRepresentation: true,
+      autoExpandToRevealSelection: true,
+      synchronizedWithSelection: true,
       message: null,
     },
     states: {
@@ -139,8 +142,8 @@ export const explorerViewMachine = Machine<ExplorerViewContext, ExplorerViewStat
                 target: 'ready',
                 actions: 'handleSubscriptionResult',
               },
-              SYNCHRONIZE_SELECTION: {
-                actions: 'synchronizeSelection',
+              AUTO_EXPAND_TO_REVEAL_SELECTION: {
+                actions: 'autoExpandToRevealSelection',
               },
               HANDLE_EXPANDED: {
                 actions: 'expand',
@@ -151,8 +154,8 @@ export const explorerViewMachine = Machine<ExplorerViewContext, ExplorerViewStat
               HANDLE_COMPLETE: {
                 target: 'complete',
               },
-              SYNCHRONISE_WITH_REPRESENTATION: {
-                actions: 'synchronizeWithRepresentation',
+              SYNCHRONISE_WITH_SELECTION: {
+                actions: 'synchronizeWithSelection',
               },
             },
           },
@@ -182,16 +185,20 @@ export const explorerViewMachine = Machine<ExplorerViewContext, ExplorerViewStat
         }
         return {};
       }),
-      synchronizeWithRepresentation: assign((_, event) => {
-        const { synchronizedWithRepresentation } = event as SynchronizeWithRepresentationEvent;
-        return {
-          synchronizedWithRepresentation: synchronizedWithRepresentation,
-          synchronized: synchronizedWithRepresentation,
-        };
+      synchronizeWithSelection: assign((_, event) => {
+        const { synchronizedWithSelection } = event as SynchronizeWithSelectionEvent;
+        if (synchronizedWithSelection) {
+          return { synchronizedWithSelection };
+        }
+        return { synchronizedWithSelection, autoExpandToRevealSelection: false };
       }),
-      synchronizeSelection: assign((_, event) => {
-        const { synchronizedSelection } = event as SynchronizeSelectionEvent;
-        return { synchronizedSelection };
+      autoExpandToRevealSelection: assign((context, event) => {
+        const { autoExpandToRevealSelection } = event as AutoExpandToRevealSelectionEvent;
+        const { synchronizedWithSelection } = context;
+        if (synchronizedWithSelection) {
+          return { autoExpandToRevealSelection };
+        }
+        return {};
       }),
       expand: assign((context, event) => {
         const { expanded, maxDepth } = context;
@@ -202,7 +209,7 @@ export const explorerViewMachine = Machine<ExplorerViewContext, ExplorerViewStat
           newExpanded.splice(newExpanded.indexOf(id), 1);
 
           // Disable synchronize mode on collapse
-          return { expanded: newExpanded, synchronizedSelection: false, maxDepth: Math.max(maxDepth, depth) };
+          return { expanded: newExpanded, autoExpandToRevealSelection: false, maxDepth: Math.max(maxDepth, depth) };
         }
         return { expanded: [...expanded, id], maxDepth: Math.max(maxDepth, depth) };
       }),