Merge pull request #2284 from eriese/eriese-fix-memo

fix measurement gist issues with better state management  #2281

sites/shared/components/workbench/inputs/measurement.js

@@ -11,25 +11,26 @@ import measurementAsMm from '@freesewing/utils/measurementAsMm'
  * m holds the measurement name. It's just so long to type
  * measurement and I always have some typo in it because dyslexia.
  */
-const MeasurementInput = ({ m, gist, app, updateMeasurements }) => {
+const MeasurementInput = ({ m, gist, app, updateMeasurements, focus }) => {
   const { t } = useTranslation(['app', 'measurements'])
   const prefix = (app.site === 'org') ? '' : 'https://freesewing.org'
   const title = t(`measurements:${m}`)
 
   const isDegree = isDegreeMeasurement(m);
-  const factor = useMemo(() => (isDegree ? 1 : (gist?.units == 'imperial' ? 25.4 : 10)), [gist?.units])
+  const factor = useMemo(() => (isDegree ? 1 : (gist.units == 'imperial' ? 25.4 : 10)), [gist.units])
 
   const isValValid = val => (typeof val === 'undefined' || val === '')
       ? null
-      : val !== false && !isNaN(val)
+      : val != false && !isNaN(val)
   const isValid = (newVal) => (typeof newVal === 'undefined')
     ? isValValid(val)
     : isValValid(newVal)
 
-  const [val, setVal] = useState(gist?.measurements?.[m] / factor || '')
+  const [val, setVal] = useState(gist.measurements?.[m] / factor || '')
 
   // keep a single reference to a debounce timer
   const debounceTimeout = useRef(null);
+  const input = useRef(null);
 
   // onChange
   const update = useCallback((evt) => {
@@ -38,7 +39,7 @@ const MeasurementInput = ({ m, gist, app, updateMeasurements }) => {
     // set Val immediately so that the input reflects it
     setVal(evtVal)
 
-    let useVal = isDegree ? evtVal : measurementAsMm(evtVal, gist?.units);
+    let useVal = isDegree ? evtVal : measurementAsMm(evtVal, gist.units);
     const ok = isValid(useVal)
     // only set to the gist if it's valid
     if (ok) {
@@ -50,14 +51,14 @@ const MeasurementInput = ({ m, gist, app, updateMeasurements }) => {
         updateMeasurements(useVal, m)
       }, 500);
     }
-  }, [gist?.units])
+  }, [gist.units])
 
   // use this for better update efficiency
   // FIXME: This breaks gist updates.
   // See: https://github.com/freesewing/freesewing/issues/2281
-  const memoVal = gist?.measurements?.[m] //useMemo(() => gist?.measurements?.[m], [gist])
+  const memoVal = useMemo(() => gist.measurements?.[m], [gist])
   // track validity against the value and the units
-  const valid = useMemo(() => isValid(isDegree ? val : measurementAsMm(val, gist?.units)), [val, gist?.units])
+  const valid = useMemo(() => isValid(isDegree ? val : measurementAsMm(val, gist.units)), [val, gist.units])
 
   // hook to update the value or format when the gist changes
   useEffect(() => {
@@ -68,8 +69,17 @@ const MeasurementInput = ({ m, gist, app, updateMeasurements }) => {
       }
   }, [memoVal, factor])
 
+  // focus when prompted by parent
+  useEffect(() => {
+    if (focus) {
+      input.current.focus();
+    }
+  }, [focus])
+
   // cleanup
-  useEffect(() => clearTimeout(debounceTimeout.current), [])
+  useEffect(() => {
+    clearTimeout(debounceTimeout.current)
+  }, [])
 
   if (!m) return null
 
