Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve Customize Selections in State on SiteGen Editor Page #464

Merged
merged 7 commits into from
Feb 20, 2024
Merged

Preserve Customize Selections in State on SiteGen Editor Page #464

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
804c4cc
Save the customize sidebar selection
mr-vara Feb 13, 2024
4d8d880
Fix lint issues
mr-vara Feb 13, 2024
98f92ee
Fix issue with Reset
mr-vara Feb 14, 2024
398431f
Revamp DesignColorsPanel code to make it more readable
mr-vara Feb 14, 2024
f02913c
Revamp DesignFontsPanel code to make it more readable
mr-vara Feb 14, 2024
64cf19c
Fix issue with fonts in the preview page
mr-vara Feb 16, 2024
52f9d20
Fix issue with preselected fonts
mr-vara Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions src/OnboardingSPA/components/Sidebar/components/Customize/DesignColorsPanel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ const DesignColorsPanel = forwardRef(
setSelectedPalette( null );
setCurrentOnboardingData( updatedData );
}
currentData.sitegen.homepages.active.color.selectedPalette =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

null;
currentData.sitegen.homepages.data[
slug
].color.selectedPalette = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

setCurrentOnboardingData( currentData );
}
};

Expand Down Expand Up @@ -114,15 +120,25 @@ const DesignColorsPanel = forwardRef(
useEffect( () => {
if ( ! customColors ) {
const storedCustomColors =
currentData.sitegen.homepages.active.color
.customColors;
currentData.sitegen.homepages.active.color.customColors;
if ( storedCustomColors ) {
setCustomColors( storedCustomColors );
} else {
const defaultCustomColors = palettes[ 0 ];
setCustomColors( defaultCustomColors );
}
}
if ( ! selectedPalette ) {
const storedSelectedPalette =
currentData.sitegen.homepages.active.color.selectedPalette;
if ( storedSelectedPalette ) {
setSelectedPalette( storedSelectedPalette );
if ( storedSelectedPalette === 'custom' ) {
setShowCustomColors( true );
setSelectedCustomColors( true );
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ currentData ] );

Expand Down Expand Up @@ -188,8 +204,12 @@ const DesignColorsPanel = forwardRef(
currentData.sitegen.homepages.data[ slug ].color.customColors =
selectedColor;
currentData.sitegen.homepages.active.color.customColors =
selectedColor;
selectedColor;
}
currentData.sitegen.homepages.data[ slug ].color.selectedPalette =
selectedPalette;
currentData.sitegen.homepages.active.color.selectedPalette =
selectedPalette;

const colorPaletteIndex =
selectedPalette === 'custom' ? 0 : selectedPalette;
Expand Down Expand Up @@ -345,9 +365,9 @@ const DesignColorsPanel = forwardRef(
label={
idx === 0
? __(
'Default',
'wp-module-onboarding'
)
'Default',
'wp-module-onboarding'
)
: ''
}
selectedPalette={ selectedPalette }
Expand Down
28 changes: 24 additions & 4 deletions src/OnboardingSPA/components/Sidebar/components/Customize/DesignFontsPanel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,13 @@ const DesignFontsPanel = forwardRef(
const resetToDefaultFonts = () => {
setStylesOfCurrentData();
setSelectedGroup( null );
setShowCustomFonts( false );
setSelectedCustomFont( null );
const slug = currentData.sitegen?.homepages?.active?.slug;
arunshenoy99 marked this conversation as resolved.
Show resolved Hide resolved
if ( slug ) {
currentData.sitegen.homepages.data[ slug ].selectedFontGroup =
null;
currentData.sitegen.homepages.active.selectedFontGroup = null;
setCurrentOnboardingData( currentData );
}
};

const setStylesOfCurrentData = ( heading = '', body = '' ) => {
Expand Down Expand Up @@ -274,6 +279,17 @@ const DesignFontsPanel = forwardRef(
currentData.sitegen.homepages.active.customFont;
if ( storedCustomFonts ) {
setCustomFont( storedCustomFonts );
setSelectedCustomFont( storedCustomFonts );
}
}
if ( ! selectedGroup ) {
const storedSelectedGroup =
currentData.sitegen.homepages.active.selectedFontGroup;
if ( storedSelectedGroup ) {
setSelectedGroup( storedSelectedGroup );
if ( storedSelectedGroup === 'custom' ) {
setShowCustomFonts( true );
}
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand All @@ -282,20 +298,24 @@ const DesignFontsPanel = forwardRef(
const handleUpdatePreviewSettings = () => {
let headings;
let body;
const slug = currentData.sitegen?.homepages?.active?.slug;
if ( selectedGroup === 'custom' ) {
headings = `var(--wp--preset--font-family--${ customFont.headings })`;
body = `var(--wp--preset--font-family--${ customFont.body })`;
const slug = currentData.sitegen?.homepages?.active?.slug;
if ( slug ) {
currentData.sitegen.homepages.data[ slug ].customFont =
customFont;
currentData.sitegen.homepages.active.customFont =
customFont;
customFont;
}
} else {
headings = `var(--wp--preset--font-family--${ fontGroups[ selectedGroup ].headingsSlug })`;
body = `var(--wp--preset--font-family--${ fontGroups[ selectedGroup ].bodySlug })`;
}
currentData.sitegen.homepages.data[ slug ].selectedFontGroup =
selectedGroup;
currentData.sitegen.homepages.active.selectedFontGroup =
selectedGroup;
setStylesOfCurrentData( headings, body );
};

Expand Down
Loading