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.

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

Global styles: consolidate theme.json ref and URI resolution #64182

Merged
merged 3 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
/**
* Internal dependencies
*/
import {
setThemeFileUris,
getResolvedThemeFilePath,
} from '../theme-file-uri-utils';
import { getResolvedThemeFilePath } from '../theme-file-uri-utils';

const themeFileURIs = [
{
Expand All @@ -19,28 +16,6 @@ const themeFileURIs = [
},
];

describe( 'setThemeFileUris()', () => {
const themeJson = {
styles: {
background: {
backgroundImage: {
url: 'file:./assets/image.jpg',
},
},
},
};

it( 'should replace relative paths with resolved URIs if found in themeFileURIs', () => {
const newThemeJson = setThemeFileUris( themeJson, themeFileURIs );
expect(
newThemeJson.styles.background.backgroundImage.url ===
'https://wordpress.org/assets/image.jpg'
).toBe( true );
// Object reference should be the same as the function is mutating the object.
expect( newThemeJson ).toEqual( themeJson );
} );
} );

describe( 'getResolvedThemeFilePath()', () => {
it.each( [
[
Expand Down
120 changes: 120 additions & 0 deletions packages/block-editor/src/components/global-styles/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import {
getPresetVariableFromValue,
getValueFromVariable,
scopeFeatureSelectors,
getResolvedThemeFilePath,
getResolvedRefValue,
getResolvedValue,
} from '../utils';

describe( 'editor utils', () => {
Expand Down Expand Up @@ -52,6 +55,41 @@ describe( 'editor utils', () => {
},
},
},
styles: {
background: {
backgroundImage: {
url: 'file:./assets/image.jpg',
},
backgroundAttachment: 'fixed',
backgroundPosition: 'top left',
},
blocks: {
'core/group': {
background: {
backgroundImage: {
ref: 'styles.background.backgroundImage',
},
},
dimensions: {
minHeight: '100px',
},
},
},
},
_links: {
'wp:theme-file': [
{
name: 'file:./assets/image.jpg',
href: 'https://wordpress.org/assets/image.jpg',
target: 'styles.background.backgroundImage.url',
},
{
name: 'file:./assets/other/image.jpg',
href: 'https://wordpress.org/assets/other/image.jpg',
target: "styles.blocks.['core/group'].background.backgroundImage.url",
},
],
},
isGlobalStylesUserThemeJSON: true,
};

Expand Down Expand Up @@ -366,4 +404,86 @@ describe( 'editor utils', () => {
} );
} );
} );

describe( 'getResolvedThemeFilePath()', () => {
it.each( [
[
'file:./assets/image.jpg',
'https://wordpress.org/assets/image.jpg',
'Should return absolute URL if found in themeFileURIs',
],
[
'file:./misc/image.jpg',
'file:./misc/image.jpg',
'Should return value if not found in themeFileURIs',
],
[
'https://wordpress.org/assets/image.jpg',
'https://wordpress.org/assets/image.jpg',
'Should not match absolute URLs',
],
] )(
'Given file %s and return value %s: %s',
( file, returnedValue ) => {
expect(
getResolvedThemeFilePath(
file,
themeJson._links[ 'wp:theme-file' ]
) === returnedValue
).toBe( true );
}
);
} );

describe( 'getResolvedRefValue()', () => {
it.each( [
[ 'blue', 'blue', null ],
[ 0, 0, themeJson ],
[
{ ref: 'styles.background.backgroundImage' },
{ url: 'file:./assets/image.jpg' },
themeJson,
],
[
{
ref: 'styles.blocks.core/group.background.backgroundImage',
},
undefined,
themeJson,
],
] )(
'Given ruleValue %s return expected value of %s',
( ruleValue, returnedValue, tree ) => {
expect( getResolvedRefValue( ruleValue, tree ) ).toEqual(
returnedValue
);
}
);
} );

describe( 'getResolvedValue()', () => {
it.each( [
[ 'blue', 'blue', null ],
[ 0, 0, themeJson ],
[
{ ref: 'styles.background.backgroundImage' },
{ url: 'https://wordpress.org/assets/image.jpg' },
themeJson,
],
[
{
ref: 'styles.blocks.core/group.background.backgroundImage',
},
undefined,
themeJson,
],
] )(
'Given ruleValue %s return expected value of %s',
( ruleValue, returnedValue, tree ) => {
expect( getResolvedValue( ruleValue, tree ) ).toEqual(
returnedValue
);
}
);
} );
} );
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* Internal dependencies
*/
import { getValueFromObjectPath } from '../../utils/object';

/**
* Looks up a theme file URI based on a relative path.
*
Expand All @@ -21,57 +16,3 @@ export function getResolvedThemeFilePath( file, themeFileURIs = [] ) {

return uri?.href;
}

/**
* Mutates an object by settings a value at the provided path.
*
* @param {Object} object Object to set a value in.
* @param {number|string|Array} path Path in the object to modify.
* @param {*} value New value to set.
* @return {Object} Object with the new value set.
*/
function setMutably( object, path, value ) {
path = path.split( '.' );
const finalValueKey = path.pop();
let prev = object;

for ( const key of path ) {
const current = prev[ key ];
prev = current;
}

prev[ finalValueKey ] = value;

return object;
}

