Cleanup: console logging, debug and testing output, lint. (#486)

* Cleanup console logging in prod: turn off debugging, fix some undefined refs, etc..

* Cleanup lint.

* Cleanup debug and testing output.

* Use highlight colors from theme for pick selections.

* Revert some of the color changes.

package.json

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

src/BaseRoutes.test.jsx

@@ -2,10 +2,8 @@ import React from 'react'
 import {render} from '@testing-library/react'
 import MockRoutes from './BaseRoutesMock.test'
 import BaseRoutes from './BaseRoutes'
-import {setDebugLevel} from './utils/debug'
 
 
-setDebugLevel(0)
 /**
  * TODO(pablo): fix flaky test
  * RangeError: /Users/olegmoshkovich/Desktop/builders/Share/node_modules/web-ifc/web-ifc-api.js:

src/Components/BranchesControl.jsx

@@ -56,7 +56,7 @@ export default function Branches() {
     if (branches.length === 0 && modelPath.repo !== undefined) {
       fetchBranches()
     }
-  }, [repository])
+  }, [repository, branches.length, modelPath.branch, modelPath.filepath, modelPath.org, modelPath.repo])
 
 
   const handleSelect = (event) => {

src/Components/Buttons.test.jsx

@@ -7,12 +7,6 @@ import {MockComponent} from '../__mocks__/MockComponent'
 // When this test is ran an error is thrown by the assert as expected therefore the test is passing,
 // but the error is printed on the screen making it look like something is wrong.
 describe('<TooltipIconButton />', () => {
-  test('should throw error if missing required props', () => {
-    expect(() => render(<MockComponent>
-      <TooltipIconButton/>
-    </MockComponent>)).toThrowError('Arg 0 is not defined')
-  })
-
   test('should render successfully', async () => {
     /* eslint-disable no-empty-function */
     const rendered = render(<MockComponent>

src/Components/ExtractLevelsMenu.jsx

@@ -1,4 +1,5 @@
-import React, {useState, useEffect} from 'react'
+import React, {useEffect, useState} from 'react'
+import {useLocation} from 'react-router-dom'
 import Menu from '@mui/material/Menu'
 import MenuItem from '@mui/material/MenuItem'
 import {TooltipIconButton} from './Buttons'
@@ -24,6 +25,7 @@ export default function ExtractLevelsMenu({listOfOptions, icon, title}) {
   const [anchorEl, setAnchorEl] = useState(null)
   const [allLevelsState, setAllLevelsState] = useState([])
   const model = useStore((state) => state.modelStore)
+  const location = useLocation()
   const levelInstance = useStore((state) => state.levelInstance)
   const setLevelInstance = useStore((state) => state.setLevelInstance)
   const setCutPlaneDirection = useStore((state) => state.setCutPlaneDirection)
@@ -36,23 +38,26 @@ export default function ExtractLevelsMenu({listOfOptions, icon, title}) {
   const ceilingOffset = 0.4
 
   useEffect(() => {
-    const planeHash = getHashParams(location, 'p')
-    const fetchFloors = async () => {
-      const allLevels = await extractHeight(model)
-      setAllLevelsState(allLevels)
-      if (planeHash && model && viewer ) {
-        const levelHash = planeHash.split(':')[1]
-        if (isNumeric(levelHash)) {
-          const level = parseInt(levelHash)
-          createFloorplanPlane(allLevels[level] + floorOffset, allLevels[level + 1] - ceilingOffset, level)
+    // TODO(pablo): need to test getAllItemsOfType since it's null in
+    // our mock.  Don't know how to mock the async function correctly.
+    if (model && model.getAllItemsOfType) {
+      const planeHash = getHashParams(location, 'p')
+      const fetchFloors = async () => {
+        const allLevels = await extractHeight(model)
+        setAllLevelsState(allLevels)
+        if (planeHash && model && viewer) {
+          const levelHash = planeHash.split(':')[1]
+          if (isNumeric(levelHash)) {
+            const level = parseInt(levelHash)
+            createFloorplanPlane(allLevels[level] + floorOffset, allLevels[level + 1] - ceilingOffset, level)
+          }
         }
       }
+      fetchFloors()
     }
-    fetchFloors()
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [model])
 
-
   const createFloorplanPlane = (planeHeightBottom, planeHeightTop, level) => {
     removePlanes(viewer)
     setCutPlaneDirection(null)

src/Components/SideDrawer.jsx

@@ -1,7 +1,7 @@
 import React, {useEffect} from 'react'
 import {useLocation} from 'react-router-dom'
 import Drawer from '@mui/material/Drawer'
-import {makeStyles} from '@mui/styles'
+import {makeStyles, useTheme} from '@mui/styles'
 import useStore from '../store/useStore'
 import {getHashParams} from '../utils/location'
 import {preprocessMediaQuery} from '../utils/mediaQuery'
@@ -29,6 +29,7 @@ export function SideDrawer({
     isPropertiesOn: isPropertiesOn,
   })
   const isMobile = useIsMobile()
+  const theme = useTheme()
 
 
   useEffect(() => {
@@ -61,7 +62,15 @@ export function SideDrawer({
           className={classes.drawer}
         >
           <div className={classes.content}>
-            <div className={classes.containerNotes}>
+            <div
+              sx={{
+                overflow: 'hidden',
+                height: isPropertiesOn ? '50%' : '1200px',
+                display: isCommentsOn ? '' : 'none',
+                borderRadius: '0px',
+                borderBottom: `1px solid ${theme.palette.highlight.heaviest}`,
+              }}
+            >
               {isCommentsOn ? <NotesPanel/> : null}
             </div>
             <div className={classes.divider}/>
@@ -176,13 +185,6 @@ const useStyles = makeStyles((theme, props) => (preprocessMediaQuery(MOBILE_WIDT
     borderRadius: '5px',
     overflow: 'hidden',
   },
-  containerNotes: {
-    overflow: 'hidden ',
-    height: (p) => p.isPropertiesOn ? '50%' : '1200px',
-    display: (p) => p.isCommentsOn ? '' : 'none',
-    borderRadius: '0px',
-    borderBottom: `1px solid ${theme.palette.highlight.heaviest}`,
-  },
   containerProperties: {
     borderRadius: '5px',
     overflow: 'hidden',

src/Containers/CadView.jsx

@@ -151,7 +151,7 @@ export default function CadView({
     const preselectMat = new MeshLambertMaterial({
       transparent: true,
       opacity: 0.5,
-      color: theme.palette.highlight.light,
+      color: theme.palette.highlight.secondary,
       depthTest: true,
     })
     const selectMat = new MeshLambertMaterial({
@@ -393,7 +393,7 @@ export default function CadView({
   async function onElementSelect(expressId) {
     const lookupElt = elementsById[parseInt(expressId)]
     if (!lookupElt) {
-      console.error(`CadView#onElementSelect(${expressId}) missing in table:`, elementsById)
+      debug().error(`CadView#onElementSelect(${expressId}) missing in table:`, elementsById)
       return
     }
     await selectItemsInScene([expressId])

src/Share.jsx

@@ -59,15 +59,14 @@ export default function Share({installPrefix, appPrefix, pathPrefix}) {
     // TODO(pablo): currently expect these to both be defined.
     const {org, repo} = urlParams
     if (org && repo) {
-      console.log(`Setting GH repo ${org}/${repo}`)
       setRepository(org, repo)
     } else if (pathPrefix.startsWith('/share/v/p')) {
       debug().log('Setting default repo pablo-mayrgundter/Share')
       setRepository('pablo-mayrgundter', 'Share')
     } else {
       console.warn('No repository set for project!', pathPrefix)
     }
-  }, [appPrefix, installPrefix, modelPath, pathPrefix, setRepository, urlParams])
+  }, [appPrefix, installPrefix, modelPath, pathPrefix, setRepository, urlParams, setModelPath])
 
 
   const {theme, colorMode} = useTheme()

src/Theme.jsx

@@ -95,7 +95,7 @@ function loadTheme(mode) {
     },
     highlight: {
       main: darkGreen,
-      secondary: lightGreen,
+      secondary: darkGreen,
       heavy: grey[700],
       heavier: grey[600],
       heaviest: grey[500],

src/utils/debug.js

@@ -1,8 +1,10 @@
-const VERBOSE = 3
-// eslint-disable-next-line no-unused-vars
-const DEBUG = 2
+/* eslint-disable no-unused-vars */
+const OFF = 4
+const ERROR = 3
+const WARN = 2
 const INFO = 1
-const OFF = 0
+const DEBUG = 0
+/* eslint-enable no-unused-vars */
 let DEBUG_LEVEL = OFF
 
 
@@ -13,16 +15,16 @@ let DEBUG_LEVEL = OFF
  * @return {Function} returned function is console.log or a no-op if debugging is turned off
  */
 export default function debug(level = INFO) {
-  return level <= DEBUG_LEVEL ? console : mockLog
+  return level >= DEBUG_LEVEL ? console : mockLog
 }
 
 
 /**
- * @param {number} level One of OFF, INFO, DEBUG, VERBOSE.
+ * @param {number} level One of OFF, INFO, DEBUG, ALL.
  */
 export function setDebugLevel(level) {
-  if (!Number.isFinite(level) || level < OFF || level > VERBOSE) {
-    throw new Error(`Debug level must be a number from 0-${VERBOSE}`)
+  if (!Number.isFinite(level) || level < DEBUG || level > OFF) {
+    throw new Error(`Debug level must be a number from ${DEBUG}-${OFF}`)
   }
   DEBUG_LEVEL = level
 }
@@ -42,6 +44,7 @@ const mockLog = {
   /* eslint-disable no-empty-function */
   log: () => {},
   warn: () => {},
+  error: () => {},
   time: () => {},
   timeEnd: () => {},
 }

src/utils/extractHeight.js

@@ -5,24 +5,12 @@
  * @return {Array} elevation values
  */
 export async function extractHeight(ifcModel) {
-  try {
-    const ifcBuildingStoreyID = 3124254112
-    const ifcBuildingStorey = ifcModel.getAllItemsOfType(ifcBuildingStoreyID, true)
+  const ifcBuildingStoreyID = 3124254112
+  const storeys = await ifcModel.getAllItemsOfType(ifcBuildingStoreyID, true)
 
-    const printStorey = async () => {
-      const allStor = await ifcBuildingStorey
-      const elevValues = []
-      for (let i = 0; i < allStor.length; i++) {
-        elevValues[i] = allStor[i].Elevation.value
-      }
-      return elevValues
-    }
-    const elevValues = []
-    for (let i = 0; i < ifcBuildingStorey.length; i++) {
-      elevValues[i] = ifcBuildingStorey[i].Elevation.value
-    }
-    return await printStorey()
-  } catch {
-    console.log('No Levels detected')
+  const elevValues = []
+  for (let i = 0; i < storeys.length; i++) {
+    elevValues[i] = storeys[i].Elevation.value
   }
+  return elevValues
 }