Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
50 changes: 37 additions & 13 deletions packages/edit-site/src/components/editor/global-styles-provider.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { set, get } from 'lodash';
import { set, get, mapValues } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -15,16 +15,21 @@ import {
} from '@wordpress/element';
import { useEntityProp } from '@wordpress/core-data';
import { __EXPERIMENTAL_STYLE_PROPERTY as STYLE_PROPERTY } from '@wordpress/blocks';
import { useSelect, useDispatch } from '@wordpress/data';

/**
* Internal dependencies
*/
import getGlobalStyles from './global-styles-renderer';
import {
default as getGlobalStyles,
mergeTrees,
} from './global-styles-renderer';

const EMPTY_CONTENT = '{}';

const GlobalStylesContext = createContext( {
/* eslint-disable no-unused-vars */
mergedStyles: {},
Comment thread
jorgefilipecosta marked this conversation as resolved.
Resolved
getSetting: ( context, path ) => {},
setSetting: ( context, path, newValue ) => {},
getStyleProperty: ( context, propertyName ) => {},
Expand All @@ -48,17 +53,25 @@ export const useGlobalStylesReset = () => {
];
};

export default ( { children, baseStyles, contexts } ) => {
export default function GlobalStylesProvider( {
children,
baseStyles,
contexts,
} ) {
const [ content, setContent ] = useGlobalStylesEntityContent();

const userStyles = useMemo(
() => ( content ? JSON.parse( content ) : {} ),
[ content ]
);
const { userStyles, mergedStyles } = useMemo( () => {
const parsedContent = content ? JSON.parse( content ) : {};
return {
userStyles: parsedContent,
mergedStyles: mergeTrees( baseStyles, parsedContent ),
};
}, [ content ] );

const nextValue = useMemo(
() => ( {
contexts,
mergedStyles,
Comment thread
jorgefilipecosta marked this conversation as resolved.
Resolved
getSetting: ( context, path ) =>
get( userStyles?.[ context ]?.settings, path ),
setSetting: ( context, path, newValue ) => {
Expand Down Expand Up @@ -110,16 +123,27 @@ export default ( { children, baseStyles, contexts } ) => {
.appendChild( styleNode );
}

styleNode.innerText = getGlobalStyles(
contexts,
baseStyles,
userStyles
);
styleNode.innerText = getGlobalStyles( contexts, mergedStyles );
}, [ contexts, baseStyles, content ] );

const settings = useSelect( ( select ) =>
select( 'core/edit-site' ).getSettings()
);
Comment on lines +127 to +129

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This useSelect call is missing a dependency array.

const { updateSettings } = useDispatch( 'core/edit-site' );

useEffect( () => {
updateSettings( {
...settings,
__experimentalFeatures: mapValues(
mergedStyles,
( value ) => value.settings || {}
),
} );
}, [ mergedStyles ] );

return (
<GlobalStylesContext.Provider value={ nextValue }>
{ children }
</GlobalStylesContext.Provider>
);
};
}
19 changes: 13 additions & 6 deletions packages/edit-site/src/components/editor/global-styles-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,46 @@ import { __EXPERIMENTAL_STYLE_PROPERTY as STYLE_PROPERTY } from '@wordpress/bloc
*/
import { PRESET_CATEGORIES, LINK_COLOR_DECLARATION } from './utils';

const mergeTrees = ( baseData, userData ) => {
export const mergeTrees = ( baseData, userData ) => {
// Deep clone from base data.
//
// We don't use cloneDeep from lodash here
// because we know the data is JSON compatible,
// see https://github.com/lodash/lodash/issues/1984
const mergedTree = JSON.parse( JSON.stringify( baseData ) );

const styleKeys = [ 'typography', 'color' ];
const styleKeys = [ 'typography', 'color', 'custom', 'spacing' ];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

custom and spacing are not valid style keys, they are only relevant for settings. We probably need to port the normalize block schema (and its function) from the (server).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I renamed the variable to validMergeKeys so it can contain keys valid for style and settings. I think ideally we should update the function to no even need to rely on this hardcoded data.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But for now, it seems in an acceptable solution following a similar approach to what we had.

@oandregal oandregal Oct 16, 2020 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey, I hope you don't mind, but I submitted a commit at 6652d28 with a simple fix for this (it can be improved in subsequent PRs).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you for the enhancements :)

Object.keys( userData ).forEach( ( context ) => {
styleKeys.forEach( ( key ) => {
// Normalize object shape: make sure the key exists under styles.
// Normalize object shape: make sure the key exists under styles and settings.
if ( ! mergedTree[ context ].styles?.[ key ] ) {
mergedTree[ context ].styles[ key ] = {};
}

if ( ! mergedTree[ context ].settings?.[ key ] ) {
mergedTree[ context ].settings[ key ] = {};
}

// Merge data: base + user.
mergedTree[ context ].styles[ key ] = {
...mergedTree[ context ].styles[ key ],
...userData[ context ]?.styles?.[ key ],
};

mergedTree[ context ].settings[ key ] = {
...mergedTree[ context ].settings[ key ],
...userData[ context ]?.settings?.[ key ],
};
} );
} );

return mergedTree;
};

export default ( blockData, baseTree, userTree ) => {
export default ( blockData, tree ) => {
const styles = [];
// Can this be converted to a context, as the global context?
// See comment in the server.
styles.push( LINK_COLOR_DECLARATION );
const tree = mergeTrees( baseTree, userTree );

/**
* Transform given style tree into a set of style declarations.
Expand Down
14 changes: 14 additions & 0 deletions packages/edit-site/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,17 @@ export function setIsInserterOpened( isOpen ) {
isOpen,
};
}

/**
* Returns an action object used to update the settings.
*
* @param {Object} settings New settings.
*
* @return {Object} Action object.
*/
export function updateSettings( settings ) {
return {
type: 'UPDATE_SETTINGS',
settings,
};
}