From 347554f3a17ed292befd8ab5d65c12e9b20bd11c Mon Sep 17 00:00:00 2001 From: Enoch Riese Date: Wed, 22 Jun 2022 15:19:07 -0500 Subject: [PATCH 1/4] fix measurement gist issues with better state management --- .../workbench/inputs/measurement.js | 14 +++---- .../workbench/measurements/index.js | 16 ++------ sites/shared/components/wrappers/workbench.js | 39 +++++++------------ sites/shared/hooks/useGist.js | 36 +++++++++++++++++ sites/shared/hooks/useLocalStorage.js | 35 ++++++++--------- sites/shared/hooks/useTheme.js | 2 +- 6 files changed, 78 insertions(+), 64 deletions(-) create mode 100644 sites/shared/hooks/useGist.js diff --git a/sites/shared/components/workbench/inputs/measurement.js b/sites/shared/components/workbench/inputs/measurement.js index fab4f43422e..4f3ab9fc74d 100644 --- a/sites/shared/components/workbench/inputs/measurement.js +++ b/sites/shared/components/workbench/inputs/measurement.js @@ -17,16 +17,16 @@ const MeasurementInput = ({ m, gist, app, updateMeasurements }) => { 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); @@ -38,7 +38,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 +50,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(() => { diff --git a/sites/shared/components/workbench/measurements/index.js b/sites/shared/components/workbench/measurements/index.js index a187448401d..3a3ee7e2693 100644 --- a/sites/shared/components/workbench/measurements/index.js +++ b/sites/shared/components/workbench/measurements/index.js @@ -1,4 +1,4 @@ -import React from 'react' +import React, {useMemo} 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: { @@ -39,13 +37,12 @@ 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) } } + // Save us some typing - const inputProps = { app, updateMeasurements, gist } + const inputProps = useMemo(() => ({ app, updateMeasurements, gist }), [app, updateMeasurements, gist]) return (
@@ -54,11 +51,6 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => { {design.config.name}: {t('measurements')} - -
Debug for issue #2281
-

Current value of gist.measurements

-
{JSON.stringify(gist.measurements, null ,2)}
-

{t('cfp:preloadMeasurements')}

diff --git a/sites/shared/components/wrappers/workbench.js b/sites/shared/components/wrappers/workbench.js index 4732ed472bc..7598ca7c32f 100644 --- a/sites/shared/components/wrappers/workbench.js +++ b/sites/shared/components/wrappers/workbench.js @@ -1,5 +1,6 @@ -import { useEffect, useState } from 'react' -import useLocalStorage from 'shared/hooks/useLocalStorage.js' +import { useEffect, useState, useMemo,} from 'react' +import useLocalStorage from 'shared/hooks/useLocalStorage' +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' @@ -34,18 +35,6 @@ const views = { welcome: () =>

TODO

, } -// 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 +51,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, ready] = useGist(design, app); const [messages, setMessages] = useState([]) const [popup, setPopup] = useState(false) @@ -70,35 +59,33 @@ const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false // force view to measurements useEffect(() => { if ( - ready && gist?._state?.view !== 'measurements' + ready && gist._state?.view !== 'measurements' && !hasRequiredMeasurements(design, gist) ) updateGist(['_state', 'view'], 'measurements') - }, [ready]) + }, [ready, 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 => { diff --git a/sites/shared/hooks/useGist.js b/sites/shared/hooks/useGist.js new file mode 100644 index 00000000000..a6546feb885 --- /dev/null +++ b/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); +} diff --git a/sites/shared/hooks/useLocalStorage.js b/sites/shared/hooks/useLocalStorage.js index 3310d9f1778..a46926c9fc0 100644 --- a/sites/shared/hooks/useLocalStorage.js +++ b/sites/shared/hooks/useLocalStorage.js @@ -1,36 +1,35 @@ -import { useState, useEffect, useRef } from 'react' +import { useState, useEffect, useRef, useCallback, 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); }, []) diff --git a/sites/shared/hooks/useTheme.js b/sites/shared/hooks/useTheme.js index e3dff9f135e..a515d14cd50 100644 --- a/sites/shared/hooks/useTheme.js +++ b/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') } From 018b5372fead134edfa44d84123ccc0cf001e59d Mon Sep 17 00:00:00 2001 From: Enoch Riese Date: Wed, 22 Jun 2022 15:38:15 -0500 Subject: [PATCH 2/4] fix deepscan issues --- sites/shared/components/workbench/measurements/index.js | 2 +- sites/shared/components/wrappers/workbench.js | 8 ++------ sites/shared/hooks/useLocalStorage.js | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/sites/shared/components/workbench/measurements/index.js b/sites/shared/components/workbench/measurements/index.js index 3a3ee7e2693..393cbd8de39 100644 --- a/sites/shared/components/workbench/measurements/index.js +++ b/sites/shared/components/workbench/measurements/index.js @@ -42,7 +42,7 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => { } // Save us some typing - const inputProps = useMemo(() => ({ app, updateMeasurements, gist }), [app, updateMeasurements, gist]) + const inputProps = useMemo(() => ({ app, updateMeasurements, gist }), [app, gist]) return (
diff --git a/sites/shared/components/wrappers/workbench.js b/sites/shared/components/wrappers/workbench.js index 7598ca7c32f..2edbc06891c 100644 --- a/sites/shared/components/wrappers/workbench.js +++ b/sites/shared/components/wrappers/workbench.js @@ -1,11 +1,7 @@ import { useEffect, useState, useMemo,} from 'react' -import useLocalStorage from 'shared/hooks/useLocalStorage' 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' @@ -100,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 { @@ -128,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 diff --git a/sites/shared/hooks/useLocalStorage.js b/sites/shared/hooks/useLocalStorage.js index a46926c9fc0..793df99fbb3 100644 --- a/sites/shared/hooks/useLocalStorage.js +++ b/sites/shared/hooks/useLocalStorage.js @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef, useCallback, useReducer } from 'react' +import { useState, useEffect, useRef, useReducer } from 'react' // See: https://usehooks.com/useLocalStorage/ function useLocalStorage(key, initialValue, reducer) { From ce23291fc5d7798ae93f0096bddba63b6bbf360a Mon Sep 17 00:00:00 2001 From: Enoch Riese Date: Wed, 22 Jun 2022 19:13:40 -0500 Subject: [PATCH 3/4] focus the first invalid measurement on page load and when trying to leave --- .../workbench/inputs/measurement.js | 15 +++++++++++-- .../workbench/measurements/index.js | 22 +++++++++++++++---- sites/shared/components/wrappers/workbench.js | 8 +++---- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/sites/shared/components/workbench/inputs/measurement.js b/sites/shared/components/workbench/inputs/measurement.js index 4f3ab9fc74d..f9b667e702d 100644 --- a/sites/shared/components/workbench/inputs/measurement.js +++ b/sites/shared/components/workbench/inputs/measurement.js @@ -11,7 +11,7 @@ import measurementAsMm from 'pkgs/utils/src/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}`) @@ -30,6 +30,7 @@ const MeasurementInput = ({ m, gist, app, updateMeasurements }) => { // keep a single reference to a debounce timer const debounceTimeout = useRef(null); + const input = useRef(null); // onChange const update = useCallback((evt) => { @@ -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 }) => {
-
+

{t('cfp:enterMeasurements')}

@@ -94,7 +108,7 @@ const WorkbenchMeasurements = ({ app, design, gist, updateGist }) => { <>

{t('requiredMeasurements')}

{design.config.measurements.map(m => ( - + ))} )} diff --git a/sites/shared/components/wrappers/workbench.js b/sites/shared/components/wrappers/workbench.js index 2edbc06891c..1f4fb29f196 100644 --- a/sites/shared/components/wrappers/workbench.js +++ b/sites/shared/components/wrappers/workbench.js @@ -47,7 +47,7 @@ const hasRequiredMeasurements = (design, gist) => { const WorkbenchWrapper = ({ app, design, preload=false, from=false, layout=false }) => { // State for gist - const [gist, setGist, ready] = useGist(design, app); + const [gist, setGist, gistReady] = useGist(design, app); const [messages, setMessages] = useState([]) const [popup, setPopup] = useState(false) @@ -55,10 +55,10 @@ 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, gist._state.view]) + }, [gistReady, gist._state.view]) // If we need to preload the gist, do so useEffect(() => { @@ -109,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, From 7b5ed688392135ca714d67d342bec7f7afd22a22 Mon Sep 17 00:00:00 2001 From: Enoch Riese Date: Wed, 22 Jun 2022 19:13:52 -0500 Subject: [PATCH 4/4] fix gist loading --- sites/shared/components/workbench/preload.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sites/shared/components/workbench/preload.js b/sites/shared/components/workbench/preload.js index 48325f043fb..ea5009fad21 100644 --- a/sites/shared/components/workbench/preload.js +++ b/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'] } }