OpenNeo/impress-2020
OpenNeo/impress-2020
1
0
Fork 0
Code Pull requests Activity

Fix performance regression in SpeciesColorPicker

Browse source 
Oops, my cute API idea for `speciesPickerProps` breaks `React.memo`, of course!

We could fix this by having the caller memoize the `speciesPickerProps` object, but that's too unusual and error-prone. We could also fix this by writing a custom function for `React.memo` to determine whether props match, but that seems like overkill when there's only one actual prop we're using here in practice.

So yeah, I've updated `SpeciesColorPicker` to just accept `speciesTestId` and `colorTestId` props instead!

Note that actually this component _is_ still re-rendering too often, because of a Chakra bug I just discovered and reported! So this change won't immediately improve performance, but it should stop re-rendering too often once we _also_ upgrade Chakra after this bug is fixed. https://github.com/chakra-ui/chakra-ui/issues/4080
This commit is contained in:
Emi Matchu 2021-05-24 17:50:31 -07:00
parent e3ae5e7116
commit e5551a6847
2 changed files with 13 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/app/WardrobePage/OutfitControls.js
View file

@ -191,21 +191,15 @@ function OutfitControls({
</Flex>
<Box flex="0 0 auto">
<DarkMode>
{
<SpeciesColorPicker
speciesId={outfitState.speciesId}
colorId={outfitState.colorId}
idealPose={outfitState.pose}
onChange={onSpeciesColorChange}
stateMustAlwaysBeValid
speciesPickerProps={{
"data-test-id": "wardrobe-species-picker",
}}
colorPickerProps={{
"data-test-id": "wardrobe-color-picker",
}}
/>
}
<SpeciesColorPicker
speciesId={outfitState.speciesId}
colorId={outfitState.colorId}
idealPose={outfitState.pose}
onChange={onSpeciesColorChange}
stateMustAlwaysBeValid
speciesTestId="wardrobe-species-picker"
colorTestId="wardrobe-color-picker"
/>
</DarkMode>
</Box>
<Flex flex="1 1 0" align="center" pl="2">

8
src/app/components/SpeciesColorPicker.js
View file

@ -27,8 +27,8 @@ function SpeciesColorPicker({
isDisabled = false,
speciesIsDisabled = false,
size = "md",
speciesPickerProps = {},
colorPickerProps = {},
speciesTestId = null,
colorTestId = null,
onChange,
}) {
const { loading: loadingMeta, error: errorMeta, data: meta } = useQuery(gql`
@ -190,7 +190,7 @@ function SpeciesColorPicker({
valids={valids}
speciesId={speciesId}
colorId={colorId}
{...colorPickerProps}
data-test-id={colorTestId}
>
{
// If the selected color isn't in the set we have here, show the
@ -234,7 +234,7 @@ function SpeciesColorPicker({
valids={valids}
speciesId={speciesId}
colorId={colorId}
{...speciesPickerProps}
data-test-id={speciesTestId}
>
{
// If the selected species isn't in the set we have here, show the