@@ -89,6 +99,7 @@ const MeasurementInput = ({ m, gist, app, updateMeasurements }) => {
       <label className="input-group input-group-lg">
         <input
           key={`input-${m}`}
+          ref={input}
           type="text"
           placeholder={title}
           className={`

sites/shared/components/workbench/measurements/index.js

@@ -1,4 +1,4 @@
-import React from 'react'
+import React, {useMemo, useEffect, useState} from 'react'
 import MeasurementInput from '../inputs/measurement.js'
 import { withBreasts, withoutBreasts } from '@freesewing/models'
 import nonHuman from './non-human.js'
@@ -7,8 +7,6 @@ import WithoutBreastsIcon from 'shared/components/icons/without-breasts.js'
 import { useTranslation } from 'next-i18next'
 import Setting from '../menu/core-settings/setting';
 import {settings} from '../menu/core-settings/index';
-import Popout from 'shared/components/popout'
-import Code from 'shared/components/code'
 
 const groups = {
   people: {
@@ -29,7 +27,7 @@ const icons = {
   without: <WithoutBreastsIcon />,
 }
 
-const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => {
+const WorkbenchMeasurements = ({ app, design, gist, updateGist, gistReady }) => {
   const { t } = useTranslation(['app', 'cfp'])
 
   // Method to handle measurement updates
@@ -39,13 +37,26 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => {
       updateGist('measurements', value)
     } else {
       // Set one measurement
-      const newValues = {...gist.measurements}
-      newValues[m] = value
-      updateGist('measurements', newValues)
+      updateGist(['measurements', m], value)
     }
   }
+
+  const [firstInvalid, setFirstInvalid] = useState(undefined)
+
+  useEffect(() => {
+    if (!gistReady) { return }
+    for (const m of design.config?.measurements || []) {
+      if (!gist?.measurements?.[m]) {
+        setFirstInvalid(m);
+        return;
+      }
+
+      setFirstInvalid(undefined)
+    }
+  }, [gistReady])
+
   // Save us some typing
-  const inputProps = { app, updateMeasurements, gist }
+  const inputProps = useMemo(() => ({ app, updateMeasurements, gist }), [app, gist])
 
   return (
     <div className="m-auto max-w-2xl">
@@ -54,11 +65,6 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => {
           {design.config.name}:
         </span> {t('measurements')}
       </h1>
-      <Popout fixme>
-        <h5>Debug for issue <a href="https://github.com/freesewing/freesewing/issues/2281">#2281</a></h5>
-        <p>Current value of <Code>gist.measurements</Code></p>
-        <pre>{JSON.stringify(gist.measurements, null ,2)}</pre>
-      </Popout>
       <details open className="my-2">
         <summary><h2 className="inline pl-1">{t('cfp:preloadMeasurements')}</h2></summary>
         <div className="ml-2 pl-4 border-l-2">
@@ -94,7 +100,7 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => {
         </div>
       </details>
 
-      <details className="my-2">
+      <details open className="my-2">
         <summary><h2 className="inline pl-2">{t('cfp:enterMeasurements')}</h2></summary>
         <Setting key={'units'} setting={'units'} config={settings.units} updateGist={updateGist} {...inputProps} />
         <div className="ml-2 pl-4 border-l-2">
@@ -102,7 +108,7 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => {
             <>
               <h3>{t('requiredMeasurements')}</h3>
               {design.config.measurements.map(m => (
-                <MeasurementInput key={m} m={m} {...inputProps} />
+                <MeasurementInput key={m} m={m} focus={m == firstInvalid} {...inputProps} />
               ))}
             </>
           )}

sites/shared/components/workbench/preload.js

@@ -14,12 +14,15 @@ const preload = {
 
     if (result.data.files['pattern.yaml'].content) {
       let g = yaml.load(result.data.files['pattern.yaml'].content)
-      if (g.design !== pattern.config.name) return [
+
+      if (g.design !== undefined && g.design !== pattern.config.name) return [
         false, `You tried loading a configuration for ${g.design} into a ${pattern.config.name} development environment`
       ]
 
       return g
     }
+
+    // TODO notify people of these errors
     else return [false, 'This gist does not seem to be a valid pattern configuration']
   }
 }

sites/shared/components/wrappers/workbench.js

@@ -1,10 +1,7 @@
-import { useEffect, useState } from 'react'
-import useLocalStorage from 'shared/hooks/useLocalStorage.js'
+import { useEffect, useState, useMemo,} from 'react'
+import {useGist} from 'shared/hooks/useGist'
 import Layout from 'shared/components/layouts/default'
 import Menu from 'shared/components/workbench/menu/index.js'
-import set from 'lodash.set'
-import unset from 'lodash.unset'
-import defaultSettings from 'shared/components/workbench/default-settings.js'
 import DraftError from 'shared/components/workbench/draft/error.js'
 import theme from '@freesewing/plugin-theme'
 import preloaders from 'shared/components/workbench/preload.js'
@@ -34,18 +31,6 @@ const views = {
   welcome: () => <p>TODO</p>,
 }
 
-// Generates a default design gist to start from
-const defaultGist = (design, locale='en') => {
-  const gist = {
-  design: design.config.name,
-  version: design.config.version,
-  ...defaultSettings
-  }
-  if (locale) gist.locale = locale
-
-  return gist
-}
-
 const hasRequiredMeasurements = (design, gist) => {
   for (const m of design.config.measurements || []) {
     if (!gist?.measurements?.[m]) return false
@@ -62,7 +47,7 @@ const hasRequiredMeasurements = (design, gist) => {
 const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false }) => {
 
   // State for gist
-  const [gist, setGist, ready] = useLocalStorage(`${design.config.name}_gist`, defaultGist(design, app.locale))
+  const [gist, setGist, gistReady] = useGist(design, app);
   const [messages, setMessages] = useState([])
   const [popup, setPopup] = useState(false)
 
@@ -70,35 +55,33 @@ const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false
   // force view to measurements
   useEffect(() => {
     if (
-      ready && gist?._state?.view !== 'measurements'
+      gistReady && gist._state?.view !== 'measurements'
       && !hasRequiredMeasurements(design, gist)
     ) updateGist(['_state', 'view'], 'measurements')
-  }, [ready])
+  }, [gistReady, gist._state.view])
 
   // If we need to preload the gist, do so
   useEffect(() => {
     const doPreload = async () => {
       if (preload && from && preloaders[from]) {
         const g = await preloaders[from](preload, design)
-        setGist({ ...gist, ...g.settings })
+        setGist({value: { ...gist, ...g.settings }, type: 'replace'})
       }
     }
     doPreload();
   }, [preload, from])
 
   // Helper methods to manage the gist state
-  const updateGist = (path, content, closeNav=false) => {
-    const newGist = {...gist}
-    set(newGist, path, content)
-    setGist(newGist)
+  const updateGist = useMemo(() => (path, value, closeNav=false) => {
+    setGist({path, value})
     // Force close of menu on mobile if it is open
     if (closeNav && app.primaryMenu) app.setPrimaryMenu(false)
-  }
+  }, [app])
+
   const unsetGist = (path) => {
-    const newGist = {...gist}
-    unset(newGist, path)
-    setGist(newGist)
+    setGist({path, type: 'unset'})
   }
+
   // Helper methods to handle messages
   const feedback = {
     add: msg => {
@@ -113,7 +96,7 @@ const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false
 
   // Generate the draft here so we can pass it down
   let draft = false
-  if (['draft', 'events', 'test'].indexOf(gist?._state?.view) !== -1) {
+  if (['draft', 'events', 'test'].indexOf(gist._state?.view) !== -1) {
     draft = new design(gist)
     if (gist.renderer === 'svg') draft.use(theme)
     try {
@@ -126,7 +109,7 @@ const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false
   }
 
   // Props to pass down
-  const componentProps = { app, design, gist, updateGist, unsetGist, setGist, draft, feedback, showInfo: setPopup }
+  const componentProps = { app, design, gist, updateGist, unsetGist, setGist, draft, feedback, gistReady, showInfo: setPopup }
   // Required props for layout
   const layoutProps = {
     app: app,
@@ -141,7 +124,7 @@ const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false
     ? layout
     : Layout
 
-  const Component = views[gist?._state?.view]
+  const Component = views[gist._state?.view]
     ? views[gist._state.view]
     : views.welcome
 

sites/shared/hooks/useGist.js

@@ -0,0 +1,36 @@
+import useLocalStorage from './useLocalStorage';
+import set from 'lodash.set'
+import unset from 'lodash.unset'
+import defaultSettings from 'shared/components/workbench/default-settings.js'
+
+// Generates a default design gist to start from
+const defaultGist = (design, locale='en') => {
+  const gist = {
+  design: design.config.name,
+  version: design.config.version,
+  ...defaultSettings,
+  _state: {view: 'measurements'}
+  }
+  if (locale) gist.locale = locale
+
+  return gist
+}
+
+function reducer(gistState, {path, value, type='set'}) {
+	const newGist = {... gistState};
+
+	switch(type) {
+		case 'replace' :
+			return value;
+		case 'unset' :
+			unset(newGist, path);
+			break;
+		default:
+			set(newGist, path, value);
+	}
+	return newGist;
+}
+
+export function useGist(design, app) {
+	return useLocalStorage(`${design.config.name}_gist`, defaultGist(design, app.locale), reducer);
+}

sites/shared/hooks/useLocalStorage.js

@@ -1,36 +1,35 @@
-import { useState, useEffect, useRef } from 'react'
+import { useState, useEffect, useRef, useReducer } from 'react'
 
 // See: https://usehooks.com/useLocalStorage/
-function useLocalStorage(key, initialValue) {
+function useLocalStorage(key, initialValue, reducer) {
   const prefix = 'fs_'
-  const [storedValue, setStoredValue] = useState(initialValue);
+  const [storedValue, setStoredValue] = typeof reducer == 'function' ? useReducer(reducer, initialValue) : useState(initialValue);
   // use this to track whether it's mounted. useful for doing other effects outside this hook
   const [ready, setReady] = useState(false);
   const readyInternal = useRef(false);
+  const setValue = setStoredValue
 
-  const setValue = function (value) {
-    if (!readyInternal.current) {
-      return null
+  // set to localstorage every time the storedValue changes
+  useEffect(() => {
+    if (readyInternal.current) {
+      window.localStorage.setItem(prefix + key, JSON.stringify(storedValue))
     }
-
-    try {
-      const valueToStore = value instanceof Function ? value(storedValue) : value
-      setStoredValue(valueToStore)
-      window.localStorage.setItem(prefix + key, JSON.stringify(valueToStore))
-    } catch (error) {
-      console.log(error)
-    }
-  }
+  }, [storedValue])
 
   // get the item from localstorage after the component has mounted. empty brackets mean it runs one time
   useEffect(() => {
     readyInternal.current = true;
     const item = window.localStorage.getItem(prefix + key)
+    let valueToSet = storedValue;
     if (item) {
-      setValue(JSON.parse(item));
-    } else if (storedValue) {
-      setValue(storedValue)
+      valueToSet = JSON.parse(item)
     }
+
+    if (reducer) {
+      valueToSet = {value: valueToSet, type: 'replace'}
+    }
+
+    setValue(valueToSet)
     setReady(true);
   }, [])
 

sites/shared/hooks/useTheme.js

@@ -10,7 +10,7 @@ function useTheme() {
     if (ready && storedTheme === undefined) {
       const prefersDarkMode = (typeof window !== 'undefined' && typeof window.matchMedia === 'function')
         ? window.matchMedia(`(prefers-color-scheme: dark`).matches
-        : null
+        : undefined
 
       setStoredTheme(prefersDarkMode ? 'dark' : 'light')
     }