Repository: jupytercad/JupyterCAD (language: python)

Issue:
Remove the guidata option
We should get rid of the `guidata` entry from the schema and everywhere in the codebase.

Instead we should introduce a new `Visible` property on all shapes, and a `Color` property ([done](https://github.com/jupytercad/JupyterCAD/pull/397)).

This is a breaking change and we'll have to update jupytercad-freecad accordingly.


Discussion / hints:

Fix patch:
diff --git a/packages/base/src/3dview/helpers.ts b/packages/base/src/3dview/helpers.ts
index f60424e7..15ecd305 100644
--- a/packages/base/src/3dview/helpers.ts
+++ b/packages/base/src/3dview/helpers.ts
@@ -1,4 +1,4 @@
-import { IDict, IParsedShape } from '@jupytercad/schema';
+import { IParsedShape } from '@jupytercad/schema';
 import * as THREE from 'three';
 import {
   acceleratedRaycast,
@@ -113,22 +113,14 @@ export function buildShape(options: {
   clippingPlanes: THREE.Plane[];
   selected: boolean;
   isSolid: boolean;
-  guidata?: IDict;
   objColor?: THREE.Color | string | number;
 }): {
   meshGroup: THREE.Group;
   mainMesh: THREE.Mesh<THREE.BufferGeometry, THREE.MeshPhongMaterial>;
   edgesMeshes: LineSegments2[];
 } | null {
-  const {
-    objName,
-    data,
-    guidata,
-    isSolid,
-    clippingPlanes,
-    selected,
-    objColor
-  } = options;
+  const { objName, data, isSolid, clippingPlanes, selected, objColor } =
+    options;
   const { faceList, edgeList, jcObject } = data;
 
   const vertices: Array<number> = [];
@@ -157,19 +149,8 @@ export function buildShape(options: {
     vInd += vertexCoorLength / 3;
   }
 
-  let color = objColor || DEFAULT_MESH_COLOR;
-  let visible = jcObject.visible;
-  if (guidata && guidata[objName]) {
-    const objdata = guidata[objName];
-
-    if (Object.prototype.hasOwnProperty.call(objdata, 'color')) {
-      color = new THREE.Color(color);
-    }
-
-    if (Object.prototype.hasOwnProperty.call(objdata, 'visibility')) {
-      visible = guidata[objName]['visibility'];
-    }
-  }
+  const color = objColor || DEFAULT_MESH_COLOR;
+  const visible = jcObject.visible;
 
   // Compile the connected vertices and faces into a model
   // And add to the scene
diff --git a/packages/base/src/3dview/mainview.tsx b/packages/base/src/3dview/mainview.tsx
index f2bc10c7..fde5c94d 100644
--- a/packages/base/src/3dview/mainview.tsx
+++ b/packages/base/src/3dview/mainview.tsx
@@ -577,7 +577,6 @@ export class MainView extends React.Component<IProps, IStates> {
       this._scene.remove(this._clippingPlaneMesh);
     }
 
-    const guidata = this._model.sharedModel.getOption('guidata');
     const selectedNames = this._selectedMeshes.map(sel => sel.name);
     this._selectedMeshes = [];
 
@@ -603,7 +602,6 @@ export class MainView extends React.Component<IProps, IStates> {
         clippingPlanes: this._clippingPlanes,
         selected,
         isSolid,
-        guidata,
         objColor
       });
 
@@ -665,9 +663,6 @@ export class MainView extends React.Component<IProps, IStates> {
       }
     });
 
-    if (guidata) {
-      this._model.sharedModel?.setOption('guidata', guidata);
-    }
     // Update the reflength
     this._updateRefLength();
     // Set the expoded view if it's enabled
@@ -1074,34 +1069,34 @@ export class MainView extends React.Component<IProps, IStates> {
     sender: IJupyterCadDoc,
     change: MapChange
   ): void {
-    const guidata = sender.getOption('guidata');
+    const objects = sender.objects;
 
-    if (guidata) {
-      for (const objName in guidata) {
+    if (objects) {
+      for (const objData of objects) {
+        const objName = objData.name;
         const obj = this._meshGroup?.getObjectByName(objName) as
           | BasicMesh
           | undefined;
-        const objColor = obj?.material.color;
+
         if (!obj) {
           continue;
         }
-        if (
-          Object.prototype.hasOwnProperty.call(guidata[objName], 'visibility')
-        ) {
-          const explodedLineHelper =
-            this._explodedViewLinesHelperGroup?.getObjectByName(objName);
-          const objGuiData = guidata[objName];
-          if (objGuiData) {
-            obj.parent!.visible = objGuiData['visibility'];
-
-            if (explodedLineHelper) {
-              explodedLineHelper.visible = objGuiData['visibility'];
-            }
-          }
+
+        const isVisible = objData.visible;
+
+        const objColor = obj?.material.color;
+
+        obj.parent!.visible = isVisible;
+
+        const explodedLineHelper =
+          this._explodedViewLinesHelperGroup?.getObjectByName(objName);
+        if (explodedLineHelper) {
+          explodedLineHelper.visible = isVisible;
         }
+
         if (obj.material.color) {
-          if ('color' in guidata[objName]) {
-            const rgba = guidata[objName]['color'] as number[];
+          if ('color' in objData) {
+            const rgba = objData.color as number[];
             const color = new THREE.Color(rgba[0], rgba[1], rgba[2]);
             obj.material.color = color;
           } else {
diff --git a/packages/base/src/commands.ts b/packages/base/src/commands.ts
index 6ca3a556..63da5f8f 100644
--- a/packages/base/src/commands.ts
+++ b/packages/base/src/commands.ts
@@ -78,15 +78,9 @@ export function setVisible(
   name: string,
   value: boolean
 ) {
-  const guidata = sharedModel.getOption('guidata') || {};
-
-  if (guidata && guidata[name]) {
-    guidata[name]['visibility'] = false;
-  } else {
-    guidata[name] = { visibility: false };
-  }
-
-  sharedModel.setOption('guidata', guidata);
+  sharedModel.updateObjectByName(name, {
+    data: { key: 'visible', value }
+  });
 }
 
 const PARTS = {
diff --git a/packages/base/src/panelview/objecttree.tsx b/packages/base/src/panelview/objecttree.tsx
index 00159e27..d8ad01a5 100644
--- a/packages/base/src/panelview/objecttree.tsx
+++ b/packages/base/src/panelview/objecttree.tsx
@@ -318,7 +318,7 @@ class ObjectTreeReact extends React.Component<IProps, IStates> {
   };
 
   render(): React.ReactNode {
-    const { selectedNodes, openNodes, options } = this.state;
+    const { selectedNodes, openNodes } = this.state;
     const data = this.stateToTree();
 
     const selectedNodeIds: TreeNodeId[] = [];
@@ -375,21 +375,7 @@ class ObjectTreeReact extends React.Component<IProps, IStates> {
             if (jcadObj) {
               visible = jcadObj.visible;
             }
-            if (
-              jcadObj &&
-              options &&
-              options['guidata'] &&
-              Object.prototype.hasOwnProperty.call(
-                options['guidata'],
-                jcadObj.name
-              ) &&
-              Object.prototype.hasOwnProperty.call(
-                options['guidata'][jcadObj.name],
-                'visibility'
-              )
-            ) {
-              visible = options['guidata'][jcadObj.name]['visibility'];
-            }
+
             return (
               <div
                 className={`jpcad-control-panel-tree ${
@@ -421,24 +407,16 @@ class ObjectTreeReact extends React.Component<IProps, IStates> {
                         className={'jp-ToolbarButtonComponent'}
                         onClick={() => {
                           const objectId = opts.node.parentId as string;
-
-                          const guidata =
-                            this.props.cpModel.sharedModel?.getOption(
-                              'guidata'
-                            ) || { [objectId]: {} };
-
-                          if (guidata) {
-                            if (guidata[objectId]) {
-                              guidata[objectId]['visibility'] = !visible;
-                            } else {
-                              guidata[objectId] = { visibility: !visible };
+                          const obj = this.getObjectFromName(objectId);
+                          if (obj) {
+                            const sharedModel =
+                              this.props.cpModel.jcadModel?.sharedModel;
+                            if (sharedModel) {
+                              sharedModel.updateObjectByName(objectId, {
+                                data: { key: 'visible', value: !obj.visible }
+                              });
                             }
                           }
-
-                          this.props.cpModel.sharedModel?.setOption(
-                            'guidata',
-                            guidata
-                          );
                         }}
                         icon={visible ? visibilityIcon : visibilityOffIcon}
                       />
diff --git a/packages/schema/src/doc.ts b/packages/schema/src/doc.ts
index d79dc701..58428bdc 100644
--- a/packages/schema/src/doc.ts
+++ b/packages/schema/src/doc.ts
@@ -133,11 +133,6 @@ export class JupyterCadDoc
 
     if (this._objects.length > index) {
       this._objects.delete(index);
-      const guidata = this.getOption('guidata');
-      if (guidata) {
-        delete guidata[name];
-        this.setOption('guidata', guidata);
-      }
       this.removeOutput(name);
     }
   }
diff --git a/packages/schema/src/interfaces.ts b/packages/schema/src/interfaces.ts
index 4e6c941f..41cd89d8 100644
--- a/packages/schema/src/interfaces.ts
+++ b/packages/schema/src/interfaces.ts
@@ -216,7 +216,6 @@ export interface IParsedShape {
   faceList: Array<IFace>;
   edgeList: Array<IEdge>;
   meta?: IDict;
-  guiData?: IDict;
 }
 
 export interface IPostOperatorInput {
diff --git a/packages/schema/src/schema/jcad.json b/packages/schema/src/schema/jcad.json
index aedeeef7..7dd75c0c 100644
--- a/packages/schema/src/schema/jcad.json
+++ b/packages/schema/src/schema/jcad.json
@@ -128,11 +128,7 @@
       "type": "object",
       "default": {},
       "additionalProperties": false,
-      "properties": {
-        "guidata": {
-          "type": "object"
-        }
-      }
+      "properties": {}
     }
   }
 }


Test patch:
diff --git a/ui-tests/tests/ui.spec.ts-snapshots/MultiSelect-Cut-test-jcad-linux.png b/ui-tests/tests/ui.spec.ts-snapshots/MultiSelect-Cut-test-jcad-linux.png
index de884222..c99972b3 100644
Binary files a/ui-tests/tests/ui.spec.ts-snapshots/MultiSelect-Cut-test-jcad-linux.png and b/ui-tests/tests/ui.spec.ts-snapshots/MultiSelect-Cut-test-jcad-linux.png differ