Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Product Title block: add support global style #5515

Merged
merged 8 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
55 changes: 26 additions & 29 deletions assets/js/atomic/blocks/product-elements/title/block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
useInnerBlockLayoutContext,
useProductDataContext,
} from '@woocommerce/shared-context';
import { getColorClassName, getFontSizeClass } from '@wordpress/block-editor';
import { isFeaturePluginBuild } from '@woocommerce/block-settings';
import { withProductDataContext } from '@woocommerce/shared-hocs';
import ProductName from '@woocommerce/base-components/product-name';
Expand All @@ -18,6 +17,11 @@ import { useStoreEvents } from '@woocommerce/base-context/hooks';
*/
import './style.scss';
import { Attributes } from './types';
import {
useSpacingProps,
useTypographyProps,
useColorProps,
} from '../../../../utils/style-attributes-utils';

type Props = Attributes & HTMLAttributes< HTMLDivElement >;

Expand Down Expand Up @@ -49,33 +53,21 @@ const TagName = ( {
* will be used if this is not provided.
* @return {*} The component.
*/
export const Block = ( {
className,
headingLevel = 2,
showProductLink = true,
align,
textColor,
fontSize,
style,
}: Props ): JSX.Element => {
export const Block = ( props: Props ): JSX.Element => {
const {
className,
headingLevel = 2,
showProductLink = true,
align,
} = props;

const { parentClassName } = useInnerBlockLayoutContext();
const { product } = useProductDataContext();
const { dispatchStoreEvent } = useStoreEvents();

const colorClass = getColorClassName( 'color', textColor );
const fontSizeClass = getFontSizeClass( fontSize );
const titleClasses = classnames( 'wp-block-woocommerce-product-title', {
'has-text-color': textColor || style?.color?.text || style?.color,
[ `has-font-size` ]:
fontSize || style?.typography?.fontSize || style?.fontSize,
[ colorClass ]: colorClass,
[ fontSizeClass ]: fontSizeClass,
} );

const titleStyle = {
fontSize: style?.fontSize || style?.typography?.fontSize,
color: style?.color?.text || style?.color,
};
const colorProps = useColorProps( props );
const spacingProps = useSpacingProps( props );
const typographyProps = useTypographyProps( props );

if ( ! product.id ) {
return (
Expand All @@ -88,7 +80,6 @@ export const Block = ( {
[ `${ parentClassName }__product-title` ]: parentClassName,
[ `wc-block-components-product-title--align-${ align }` ]:
align && isFeaturePluginBuild(),
[ titleClasses ]: isFeaturePluginBuild(),
}
) }
/>
Expand All @@ -109,9 +100,7 @@ export const Block = ( {
) }
>
<ProductName
className={ classnames( {
[ titleClasses ]: isFeaturePluginBuild(),
} ) }
className={ colorProps.className }
disabled={ ! showProductLink }
name={ product.name }
permalink={ product.permalink }
Expand All @@ -121,7 +110,15 @@ export const Block = ( {
product,
} );
} }
style={ isFeaturePluginBuild() ? titleStyle : {} }
style={
isFeaturePluginBuild()
? {
...spacingProps.style,
...typographyProps.style,
...colorProps.style,
}
: {}
}
/>
</TagName>
);
Expand Down
39 changes: 28 additions & 11 deletions assets/js/atomic/blocks/product-elements/title/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
BLOCK_ICON as icon,
BLOCK_DESCRIPTION as description,
} from './constants';
import { Save } from './save';

const blockConfig: BlockConfiguration = {
...sharedConfig,
Expand All @@ -24,17 +25,33 @@ const blockConfig: BlockConfiguration = {
icon: { src: icon },
attributes,
edit,
supports: isFeaturePluginBuild()
? {
html: false,
color: {
background: false,
},
typography: {
fontSize: true,
},
}
: sharedConfig.supports,
save: Save,
supports: {
...( isFeaturePluginBuild() && {
typography: {
fontSize: true,
lineHeight: true,
__experimentalFontWeight: true,
__experimentalTextTransform: true,
__experimentalFontFamily: true,
},
} ),
...( isFeaturePluginBuild() && {
color: {
text: true,
background: true,
link: false,
gradients: true,
__experimentalSkipSerialization: true,
},
} ),
...( isFeaturePluginBuild() && {
spacing: {
margin: true,
__experimentalSkipSerialization: true,
},
} ),
},
};

registerBlockType( 'woocommerce/product-title', blockConfig );
21 changes: 21 additions & 0 deletions assets/js/atomic/blocks/product-elements/title/save.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* External dependencies
*/
import { useBlockProps } from '@wordpress/block-editor';
import classnames from 'classnames';

type Props = {
attributes: Record< string, unknown > & {
className?: string;
};
};

export const Save = ( { attributes }: Props ): JSX.Element => {
return (
<div
{ ...useBlockProps.save( {
className: classnames( 'is-loading', attributes.className ),
} ) }
/>
);
};
9 changes: 9 additions & 0 deletions assets/js/atomic/blocks/product-elements/title/style.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.wp-block-woocommerce-product-title {
width: fit-content;
}

.wc-block-components-product-title {
margin-top: 0;
margin-bottom: $gap-small;
Expand All @@ -10,6 +14,11 @@
font-size: inherit;
display: block;
}

.wc-block-components-product-name {
color: inherit;
}

.is-loading {
.wc-block-components-product-title::before {
@include placeholder();
Expand Down
123 changes: 123 additions & 0 deletions assets/js/utils/style-attributes-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/* eslint-disable @wordpress/no-unsafe-wp-apis */
/**
* External dependencies
*/
import { __experimentalUseColorProps } from '@wordpress/block-editor';

/**
* Internal dependencies
*/
import { isFeaturePluginBuild } from '../settings/blocks/feature-flags';
import { isString, isObject } from '../types/type-guards';

type WithClass = {
className: string;
};

type WithStyle = {
style: Record< string, unknown >;
};

const parseStyle = ( style: unknown ): Record< string, unknown > => {
if ( isString( style ) ) {
return JSON.parse( style ) || {};
}

if ( isObject( style ) ) {
return style;
}

return {};
};

const getSpacingStyleInline = (
key: string,
values: {
top: string | null;
right: string | null;
bottom: string | null;
left: string | null;
}
) => {
return {
...( isString( values.top ) && { [ `${ key }Top` ]: values.top } ),
...( isString( values.right ) && {
[ `${ key }Right` ]: values.right,
} ),
...( isString( values.bottom ) && {
[ `${ key }Bottom` ]: values.bottom,
} ),
...( isString( values.left ) && { [ `${ key }Left` ]: values.left } ),
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 use key: value here then remove the null properties? We can even return style object with null properties because they will be omitted during render. Using spread here is quite hard to read IMO.

};
};

const parseSpacingStyle = (
spacing: Record< string, unknown >
): Record< string, unknown > => {
const keys = [ 'margin' ];

const getValueOrDefault = ( value: unknown ) => {
return isString( value ) && value.length > 0 ? value : null;
};

return Object.keys( spacing ).reduce( ( acc, key ) => {
const spacingProperty = isObject( spacing[ key ] )
? ( spacing[ key ] as Record< string, unknown > )
: {};

if ( keys.includes( key ) ) {
return {
...acc,
...getSpacingStyleInline( key, {
top: getValueOrDefault( spacingProperty.top ),
right: getValueOrDefault( spacingProperty.right ),
bottom: getValueOrDefault( spacingProperty.bottom ),
left: getValueOrDefault( spacingProperty.left ),
} ),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
...getSpacingStyleInline( key, {
top: getValueOrDefault( spacingProperty.top ),
right: getValueOrDefault( spacingProperty.right ),
bottom: getValueOrDefault( spacingProperty.bottom ),
left: getValueOrDefault( spacingProperty.left ),
} ),
...getSpacingStyleInline( key, spacingProperty ),

Copy link
Member

Choose a reason for hiding this comment

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

I think we can pass spacingProperty as the second argument then handle sides there instead. It makes the callback look cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can pass spacingProperty as the second argument then handle sides there instead. It makes the callback look cleaner.

It can be an idea, but to be honest I prefer to create a more strict typed code.

image

With this code (so without type), we lose some information about how the function getSpacingStyleInline works.
I mean without an object with top, right, bottom, and left key, it doesn't make sense invoke getSpacingStyleInlinefunction. With the typeRecord<string, unknown>` we lose this information and the developer is forced to read the whole function to understand what kind of data to pass.

But this is just my opinion, if you think that it is better to change it, I will do it :D

Copy link
Member

Choose a reason for hiding this comment

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

@gigitux With your example, I don't see the need for getSpacingStyleInline, we can add the inline style directly like this. Note that we can use undefined properties just fine because they will get omitted during render.

		if ( keys.includes( key ) ) {
			return {
				...acc,
				[ `${ key }Left` ]: spacingProperty.left,
				[ `${ key }Right` ]: spacingProperty.right,
				[ `${ key }Top` ]: spacingProperty.top,
				[ `${ key }Bottom` ]: spacingProperty.bottom,
			};
		}

};
}

return acc;
}, {} );
};

export const useSpacingProps = ( attributes: unknown ): WithStyle => {
const style = isObject( attributes ) ? parseStyle( attributes.style ) : {};
const spacingStyles = isObject( style.spacing ) ? style.spacing : {};

return {
style: parseSpacingStyle( spacingStyles ),
};
};

export const useTypographyProps = ( attributes: unknown ): WithStyle => {
const attributesObject = isObject( attributes ) ? attributes : {};
const style = parseStyle( attributesObject.style );
const typography = isObject( style.typography )
? ( style.typography as Record< string, unknown > )
: {};

return {
style: {
fontSize: attributesObject.fontSize || typography.fontSize,
lineHeight: typography.lineHeight,
fontWeight: typography.fontWeight,
textTransform: typography.textTransform,
fontFamily: attributesObject.fontFamily,
},
};
};

export const useColorProps = ( attributes: unknown ): WithStyle & WithClass => {
dinhtungdu marked this conversation as resolved.
Show resolved Hide resolved
if ( ! isFeaturePluginBuild() ) {
return {
className: '',
style: {},
};
}

const attributesObject = isObject( attributes ) ? attributes : {};
const style = parseStyle( attributesObject.style );

return __experimentalUseColorProps( { ...attributesObject, style } );
};