Preserve Customize Selections in State on SiteGen Editor Page #464
Conversation
There was a problem hiding this comment.
Hi Vara, please review the following recordings/images and confirm if this behaviour is expected:
Colors are not resetting in the preview when clicking reset.
Screen.Recording.2024-02-14.at.9.18.02.AM.mov
Selecting "Pick Your Own Colors" automatically applies the previously selected custom color instead of opening the color palette.
Screen.Recording.2024-02-14.at.9.20.07.AM.mov
Fonts selected in the editor step are not reflected in the preview step, unlike colors.
Finally, can we please take a look at reducing the number of if else statements in both src/OnboardingSPA/components/Sidebar/components/Customize/DesignColorsPanel/index.js and src/OnboardingSPA/components/Sidebar/components/Customize/DesignFontsPanel/index.js? It seems like there is a lot of nesting, and both of these files have become very hard to understand.
| currentData.sitegen.homepages.active.color.selectedPalette = | ||
| null; | ||
| currentData.sitegen.homepages.data[ | ||
| slug | ||
| ].color.selectedPalette = null; |
There was a problem hiding this comment.
We should change this to an empty object or array instead of null. Also, if we're already setting all of this to empty at the end of the function, why do all the operations above this? That code can be deleted.
| @@ -55,6 +55,12 @@ const DesignColorsPanel = forwardRef( | |||
| setSelectedPalette( null ); | |||
| setCurrentOnboardingData( updatedData ); | |||
| } | |||
| currentData.sitegen.homepages.active.color.selectedPalette = | |||
There was a problem hiding this comment.
Can we invert all the if statements in the resetToDefaultColors and other functions to check for negation? This will help reduce the overall nesting in the function.
| if ( ! selectedPalette ) { | ||
| const storedSelectedPalette = | ||
| currentData.sitegen.homepages.active.color.selectedPalette; | ||
| if ( storedSelectedPalette ) { | ||
| setSelectedPalette( storedSelectedPalette ); | ||
| if ( storedSelectedPalette === 'custom' ) { | ||
| setShowCustomColors( true ); | ||
| setSelectedCustomColors( true ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Why do we have so many states like storedCustomColors and storedSelectedPalette custom? Can we use a single state with a common data structure for storing default/pre-defined/custom colors and then just mark that as default/pre-defined/custom in another state variable? This way, we will only have to maintain two states.
src/OnboardingSPA/components/Sidebar/components/Customize/DesignFontsPanel/index.js
Show resolved
Hide resolved
mr-vara
added
QA
avneet-raj
added
Ready to merge
This pull request addresses an enhancement in the SiteGen Editor page, ensuring that user selections are now preserved in the application's state. With this update, when users revisit the page, their previous selections will be retained, providing a more seamless and user-friendly experience.