From ab2dbeb02a2816d0c0fb7278d01989c2b237d84e Mon Sep 17 00:00:00 2001 From: Matchu Date: Fri, 11 Jun 2021 06:45:11 -0700 Subject: [PATCH] Item page perf: memoize species faces This is a pretty easy change, that makes re-renders faster when something about the item preview state changes! That said, the initial render is still pretty slow, too, and that's the one that's bothering me more lol --- src/app/ItemPage.js | 443 +++++++++++++++++++++++--------------------- src/app/util.js | 21 ++- 2 files changed, 242 insertions(+), 222 deletions(-) diff --git a/src/app/ItemPage.js b/src/app/ItemPage.js index 7384e12..756b141 100644 --- a/src/app/ItemPage.js +++ b/src/app/ItemPage.js @@ -528,30 +528,40 @@ function ItemPageOutfitPreview({ itemId }) { null ); - const setPetStateFromUserAction = (newPetState) => { - setPetState(newPetState); + const setPetStateFromUserAction = React.useCallback( + (newPetState) => + setPetState((prevPetState) => { + // When the user _intentionally_ chooses a species or color, save it in + // local storage for next time. (This won't update when e.g. their + // preferred species or color isn't available for this item, so we update + // to the canonical species or color automatically.) + // + // Re the "ifs", I have no reason to expect null to come in here, but, + // since this is touching client-persisted data, I want it to be even more + // reliable than usual! + if ( + newPetState.speciesId && + newPetState.speciesId !== prevPetState.speciesId + ) { + setPreferredSpeciesId(newPetState.speciesId); + } + if ( + newPetState.colorId && + newPetState.colorId !== prevPetState.colorId + ) { + if (colorIsBasic(newPetState.colorId)) { + // When the user chooses a basic color, don't index on it specifically, + // and instead reset to use default colors. + setPreferredColorId(null); + } else { + setPreferredColorId(newPetState.colorId); + } + } - // When the user _intentionally_ chooses a species or color, save it in - // local storage for next time. (This won't update when e.g. their - // preferred species or color isn't available for this item, so we update - // to the canonical species or color automatically.) - // - // Re the "ifs", I have no reason to expect null to come in here, but, - // since this is touching client-persisted data, I want it to be even more - // reliable than usual! - if (newPetState.speciesId && newPetState.speciesId !== petState.speciesId) { - setPreferredSpeciesId(newPetState.speciesId); - } - if (newPetState.colorId && newPetState.colorId !== petState.colorId) { - if (colorIsBasic(newPetState.colorId)) { - // When the user chooses a basic color, don't index on it specifically, - // and instead reset to use default colors. - setPreferredColorId(null); - } else { - setPreferredColorId(newPetState.colorId); - } - } - }; + return newPetState; + }), + [setPreferredColorId, setPreferredSpeciesId] + ); // We don't need to reload this query when preferred species/color change, so // cache their initial values here to use as query arguments. @@ -694,6 +704,21 @@ function ItemPageOutfitPreview({ itemId }) { const isCompatible = itemLayers.length > 0; const usesHTML5 = itemLayers.every(layerUsesHTML5); + const onChange = React.useCallback( + ({ speciesId, colorId }) => { + const validPoses = getValidPoses(valids, speciesId, colorId); + const pose = getClosestPose(validPoses, idealPose); + setPetStateFromUserAction({ + speciesId, + colorId, + pose, + isValid: true, + appearanceId: null, + }); + }, + [valids, idealPose, setPetStateFromUserAction] + ); + const borderColor = useColorModeValue("green.700", "green.400"); const errorColor = useColorModeValue("red.600", "red.400"); @@ -824,17 +849,7 @@ function ItemPageOutfitPreview({ itemId }) { selectedColorId={petState.colorId} compatibleBodies={compatibleBodies} couldProbablyModelMoreData={couldProbablyModelMoreData} - onChange={({ speciesId, colorId }) => { - const validPoses = getValidPoses(valids, speciesId, colorId); - const pose = getClosestPose(validPoses, idealPose); - setPetStateFromUserAction({ - speciesId, - colorId, - pose, - isValid: true, - appearanceId: null, - }); - }} + onChange={onChange} isLoading={loadingGQL || loadingValids} /> @@ -1087,195 +1102,197 @@ function SpeciesFacesPicker({ ); } -function SpeciesFaceOption({ - speciesId, - speciesName, - colorId, - neopetsImageHash, - isSelected, - bodyIsCompatible, - isValid, - couldProbablyModelMoreData, - onChange, - isLoading, -}) { - const selectedBorderColor = useColorModeValue("green.600", "green.400"); - const selectedBackgroundColor = useColorModeValue("green.200", "green.600"); - const focusBorderColor = "blue.400"; - const focusBackgroundColor = "blue.100"; - const [ - selectedBorderColorValue, - selectedBackgroundColorValue, - focusBorderColorValue, - focusBackgroundColorValue, - ] = useToken("colors", [ - selectedBorderColor, - selectedBackgroundColor, - focusBorderColor, - focusBackgroundColor, - ]); - const xlShadow = useToken("shadows", "xl"); +const SpeciesFaceOption = React.memo( + ({ + speciesId, + speciesName, + colorId, + neopetsImageHash, + isSelected, + bodyIsCompatible, + isValid, + couldProbablyModelMoreData, + onChange, + isLoading, + }) => { + const selectedBorderColor = useColorModeValue("green.600", "green.400"); + const selectedBackgroundColor = useColorModeValue("green.200", "green.600"); + const focusBorderColor = "blue.400"; + const focusBackgroundColor = "blue.100"; + const [ + selectedBorderColorValue, + selectedBackgroundColorValue, + focusBorderColorValue, + focusBackgroundColorValue, + ] = useToken("colors", [ + selectedBorderColor, + selectedBackgroundColor, + focusBorderColor, + focusBackgroundColor, + ]); + const xlShadow = useToken("shadows", "xl"); - const [labelIsHovered, setLabelIsHovered] = React.useState(false); - const [inputIsFocused, setInputIsFocused] = React.useState(false); + const [labelIsHovered, setLabelIsHovered] = React.useState(false); + const [inputIsFocused, setInputIsFocused] = React.useState(false); - const isDisabled = isLoading || !isValid || !bodyIsCompatible; - const isHappy = isLoading || (isValid && bodyIsCompatible); - const emotionId = isHappy ? "1" : "2"; - const cursor = isLoading ? "wait" : isDisabled ? "not-allowed" : "pointer"; + const isDisabled = isLoading || !isValid || !bodyIsCompatible; + const isHappy = isLoading || (isValid && bodyIsCompatible); + const emotionId = isHappy ? "1" : "2"; + const cursor = isLoading ? "wait" : isDisabled ? "not-allowed" : "pointer"; - let disabledExplanation = null; - if (isLoading) { - // If we're still loading, don't try to explain anything yet! - } else if (!isValid) { - disabledExplanation = "(Can't be this color)"; - } else if (!bodyIsCompatible) { - disabledExplanation = couldProbablyModelMoreData - ? "(Item needs models)" - : "(Not compatible)"; - } + let disabledExplanation = null; + if (isLoading) { + // If we're still loading, don't try to explain anything yet! + } else if (!isValid) { + disabledExplanation = "(Can't be this color)"; + } else if (!bodyIsCompatible) { + disabledExplanation = couldProbablyModelMoreData + ? "(Item needs models)" + : "(Not compatible)"; + } - const tooltipLabel = ( -
- {speciesName} - {disabledExplanation && ( -
- {disabledExplanation} -
- )} -
- ); + const tooltipLabel = ( +
+ {speciesName} + {disabledExplanation && ( +
+ {disabledExplanation} +
+ )} +
+ ); - // NOTE: Because we render quite a few of these, avoiding using Chakra - // elements like Box helps with render performance! - return ( - - {({ css }) => ( - - - - )} - - ); -} +
+ +
+ + + )} + + ); + } +); function ItemZonesInfo({ compatibleBodiesAndTheirZones, restrictedZones }) { // Reorganize the body-and-zones data, into zone-and-bodies data. Also, we're diff --git a/src/app/util.js b/src/app/util.js index fcc50b3..c78c0ea 100644 --- a/src/app/util.js +++ b/src/app/util.js @@ -309,15 +309,18 @@ export function useLocalStorage(key, initialValue) { const [storedValue, setStoredValue] = React.useState(loadValue); - const setValue = (value) => { - try { - setStoredValue(value); - window.localStorage.setItem(key, JSON.stringify(value)); - storageListeners.forEach((l) => l()); - } catch (error) { - console.error(error); - } - }; + const setValue = React.useCallback( + (value) => { + try { + setStoredValue(value); + window.localStorage.setItem(key, JSON.stringify(value)); + storageListeners.forEach((l) => l()); + } catch (error) { + console.error(error); + } + }, + [key] + ); const reloadValue = React.useCallback(() => { setStoredValue(loadValue());