Clean up

Author: gjmooney

Untitled.ipynb

@@ -1066,9 +1066,6 @@ export function addCommands(
         : false;
     },
     execute: args => {
-      console.log('addLandmark command executing');
-      // only want to use one story for now so hardcoding the id for now
-
       const storyMapId = UUID.uuid4();
       const newLandmarkId = UUID.uuid4();
       const current = tracker.currentWidget;
@@ -1078,7 +1075,7 @@ export function addCommands(
       const { zoom, extent } = current.model.getOptions();
 
       if (!zoom || !extent) {
-        console.log('shut up');
+        console.log('No extent or zoom found');
         return;
       }
 
@@ -1106,11 +1103,10 @@ export function addCommands(
 
         current.model.sharedModel.addStoryMap(storyMapId, storyMap);
       } else {
-        // else need to update tories
-        // get first story
-        const story = current.model.getSelectedStory().story;
+        // else need to update stories
+        const { story } = current.model.getSelectedStory();
         if (!story) {
-          console.log('brok');
+          console.log('No story found, something went wrong');
           return;
         }
         const newStory: IJGISStoryMap = {
@@ -1182,7 +1178,6 @@ namespace Private {
     return async () => {
       const current = tracker.currentWidget;
 
-      console.log('current', current);
       if (!current) {
         return;
       }

packages/base/src/commands/index.ts

@@ -2043,13 +2043,15 @@ export class MainView extends React.Component<IProps, IStates> {
     const layer = this.getLayer(id);
     const source = layer?.getSource();
 
+    // TODO: Landmark layers don't have an associated OL layer
+    // This could be better
     if (!layer) {
-      const jLayer = this._model.getLayer(id);
-      const p = jLayer?.parameters as ILandmarkLayer;
-      this._Map.getView().fit(p.extent!, {
+      const jgisLayer = this._model.getLayer(id);
+      const layerParams = jgisLayer?.parameters as ILandmarkLayer;
+      this._Map.getView().fit(layerParams.extent!, {
         size: this._Map.getSize(),
         duration: 500,
-        maxZoom: p.zoom!,
+        maxZoom: layerParams.zoom!,
       });
       return;
     }

packages/base/src/mainview/mainView.tsx

@@ -45,14 +45,14 @@ interface IBodyProps {
   model: IJupyterGISModel;
   commands: CommandRegistry;
   state: IStateDB;
-  layerTree?: IJGISLayerTree;
+  layerTree: IJGISLayerTree;
 }
 
 export const LayersBodyComponent: React.FC<IBodyProps> = props => {
   const model = props.model;
 
   const [layerTree, setLayerTree] = useState<IJGISLayerTree>(
-    props.layerTree || model?.getLayerTree() || [],
+    props.layerTree || [],
   );
 
   const notifyCommands = () => {
@@ -155,7 +155,7 @@ export const LayersBodyComponent: React.FC<IBodyProps> = props => {
     } else {
       // Check if new selection is the same type as previous selections
       const isSelectedSameType = Object.values(selectedValue).some(
-        selection => (selection as ISelection).type === type,
+        selection => selection.type === type,
       );
 
       if (!isSelectedSameType) {
@@ -186,29 +186,6 @@ export const LayersBodyComponent: React.FC<IBodyProps> = props => {
     onSelect({ type, item, event });
   };
 
-  /**
-   * Listen to the layers and layer tree changes.
-   */
-  useEffect(() => {
-    const updateLayers = () => {
-      setLayerTree(props.layerTree || model?.getLayerTree() || []);
-    };
-
-    // Only listen to changes if layerTree is not provided as prop
-    if (!props.layerTree) {
-      model?.sharedModel.layersChanged.connect(updateLayers);
-      model?.sharedModel.layerTreeChanged.connect(updateLayers);
-    }
-
-    updateLayers();
-    return () => {
-      if (!props.layerTree) {
-        model?.sharedModel.layersChanged.disconnect(updateLayers);
-        model?.sharedModel.layerTreeChanged.disconnect(updateLayers);
-      }
-    };
-  }, [model, props.layerTree]);
-
   // Update layerTree when prop changes
   useEffect(() => {
     if (props.layerTree) {

packages/base/src/panelview/components/layers.tsx

@@ -19,10 +19,6 @@ export function StoryEditorPanel({ model, togglePreview }: IStoryPanelProps) {
   }, [model, model.sharedModel.storiesMap]);
 
   const syncStoryData = (properties: IDict) => {
-    if (!landmarkId) {
-      return;
-    }
-
     const { title, storyType, landmarks } = properties;
     const updatedStory: IJGISStoryMap = { title, storyType, landmarks };
 

packages/base/src/panelview/components/story-maps/StoryEditorPanel.tsx

@@ -15,7 +15,7 @@ interface IStoryViewerPanelProps {
 }
 
 function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
-  const [currentRankDisplayed, setCurrentRankDisplayed] = useState(0);
+  const [currentIndexDisplayed, setCurrentIndexDisplayed] = useState(0);
   const [storyData, setStoryData] = useState<IJGISStoryMap | null>(null);
 
   // Derive landmarks from story data
@@ -29,10 +29,10 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
       .filter((layer): layer is IJGISLayer => layer !== undefined);
   }, [storyData, model]);
 
-  // Derive current landmark from landmarks and currentRankDisplayed
+  // Derive current landmark from landmarks and currentIndexDisplayed
   const currentLandmark = useMemo(() => {
-    return landmarks[currentRankDisplayed];
-  }, [landmarks, currentRankDisplayed]);
+    return landmarks[currentIndexDisplayed];
+  }, [landmarks, currentIndexDisplayed]);
 
   // Derive active slide and layer name from current landmark
   const activeSlide = useMemo(() => {
@@ -45,15 +45,15 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
 
   // Derive landmark ID for zooming
   const currentLandmarkId = useMemo(() => {
-    return storyData?.landmarks?.[currentRankDisplayed];
-  }, [storyData, currentRankDisplayed]);
+    return storyData?.landmarks?.[currentIndexDisplayed];
+  }, [storyData, currentIndexDisplayed]);
 
   useEffect(() => {
     const updateStory = () => {
       const { story } = model.getSelectedStory();
       setStoryData(story ?? null);
       // Reset to first slide when story changes
-      setCurrentRankDisplayed(0);
+      setCurrentIndexDisplayed(0);
     };
 
     updateStory();
@@ -73,11 +73,11 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
   }, [currentLandmarkId, model]);
 
   // Listen for layer selection changes in unguided mode
-  // ! TODO refactor selection stuff
   useEffect(() => {
-    const handleAwarenessChange = (thig: any, more: any, extra: any) => {
+    // ! TODO this logic (getting a single selected layer) is also in the processing index.ts, move to tools
+    const handleSelectedLandmarkChange = (thig: any, more: any, extra: any) => {
       // This is just to update the displayed content
-      // So bail early if we don't need to do thath
+      // So bail early if we don't need to do that
       if (!storyData || storyData.storyType !== 'unguided') {
         return;
       }
@@ -105,13 +105,13 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
         return;
       }
 
-      setCurrentRankDisplayed(index);
+      setCurrentIndexDisplayed(index);
     };
 
-    model.sharedModel.awareness.on('change', handleAwarenessChange);
+    model.sharedModel.awareness.on('change', handleSelectedLandmarkChange);
 
     return () => {
-      model.sharedModel.awareness.off('change', handleAwarenessChange);
+      model.sharedModel.awareness.off('change', handleSelectedLandmarkChange);
     };
   }, [model, storyData]);
 
@@ -122,14 +122,14 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
   };
 
   const handlePrev = () => {
-    if (currentRankDisplayed > 0) {
-      setCurrentRankDisplayed(currentRankDisplayed - 1);
+    if (currentIndexDisplayed > 0) {
+      setCurrentIndexDisplayed(currentIndexDisplayed - 1);
     }
   };
 
   const handleNext = () => {
-    if (currentRankDisplayed < landmarks.length - 1) {
-      setCurrentRankDisplayed(currentRankDisplayed + 1);
+    if (currentIndexDisplayed < landmarks.length - 1) {
+      setCurrentIndexDisplayed(currentIndexDisplayed + 1);
     }
   };
 
@@ -175,7 +175,7 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
               textAlign: 'center',
             }}
           >
-            {`Slide ${currentRankDisplayed + 1} - ${layerName ? layerName : 'Landmark Name'}`}
+            {`Slide ${currentIndexDisplayed + 1} - ${layerName ? layerName : 'Landmark Name'}`}
           </h1>
         </div>
       ) : (
@@ -186,11 +186,6 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
           ? activeSlide.content.title
           : 'Slide Title'}
       </h2>
-      {/* <h3 style={{ paddingLeft: 2 }}>
-        {activeSlide?.content?.title
-          ? activeSlide.content.title
-          : 'Slide Title'}
-      </h3> */}
       {activeSlide?.content?.markdown && (
         <div className="jgis-story-viewer-content" style={{ paddingLeft: 16 }}>
           <Markdown>{activeSlide.content.markdown}</Markdown>
@@ -201,8 +196,8 @@ function StoryViewerPanel({ model, togglePreview }: IStoryViewerPanelProps) {
         <StoryNavBar
           onPrev={handlePrev}
           onNext={handleNext}
-          hasPrev={currentRankDisplayed > 0}
-          hasNext={currentRankDisplayed < landmarks.length - 1}
+          hasPrev={currentIndexDisplayed > 0}
+          hasNext={currentIndexDisplayed < landmarks.length - 1}
         />
       )}
       <Button onClick={togglePreview}>Edit Story</Button>

packages/base/src/panelview/components/story-maps/StoryViewerPanel.tsx

@@ -59,7 +59,8 @@ export const LeftPanel: React.FC<ILeftPanelProps> = (
     };
   }, [props.model]);
 
-  // Process layer tree once to create both filtered and landmark trees
+  // Since landmarks are technically layers they are stored in the layer tree, so we separate them
+  // from regular layers. Process the tree once to build both filtered and landmark trees.
   const { filteredLayerTree, landmarkLayerTree } = React.useMemo(() => {
     const filtered: IJGISLayerTree = [];
     const landmarks: IJGISLayerTree = [];
@@ -121,11 +122,6 @@ export const LeftPanel: React.FC<ILeftPanelProps> = (
     };
   }, [layerTree]);
 
-  /**
-   * ! TODO LOOK HERE
-   * If we want to support multiple stories then we need a way to assign landmarks to them
-   * Probably just wont do multiple stories then
-   */
   // Updates landmarks array based on layer tree array
   React.useEffect(() => {
     const { landmarkId, story } = props.model.getSelectedStory();

packages/base/src/panelview/leftpanel.tsx

@@ -1,7 +1,6 @@
 .jgis-panel-tabs {
   display: flex;
   flex-direction: column;
-  /* gap: 1rem; */
   justify-content: center;
   align-items: center;
   background-color: var(--jp-layout-color0);

packages/base/style/shared/tabs.css

@@ -300,10 +300,6 @@ export class JupyterGISDoc
     this.transact(() => this._sources.set(id, value));
   }
 
-  storyMapExists(id: string): boolean {
-    return Boolean(this._getStoryMapAsYMap(id));
-  }
-
   removeStoryMap(id: string): void {
     this.transact(() => {
       this._storiesMap.delete(id);
@@ -389,13 +385,6 @@ export class JupyterGISDoc
     return undefined;
   }
 
-  private _getStoryMapAsYMap(id: string): Y.Map<any> | undefined {
-    if (this._storiesMap.has(id)) {
-      return this._storiesMap.get(id);
-    }
-    return undefined;
-  }
-
   private _layersObserver(events: Y.YEvent<any>[]): void {
     const changes: Array<{
       id: string;

packages/schema/src/doc.ts

@@ -132,7 +132,6 @@ export interface IJupyterGISDoc extends YDocument<IJupyterGISDocChange> {
   addSource(id: string, value: IJGISSource): void;
   updateSource(id: string, value: IJGISSource): void;
 
-  storyMapExists(id: string): boolean;
   getStoryMap(id: string): IJGISStoryMap | undefined;
   removeStoryMap(id: string): void;
   addStoryMap(id: string, value: IJGISStoryMap): void;