/**
* Resolves any relative paths if a corresponding theme file URI is available.
* Note: this function mutates the object and is specifically to be used in
* an async styles build context in useGlobalStylesOutput
*
* @param {Object} themeJson Theme.json/Global styles tree.
* @param {Array<Object>} themeFileURIs A collection of absolute theme file URIs and their corresponding file paths.
* @return {Object} Returns mutated object.
*/
export function setThemeFileUris( themeJson, themeFileURIs ) {
if ( ! themeJson?.styles || ! themeFileURIs ) {
return themeJson;
}

themeFileURIs.forEach( ( { name, href, target } ) => {
const value = getValueFromObjectPath( themeJson, target );
if ( value === name ) {
/*
* The object must not be updated immutably here because the
* themeJson is a reference to the global styles tree used as a dependency in the
* useGlobalStylesOutputWithConfig() hook. If we don't mutate the object,
* the hook will detect the change and re-render the component, resulting
* in a maximum depth exceeded error.
*/
themeJson = setMutably( themeJson, target, href );
}
} );

return themeJson;
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
scopeFeatureSelectors,
appendToSelector,
getBlockStyleVariationSelector,
compileStyleValue,
getResolvedValue,
} from './utils';
import { getBlockCSSSelector } from './get-block-css-selector';
import { getTypographyFontSizeValue } from './typography-utils';
Expand All @@ -36,7 +38,6 @@ import { store as blockEditorStore } from '../../store';
import { LAYOUT_DEFINITIONS } from '../../layouts/definitions';
import { getValueFromObjectPath, setImmutably } from '../../utils/object';
import { unlock } from '../../lock-unlock';
import { setThemeFileUris } from './theme-file-uri-utils';

// Elements that rely on class names in their selectors.
const ELEMENT_CLASS_NAMES = {
Expand All @@ -54,21 +55,6 @@ const BLOCK_SUPPORT_FEATURE_LEVEL_SELECTORS = {
};
const { kebabCase } = unlock( componentsPrivateApis );

function compileStyleValue( uncompiledValue ) {
const VARIABLE_REFERENCE_PREFIX = 'var:';
const VARIABLE_PATH_SEPARATOR_TOKEN_ATTRIBUTE = '|';
const VARIABLE_PATH_SEPARATOR_TOKEN_STYLE = '--';

if ( uncompiledValue?.startsWith?.( VARIABLE_REFERENCE_PREFIX ) ) {
const variable = uncompiledValue
.slice( VARIABLE_REFERENCE_PREFIX.length )
.split( VARIABLE_PATH_SEPARATOR_TOKEN_ATTRIBUTE )
.join( VARIABLE_PATH_SEPARATOR_TOKEN_STYLE );
return `var(--wp--${ variable })`;
}
return uncompiledValue;
}

/**
* Transform given preset tree into a set of style declarations.
*
Expand Down Expand Up @@ -395,21 +381,40 @@ export function getStylesDeclarations(
);

/*
* Set background defaults.
* Applies to all background styles except the top-level site background.
* Preprocess background image values.
*
* Note: As we absorb more and more styles into the engine, we could simplify this function.
* A refactor is for the style engine to handle ref resolution (and possibly defaults)
* via a public util used internally and externally. Theme.json tree and defaults could be passed
* as options.
*/
if ( ! isRoot && !! blockStyles.background ) {
blockStyles = {
...blockStyles,
background: {
...blockStyles.background,
...setBackgroundStyleDefaults( blockStyles.background ),
},
};
if ( !! blockStyles.background ) {
/*
* Resolve dynamic values before they are compiled by the style engine,
* which doesn't (yet) resolve dynamic values.
*/
if ( blockStyles.background?.backgroundImage ) {
blockStyles.background.backgroundImage = getResolvedValue(
andrewserong marked this conversation as resolved.
Show resolved Hide resolved
blockStyles.background.backgroundImage,
tree
);
}

/*
* Set default values for block background styles.
* Top-level styles are an exception as they are applied to the body.
*/
if ( ! isRoot ) {
blockStyles = {
...blockStyles,
background: {
...blockStyles.background,
...setBackgroundStyleDefaults( blockStyles.background ),
},
};
}
}

// The goal is to move everything to server side generated engine styles
// This is temporary as we absorb more and more styles into the engine.
const extraRules = getCSSRules( blockStyles );
extraRules.forEach( ( rule ) => {
// Don't output padding properties if padding variables are set or if we're not editing a full template.
Expand All @@ -424,18 +429,7 @@ export function getStylesDeclarations(
? rule.key
: kebabCase( rule.key );

let ruleValue = rule.value;
if ( typeof ruleValue !== 'string' && ruleValue?.ref ) {
const refPath = ruleValue.ref.split( '.' );
ruleValue = compileStyleValue(
getValueFromObjectPath( tree, refPath )
);
// Presence of another ref indicates a reference to another dynamic value.
// Pointing to another dynamic value is not supported.
if ( ! ruleValue || ruleValue?.ref ) {
return;
}
}
let ruleValue = getResolvedValue( rule.value, tree, null );

// Calculate fluid typography rules where available.
if ( cssProperty === 'font-size' ) {
Expand Down Expand Up @@ -1398,10 +1392,6 @@ export function useGlobalStylesOutputWithConfig(
disableRootPadding
) {
const [ blockGap ] = useGlobalSetting( 'spacing.blockGap' );
mergedConfig = setThemeFileUris(
mergedConfig,
mergedConfig?._links?.[ 'wp:theme-file' ]
);
const hasBlockGapSupport = blockGap !== null;
const hasFallbackGapSupport = ! hasBlockGapSupport; // This setting isn't useful yet: it exists as a placeholder for a future explicit fallback styles support.
const disableLayoutStyles = useSelect( ( select ) => {
Expand Down
Loading
Loading