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

[BBT-76] Schema UI - Border Component #18

Merged
merged 18 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 11 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
51 changes: 46 additions & 5 deletions src/editor/components/fields/ThemerComponent.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { mergeWith, isEmpty } from 'lodash';
import { Button, Spinner, TabPanel } from '@wordpress/components';
import { Button, Spinner } from '@wordpress/components';
import { useSelect, dispatch } from '@wordpress/data';
import { useEffect, useState, useMemo } from '@wordpress/element';
import apiFetch from '@wordpress/api-fetch';

import schemaComponents from '../../ui/utils/schema-to-components';
import EditorContext from '../../context/EditorContext';
import StylesContext from '../../context/StylesContext';

import Preview from './Preview';
import Fields from './Fields';
import ResponsiveButton from './ResponsiveButton';
import ButtonExport from '../ButtonExport';

Expand All @@ -16,6 +19,15 @@ const ThemerComponent = () => {
const [ previewCss, setPreviewCss ] = useState( '' );
const [ previewSize, setPreviewSize ] = useState();

const setUserConfig = ( config ) => {
dispatch( 'core' ).editEntityRecord(
'root',
'globalStyles',
globalStylesId,
config
);
};

const { globalStylesId, baseConfig, userConfig } = useSelect(
( select ) => {
const {
Expand Down Expand Up @@ -73,7 +85,7 @@ const ThemerComponent = () => {
* saves edited entity data
*/
const save = async () => {
dispatch( 'core' ).undo();
// dispatch( 'core' ).undo();
Copy link
Member

Choose a reason for hiding this comment

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

Is this no longer required? If so should we remove it

Copy link
Contributor

@chrishbite chrishbite Aug 7, 2023

Choose a reason for hiding this comment

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

I have removed this, I believe this was required originally when updating but with the new components, on a Save and page refresh the settings would revert back one edit in the history.

const save = async () => {
try {
await dispatch( 'core' ).saveEditedEntityRecord(

try {
await dispatch( 'core' ).saveEditedEntityRecord(
'root',
Expand All @@ -98,6 +110,18 @@ const ThemerComponent = () => {
);
};

/* TODO: refactor */
Copy link
Member

Choose a reason for hiding this comment

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

does this need a refactor? can we add a note explaining why, or preferably refactor now OR remove the comment

Copy link
Contributor

@chrishbite chrishbite Aug 7, 2023

Choose a reason for hiding this comment

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

So this is related to initialising the component map and is not really related to the building of the components themselves, was needed only to get the demo working for a component.

This would need to be refactored and moved completely out of this location and incorporated into the loading state of the full application based around the fetching and parsing of the external schema file.

const [ components, setComponents ] = useState( {} );
useEffect( () => {
const generate = async () => {
const mappedComponents = await schemaComponents();
setComponents( mappedComponents );
};

generate();
}, [] );
const { border: Border } = components?.styles ?? {};

if ( ! themeConfig || ! previewCss ) {
return (
<>
Expand All @@ -108,13 +132,30 @@ const ThemerComponent = () => {

return (
<>
<style>{ previewCss }</style>
Copy link
Member

Choose a reason for hiding this comment

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

this could cause styles to bleed into unexpected areas, since some of the CSS rules are scoped to body and designed to be used within a block editor instance.

I can see a small strip of background colour at the bottom of the page for example:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Good spot, I've ended up removing this as the issue I had originally doesn't seem to be reoccurring with it removed, colours in the Border component look to be working correctly.

return (
<>
<div className="themer-topbar">
<Button isSecondary onClick={ () => reset() } text="Reset" />

<div className="themer-topbar">
<Button isSecondary onClick={ () => reset() } text="Reset" />
<Button isPrimary onClick={ () => save() } text="Save" />
</div>
<div className="themer-body">
<div className="themer-nav-container">
<TabPanel
{ /* demo */ }
<EditorContext.Provider
value={ {
globalStylesId,
themeConfig,
} }
>
<StylesContext.Provider
value={ {
setUserConfig,
} }
>
Copy link
Member

Choose a reason for hiding this comment

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

Should we move the contexts to the outermost wrapper of this component?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've moved the context to the top of this component in 4076d63

<Border selector="styles.blocks.core/pullquote.border" />
</StylesContext.Provider>
</EditorContext.Provider>
{ /* demo */ }
{ /* <TabPanel
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should render the demo UI component inside the first tab, rather than comment out all of this area?

Copy link
Contributor

Choose a reason for hiding this comment

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

Restored the TabPanel and Fields, moved Border demo component into TabPanel in 4076d63

className="themer-tab-panel"
activeClass="active-themer-tab"
tabs={ [
Expand All @@ -141,7 +182,7 @@ const ThemerComponent = () => {
<Fields sourceObject={ themeConfig } />
</>
) }
</TabPanel>
</TabPanel> */ }
</div>
<div className="themer-preview-container">
<ResponsiveButton
Expand Down
6 changes: 6 additions & 0 deletions src/editor/context/EditorContext.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { createContext } from '@wordpress/element';

/**
* Sets a default context for use as state management across the plugin
*/
export default createContext();
6 changes: 6 additions & 0 deletions src/editor/context/StylesContext.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { createContext } from '@wordpress/element';

/**
* Sets a default context for use as state management across the plugin
*/
export default createContext();
50 changes: 50 additions & 0 deletions src/editor/ui/components/styles/Border.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/* eslint-disable @wordpress/no-unsafe-wp-apis */

import { set } from 'lodash';
import { __ } from '@wordpress/i18n';
import { useContext, useState, useEffect } from '@wordpress/element';
import { __experimentalBorderBoxControl as BorderBoxControl } from '@wordpress/components';

import getThemeOption from '../../utils/get-theme-option';
import EditorContext from '../../../context/EditorContext';
import StylesContext from '../../../context/StylesContext';

/**
* Reusable border control style component
*
* @param {Object} props Component props
* @param {string} props.selector Property target selector
*/
const Border = ( { selector } ) => {
const { themeConfig } = useContext( EditorContext );
const { setUserConfig } = useContext( StylesContext );
const value = getThemeOption( selector, themeConfig );
const colors = getThemeOption(
'settings.color.palette.theme',
themeConfig
);
const [ borders, setBorders ] = useState( value );

const onChange = ( newValue ) => {
setBorders( newValue );
};

useEffect( () => {
let config = structuredClone( themeConfig );
config = set( config, selector, {} );
config = set( config, selector, borders );

setUserConfig( config );
}, [ borders ] );
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 that themeConfig, setUserConfig and selector should be in the dependency array here - https://react.dev/learn/removing-effect-dependencies#dependencies-should-match-the-code

Copy link
Contributor

@chrishbite chrishbite Aug 7, 2023

Choose a reason for hiding this comment

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

Hey @g-elwell @scottblackburn I've refactored the Border component internals to avoid using useState so the useEffect is no longer needed and will resolve the above comment.

useState was originally used here to avoid a TOCTOU race condition, this seems to have corrected itself with the movement of the setUserConfig function to a context provider.

I also just spotted a bug with using useState internally, as the when the global Reset button is pressed although the editEntityRecord would be updated this would not be reflected on these individual components and would need a complete page refresh or React reinitialisation.


return (
<BorderBoxControl
colors={ colors }
label={ __( 'Borders', 'themer' ) }
onChange={ onChange }
value={ borders }
/>
);
};

export default Border;
13 changes: 13 additions & 0 deletions src/editor/ui/utils/component-map.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Border from '../components/styles/Border';

/**
* Style properties and their corresponding React components
*/
export const styleComponentMap = {
border: Border,
};

/**
* Settings properties and their corresponding React components
*/
export const settingComponentMap = {};
14 changes: 14 additions & 0 deletions src/editor/ui/utils/get-theme-option.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Returns the value of the specified selector from the base object
*
* @param {string} selector Property target selector
* @param {Object} base Theme settings
*
* @return {*} Value of selector
*/
const getThemeOption = ( selector, base ) => {
const selectorArray = selector.split( '.' );
return selectorArray.reduce( ( acc, curr ) => acc[ curr ], base );
};

export default getThemeOption;
57 changes: 57 additions & 0 deletions src/editor/ui/utils/schema-to-components.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { styleComponentMap } from './component-map';

/**
* Maps style properties to React components
*
* @param {Object} components Theme to React component map
* @param {Object} properties Theme style allowed properties
*/
const generateStyleComponents = ( components, properties ) => {
if ( ! properties ) {
return;
}

for ( const property in properties ) {
const style = styleComponentMap?.[ property ];

if ( ! style ) {
continue;
}

components.styles[ property ] = style;
}
};

/**
* Generates React components for parts of the theme.json schema
*
* @return {Object} Mapped components
*/
const schemaComponents = async () => {
const url =
'https://github.com/raw/WordPress/gutenberg/trunk/schemas/json/theme.json';

try {
const response = await fetch( url );
const components = { settings: {}, styles: {} };

if ( ! response?.ok ) {
throw new Error(
`${ response?.status } ${ response?.statusText }`
);
}

const schema = await response.json();

generateStyleComponents(
components,
schema?.definitions?.stylesProperties?.properties
);

return components;
} catch ( error ) {
console.error( error ); // eslint-disable-line no-console -- Output of caught error
}
};

export default schemaComponents;