Simplify some plumbing for element selection. Fix small cameraControl…

… react warning. Improve Tree selection visibility in dark theme. Misc debugging.

__mocks__/web-ifc-viewer.js

@@ -4,7 +4,7 @@ const ifcjsMock = jest.createMockFromModule('web-ifc-viewer')
 // are not present in the instantiated IfcViewerAPI.
 const loadedModel = {
   ifcManager: {
-    getSpatialStructure: jest.fn(() => ({expressID: 0, children: []})),
+    getSpatialStructure: jest.fn(),
     getProperties: jest.fn((eltId) => ({})),
   },
   getIfcType: jest.fn(),

package.json

@@ -1,6 +1,6 @@
 {
   "name": "bldrs",
-  "version": "1.0.0-r402",
+  "version": "1.0.0-r403",
   "main": "src/index.jsx",
   "homepage": "https://github.com/bldrs-ai/Share",
   "bugs": {

src/Components/CameraControl.jsx

@@ -27,16 +27,16 @@ import {roundCoord} from '../utils/math'
  */
 export default function CameraControl({viewer}) {
   assertDefined(viewer, viewer.IFC, viewer.IFC.context, viewer.IFC.context.ifcCamera)
-  const setCameraControls = useStore((state) => state.setCameraControls)
   const cameraControls = viewer.IFC.context.ifcCamera.cameraControls
-  setCameraControls(cameraControls)
+  const setCameraControls = useStore((state) => state.setCameraControls)
   const location = useLocation()
 
 
   useEffect(() => {
+    setCameraControls(cameraControls)
     onHash(location, cameraControls)
     onLoad(location, cameraControls)
-  }, [location, cameraControls])
+  }, [location, cameraControls, setCameraControls])
 
 
   // NOTE: NOT DISPLAYED

src/Components/NavPanel.jsx

@@ -39,7 +39,6 @@ export function NavPanelControl({topOffset, onClickMenuCb}) {
  * @param {Array} selectedElements
  * @param {Array} defaultExpandedElements
  * @param {Array} expandedElements
- * @param {function} onElementSelect
  * @param {function} setExpandedElements
  * @param {string} pathPrefix
  * @return {Object}
@@ -50,7 +49,6 @@ export default function NavPanel({
   selectedElements,
   defaultExpandedElements,
   expandedElements,
-  onElementSelect,
   setExpandedElements,
   pathPrefix,
 }) {
@@ -80,8 +78,6 @@ export default function NavPanel({
               model={model}
               element={element}
               pathPrefix={pathPrefix}
-              onElementSelect={onElementSelect}
-              setExpandedElements={setExpandedElements}
             />
           }
         </TreeView>

src/Components/NavTree.jsx

@@ -45,16 +45,12 @@ const NavTreePropTypes = {
  * @param {Object} element IFC element of the model
  * @param {string} pathPrefix URL prefix for constructing links to
  *   elements, recursively grown as passed down the tree
- * @param {string} onElementSelect Callback when tree item element is selected
- * @param {string} setExpandedElements React state setter to update items to expand in tree
  * @return {Object} React component
  */
 export default function NavTree({
   model,
   element,
   pathPrefix,
-  onElementSelect,
-  setExpandedElements,
 }) {
   const CustomContent = React.forwardRef(function CustomContent(props, ref) {
     const {
@@ -94,7 +90,6 @@ export default function NavTree({
 
     useEffect(() => {
       if (selectedElement) {
-        onElementSelect(selectedElement)
         const newPath =
               `${pathPrefix}/${computeElementPathIds(element, (elt) => elt.expressID).join('/')}`
         navigate(newPath)
@@ -148,8 +143,6 @@ export default function NavTree({
                 model={model}
                 element={child}
                 pathPrefix={pathPrefix}
-                onElementSelect={onElementSelect}
-                setExpandedElements={setExpandedElements}
               />
             </React.Fragment>
           )

src/Components/SideDrawerPanels.jsx

@@ -35,22 +35,23 @@ export function PropertiesPanel() {
   const toggleIsPropertiesOn = useStore((state) => state.toggleIsPropertiesOn)
   const selectedElement = useStore((state) => state.selectedElement)
   const classes = useStyles(useTheme())
+  // TODO(pablo): this render was sometimes coming up with a react
+  // error where createElement is undefined.  I've refactored a little
+  // and now can't reproduce.
   return (
     <>
       <PanelTitle
         title='Properties'
         controlsGroup={
-          <div>
-            <TooltipIconButton
-              title='toggle drawer'
-              onClick={toggleIsPropertiesOn}
-              icon={<CloseIcon style={{width: '24px', height: '24px'}}/>}
-            />
-          </div>
+          <TooltipIconButton
+            title='Close'
+            onClick={toggleIsPropertiesOn}
+            icon={<CloseIcon style={{width: '24px', height: '24px'}}/>}
+          />
         }
       />
       <div className={classes.contentContainer}>
-        {selectedElement ? <ItemProperties/> : null}
+        {selectedElement && <ItemProperties/>}
       </div>
     </>
   )

src/Containers/CadView.jsx

@@ -1,5 +1,5 @@
 import React, {useContext, useEffect, useState} from 'react'
-import {useNavigate, useSearchParams} from 'react-router-dom'
+import {useLocation, useNavigate, useSearchParams} from 'react-router-dom'
 import {Color} from 'three'
 import {IfcViewerAPI} from 'web-ifc-viewer'
 import {makeStyles} from '@mui/styles'
@@ -53,7 +53,7 @@ export default function CadView({
   // IFC
   const [viewer, setViewer] = useState(null)
   const [rootElement, setRootElement] = useState({})
-  const elementsById = useState({})
+  const [elementsById] = useState({})
   const [defaultExpandedElements, setDefaultExpandedElements] = useState([])
   const [selectedElements, setSelectedElements] = useState([])
   const [expandedElements, setExpandedElements] = useState([])
@@ -99,6 +99,22 @@ export default function CadView({
   useEffect(() => {
     onSearchParams()
   }, [searchParams])
+
+
+  // Watch for path changes within the model.
+  // TODO(pablo): would be nice to have more consistent handling of path parsing.
+  const location = useLocation()
+  useEffect(() => {
+    if (model) {
+      (async () => {
+        const parts = location.pathname.split(/\.ifc/i)
+        const expectedPartCount = 2
+        if (parts.length === expectedPartCount) {
+          await selectElementBasedOnFilepath(parts[1])
+        }
+      })()
+    }
+  }, [location, model])
   /* eslint-enable */
 
 
@@ -244,14 +260,8 @@ export default function CadView({
     setupLookupAndParentLinks(rootElt, elementsById)
     setDoubleClickListener()
     initSearch(m, rootElt)
-    // The spatial structure doesn't contain properties.  NavTree uses
-    // the callback for onElementSelect in this class to fill out
-    // details for the rest of the tree items, so just root needs
-    // special handling.
-    // TODO(pablo): not sure if this is expected or a problem with the
-    // IFC API.  Could also try to get the initial root elt load to
-    // use shared logic with setSelectedElement.
     const rootProps = await viewer.getProperties(0, rootElt.expressID)
+    // console.log('setupLookupAndParentLinks', rootElt, elementsById, rootProps)
     rootElt.Name = rootProps.Name
     rootElt.LongName = rootProps.LongName
     setRootElement(rootElt)
@@ -289,7 +299,7 @@ export default function CadView({
         throw new Error('IllegalState: empty search query')
       }
       const resultIDs = searchIndex.search(query)
-      selectItems(resultIDs)
+      selectItemsInScene(resultIDs)
       setDefaultExpandedElements(resultIDs.map((id) => `${id }`))
       Privacy.recordEvent('search', {
         search_term: query,
@@ -309,27 +319,6 @@ export default function CadView({
   }
 
 
-  /**
-   * Select items in model when they are double-clicked.
-   */
-  async function setDoubleClickListener() {
-    window.ondblclick = async (event) => {
-      if (event.target && event.target.tagName === 'CANVAS') {
-        const item = await viewer.IFC.pickIfcItem(true)
-        if (item && Number.isFinite(item.modelID) && Number.isFinite(item.id)) {
-          const pathIds = computeElementPathIds(elementsById[item.id], (elt) => elt.expressID)
-          const repoFilePath = modelPath.gitpath ? modelPath.getRepoPath() : modelPath.filepath
-          const path = pathIds.join('/')
-          navigate(`${pathPrefix}${repoFilePath}/${path}`)
-          setSelectedElement(item)
-          setExpandedElements(pathIds.map((n) => `${n}`))
-          setSelectedElements(`${item.id}`)
-        }
-      }
-    }
-  }
-
-
   /** Unpick active scene elts and remove clip planes. */
   function unSelectItems() {
     setSelectedElement({})
@@ -342,7 +331,7 @@ export default function CadView({
    * Pick the given items in the scene.
    * @param {Array} resultIDs Array of expressIDs
    */
-  async function selectItems(resultIDs) {
+  async function selectItemsInScene(resultIDs) {
     setSelectedElements(resultIDs.map((id) => `${id}`))
     try {
       await viewer.pickIfcItemsByID(0, resultIDs, true)
@@ -356,32 +345,25 @@ export default function CadView({
 
 
   /**
-   * Select the items in the NavTree and update item properties for ItemPanel.
-   * @param {Object} elt The selected IFC element.
+   * Select the items in the NavTree and update ItemProperties.
+   * Returns the ids of path parts from root to this elt in spatial
+   * structure.
+   * @param {number} expressId
+   * @return {array} pathIds
    */
-  async function onElementSelect(elt) {
-    const id = elt.expressID
-    if (id === undefined) {
-      throw new Error('Selected element is missing Express ID')
+  async function onElementSelect(expressId) {
+    const lookupElt = elementsById[parseInt(expressId)]
+    if (!lookupElt) {
+      console.error(`CadView#onElementSelect(${expressId}) missing in table:`, elementsById)
+      return
     }
-    selectItems([id])
-    const props = await viewer.getProperties(0, elt.expressID)
+    selectItemsInScene([expressId])
+    const pathIds = computeElementPathIds(lookupElt, (elt) => elt.expressID)
+    setExpandedElements(pathIds.map((n) => `${n}`))
+    setSelectedElements(`${expressId}`)
+    const props = await viewer.getProperties(0, expressId)
     setSelectedElement(props)
-
-    // TODO(pablo): just found out this method is getting called a lot
-    // when i added navigation on select, which flooded the browser
-    // IPC.
-  }
-
-
-  const addThemeListener = () => {
-    colorModeContext.addThemeChangeListener((newMode, theme) => {
-      if (theme && theme.palette && theme.palette.background && theme.palette.background.paper) {
-        const intializedViewer = initViewer(pathPrefix, theme.palette.background.paper)
-        setViewer(intializedViewer)
-        setViewerStore(intializedViewer)
-      }
-    })
+    return pathIds
   }
 
 
@@ -395,12 +377,39 @@ export default function CadView({
       debug().log('CadView#selectElementBasedOnUrlPath: have path', parts)
       const targetId = parseInt(parts[parts.length - 1])
       if (isFinite(targetId)) {
-        onElementSelect({expressID: targetId})
-        setExpandedElements(parts)
+        onElementSelect(targetId)
+      }
+    }
+  }
+
+
+  /** Select items in model when they are double-clicked. */
+  async function setDoubleClickListener() {
+    window.ondblclick = async (event) => {
+      if (event.target && event.target.tagName === 'CANVAS') {
+        const item = await viewer.IFC.pickIfcItem(true)
+        if (item && Number.isFinite(item.modelID) && Number.isFinite(item.id)) {
+          const pathIds = await onElementSelect(item.id)
+          const repoFilePath = modelPath.gitpath ? modelPath.getRepoPath() : modelPath.filepath
+          const path = pathIds.join('/')
+          navigate(`${pathPrefix}${repoFilePath}/${path}`)
+        }
       }
     }
   }
 
+
+  const addThemeListener = () => {
+    colorModeContext.addThemeChangeListener((newMode, theme) => {
+      if (theme && theme.palette && theme.palette.background && theme.palette.background.paper) {
+        const intializedViewer = initViewer(pathPrefix, theme.palette.background.paper)
+        setViewer(intializedViewer)
+        setViewerStore(intializedViewer)
+      }
+    })
+  }
+
+
   return (
     <div className={classes.root}>
       <div className={classes.view} id='viewer-container'></div>
@@ -426,7 +435,6 @@ export default function CadView({
             selectedElements={selectedElements}
             defaultExpandedElements={defaultExpandedElements}
             expandedElements={expandedElements}
-            onElementSelect={onElementSelect}
             setExpandedElements={setExpandedElements}
             pathPrefix={
               pathPrefix + (modelPath.gitpath ? modelPath.getRepoPath() : modelPath.filepath)

src/Containers/CadView.test.jsx

@@ -3,6 +3,7 @@ import {render, renderHook, screen, waitFor} from '@testing-library/react'
 import CadView from './CadView'
 import ShareMock from '../ShareMock'
 import {actAsyncFlush} from '../utils/tests'
+import {makeTestTree} from '../utils/TreeUtils.test'
 import {__getIfcViewerAPIMockSingleton} from 'web-ifc-viewer'
 
 
@@ -16,6 +17,8 @@ describe('CadView', () => {
     const modelPath = {
       filepath: `index.ifc`,
     }
+    const viewer = __getIfcViewerAPIMockSingleton()
+    viewer._loadedModel.ifcManager.getSpatialStructure.mockReturnValueOnce(makeTestTree())
     const {result} = renderHook(() => useState(modelPath))
     render(
         <ShareMock>
@@ -34,11 +37,14 @@ describe('CadView', () => {
 
 
   it('renders and selects the element ID from URL', async () => {
-    const eltId = 1234
+    const testTree = makeTestTree()
+    const targetEltId = testTree.children[0].expressID
     const modelPath = {
-      filepath: `index.ifc/${eltId}`,
+      filepath: `index.ifc/${targetEltId}`,
       gitpath: undefined,
     }
+    const viewer = __getIfcViewerAPIMockSingleton()
+    viewer._loadedModel.ifcManager.getSpatialStructure.mockReturnValueOnce(testTree)
     const {result} = renderHook(() => useState(modelPath))
     render(
         <ShareMock>
@@ -51,14 +57,13 @@ describe('CadView', () => {
         </ShareMock>)
     await waitFor(() => screen.getByTitle(/Bldrs: 1.0.0/i))
     await actAsyncFlush()
-    const viewer = __getIfcViewerAPIMockSingleton()
     const getPropsCalls = viewer.getProperties.mock.calls
     const numCallsExpected = 2 // First for root, second from URL path
     expect(getPropsCalls.length).toBe(numCallsExpected)
     expect(getPropsCalls[0][0]).toBe(0) // call 1, arg 1
     expect(getPropsCalls[0][0]).toBe(0) // call 2, arg 2
     expect(getPropsCalls[1][0]).toBe(0) // call 2, arg 1
-    expect(getPropsCalls[1][1]).toBe(eltId) // call 2, arg 2
+    expect(getPropsCalls[1][1]).toBe(targetEltId) // call 2, arg 2
     await actAsyncFlush()
   })
 })

src/Share.jsx

@@ -45,7 +45,7 @@ export default function Share({installPrefix, appPrefix, pathPrefix}) {
       console.log(`Setting GH repo ${org}/${repo}`)
       setRepository(org, repo)
     } else if (pathPrefix.startsWith('/share/v/p')) {
-      console.log('Setting default repo pablo-mayrgundter/Share')
+      debug().log('Setting default repo pablo-mayrgundter/Share')
       setRepository('pablo-mayrgundter', 'Share')
     } else {
       console.warn('No repository set for project!', pathPrefix)

src/Theme.jsx

@@ -121,7 +121,7 @@ function loadTheme(mode) {
     MuiTreeItem: {
       styleOverrides: {
         root: {
-          '& > div.Mui-selected': {
+          '& > div.Mui-selected, & > div.Mui-selected:hover': {
             color: activePalette.secondary.main,
             backgroundColor: activePalette.secondary.background,
           },