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

Component System: Upgrade FontSizePicker #27594

Merged
merged 7 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
525 changes: 496 additions & 29 deletions package-lock.json

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"npm": ">=6.9.0"
},
"config": {
"GUTENBERG_PHASE": 2
"GUTENBERG_PHASE": 2,
"COMPONENT_SYSTEM_PHASE": 0
Copy link
Contributor

@youknowriad youknowriad Dec 10, 2020

Choose a reason for hiding this comment

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

Why is this flag used for? Is't it redundunt with the "context" component provider?

Copy link
Member

@gziolo gziolo Dec 10, 2020

Choose a reason for hiding this comment

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

We want to merge this PR with all the new code removed with Tree Shaking (100kb as of today according to the CI job) when running npm run build. This way we can integrate a few components, remove code duplication wherever possible and enable this flag (or remove completely) afterward.

So this is something that would be only enabled on demand. I think we can enable it by default on Storybook to have the place to preview migrated components.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of merging components and not really use them in the plugin?

Copy link
Member

Choose a reason for hiding this comment

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

I can also confirm now that Tree Shaking works like a charm:
Screen Shot 2020-12-10 at 09 18 48

🎉

},
"dependencies": {
"@wordpress/a11y": "file:packages/a11y",
Expand Down
24 changes: 13 additions & 11 deletions packages/block-editor/src/hooks/font-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,6 @@ export function FontSizeEdit( props ) {
const isDisabled = useIsFontSizeDisabled( props );
const fontSizes = useEditorFeature( 'typography.fontSizes' );

if ( isDisabled ) {
return null;
}

const fontSizeObject = getFontSize(
fontSizes,
fontSize,
style?.typography?.fontSize
);
const onChange = ( value ) => {
const fontSizeSlug = getFontSizeObjectByValue( fontSizes, value ).slug;

Expand All @@ -129,9 +120,20 @@ export function FontSizeEdit( props ) {
} );
};

return (
<FontSizePicker value={ fontSizeObject.size } onChange={ onChange } />
if ( isDisabled ) {
return null;
}

const fontSizeObject = getFontSize(
fontSizes,
fontSize,
style?.typography?.fontSize
);

const fontSizeValue =
fontSizeObject?.size || style?.typography?.fontSize || fontSize;

return <FontSizePicker onChange={ onChange } value={ fontSizeValue } />;
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 mapping necessary here? Wouldn't it be something to put inside the adapter function?

Copy link
Author

Choose a reason for hiding this comment

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

Is this mapping necessary here?

@gziolo I believe so. style.typography.fontSize is very specific to the hooks/ part of the editor. After testing, it appears that fontSizeValue my change depending on values defined in the theme.json setup. If that's correct, the prop "remap" feels correct in this hooks/ area (vs. in the components package).

}

/**
Expand Down
22 changes: 16 additions & 6 deletions packages/block-editor/src/hooks/typography.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
* WordPress dependencies
*/
import { hasBlockSupport } from '@wordpress/blocks';
import { PanelBody } from '@wordpress/components';
/**
* External dependencies
*/
import {
PanelBody,
__unstableComponentSystemProvider as ComponentSystemProvider,
} from '@wordpress/components';
import { Platform } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

Expand Down Expand Up @@ -61,11 +67,15 @@ export function TypographyPanel( props ) {
return (
<InspectorControls>
<PanelBody title={ __( 'Typography' ) }>
<FontFamilyEdit { ...props } />
<FontSizeEdit { ...props } />
<FontAppearanceEdit { ...props } />
<LineHeightEdit { ...props } />
<TextDecorationAndTransformEdit { ...props } />
<ComponentSystemProvider
__unstableNextInclude={ [ 'WPComponentsFontSizePicker' ] }
>
<FontFamilyEdit { ...props } />
<FontSizeEdit { ...props } />
<FontAppearanceEdit { ...props } />
<LineHeightEdit { ...props } />
<TextDecorationAndTransformEdit { ...props } />
</ComponentSystemProvider>
</PanelBody>
</InspectorControls>
);
Expand Down
6 changes: 5 additions & 1 deletion packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
"@wordpress/primitives": "file:../primitives",
"@wordpress/rich-text": "file:../rich-text",
"@wordpress/warning": "file:../warning",
"@wp-g2/components": "^0.0.139",
"@wp-g2/context": "^0.0.139",
"@wp-g2/styles": "^0.0.139",
"@wp-g2/utils": "^0.0.139",
"classnames": "^2.2.5",
"dom-scroll-into-view": "^1.2.1",
"downshift": "^5.4.0",
Expand All @@ -57,7 +61,7 @@
"react-merge-refs": "^1.0.0",
"react-resize-aware": "^3.0.1",
"react-spring": "^8.0.20",
"react-use-gesture": "^7.0.15",
"react-use-gesture": "^7.0.16",
"reakit": "^1.3.4",
"rememo": "^3.0.0",
"tinycolor2": "^1.4.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* External dependencies
*/
import { ContextSystemProvider } from '@wp-g2/components';

export function ComponentSystemProvider( {
__unstableNextInclude = [],
children,
value = {},
} ) {
if ( process.env.COMPONENT_SYSTEM_PHASE === 1 ) {
const contextValue = { ...value };

__unstableNextInclude.forEach( ( namespace ) => {
const baseValue = contextValue[ namespace ] || {};
contextValue[ namespace ] = {
...baseValue,
__unstableVersion: 'next',
};
} );

return (
<ContextSystemProvider value={ contextValue }>
{ children }
</ContextSystemProvider>
);
}

return children;
}
2 changes: 2 additions & 0 deletions packages/components/src/__next/context/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { ComponentSystemProvider as __unstableComponentSystemProvider } from './component-system-provider';
export { withNext as __unstableWithNext } from './with-next';
35 changes: 35 additions & 0 deletions packages/components/src/__next/context/with-next.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* External dependencies
*/
import { contextConnect, useContextSystem } from '@wp-g2/components';
/**
* WordPress dependencies
*/
import { forwardRef } from '@wordpress/element';

export function withNext(
CurrentComponent = () => null,
NextComponent = () => null,
namespace = 'Component',
adapter = ( p ) => p
) {
if ( process.env.COMPONENT_SYSTEM_PHASE === 1 ) {
const WrappedComponent = ( props, ref ) => {
const { __unstableVersion, ...otherProps } = useContextSystem(
props,
namespace
);

if ( __unstableVersion === 'next' ) {
const nextProps = adapter( otherProps );
return <NextComponent { ...nextProps } ref={ ref } />;
}

return <CurrentComponent { ...props } ref={ ref } />;
};

return contextConnect( WrappedComponent, namespace );
}

return forwardRef( CurrentComponent );
}
27 changes: 18 additions & 9 deletions packages/components/src/font-size-picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Button from '../button';
import RangeControl from '../range-control';
import CustomSelectControl from '../custom-select-control';
import VisuallyHidden from '../visually-hidden';
import { withNextComponent } from './next/';

const DEFAULT_FONT_SIZE = 'default';
const CUSTOM_FONT_SIZE = 'custom';
Expand Down Expand Up @@ -52,14 +53,17 @@ function getSelectOptions( optionsArray, disableCustomFontSizes ) {
} ) );
}

export default function FontSizePicker( {
fallbackFontSize,
fontSizes = [],
disableCustomFontSizes = false,
onChange,
value,
withSlider = false,
} ) {
function FontSizePicker(
{
fallbackFontSize,
fontSizes = [],
disableCustomFontSizes = false,
onChange,
value,
withSlider = false,
},
ref
gziolo marked this conversation as resolved.
Show resolved Hide resolved
) {
const hasUnits =
isString( value ) ||
( fontSizes[ 0 ] && isString( fontSizes[ 0 ].size ) );
Expand Down Expand Up @@ -90,7 +94,10 @@ export default function FontSizePicker( {
const fontSizePickerNumberId = `components-font-size-picker__number#${ instanceId }`;

return (
<fieldset className="components-font-size-picker">
<fieldset
className="components-font-size-picker"
{ ...( ref ? {} : { ref } ) }
>
<VisuallyHidden as="legend">{ __( 'Font size' ) }</VisuallyHidden>
<div className="components-font-size-picker__controls">
{ fontSizes.length > 0 && (
Expand Down Expand Up @@ -169,3 +176,5 @@ export default function FontSizePicker( {
</fieldset>
);
}

export default withNextComponent( FontSizePicker );
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* External dependencies
*/
import { contextConnect, useContextSystem } from '@wp-g2/context';
import { noop } from '@wp-g2/utils';
import {
Grid,
TextInput,
SelectDropdown,
FormGroup,
Button,
View,
} from '@wp-g2/components';

/**
* Internal dependencies
*/
import { getSelectTemplateColumns } from './font-size-control-utils';

function renderItem( { name, style } ) {
return <span style={ style }>{ name }</span>;
}

function FontSizeControlSelect( props, forwardedRef ) {
const {
customLabel,
disabled,
inputValue,
isDefaultValue,
label,
max,
min,
onChange = noop,
onInputChange = noop,
onReset = noop,
options = [],
size,
value,
withNumberInput,
withSelect,
...otherProps
} = useContextSystem( props, 'FontSizeControlSelect' );

const templateColumns = getSelectTemplateColumns( withNumberInput );
const subControlsTemplateColumns = withNumberInput ? '1fr 1fr' : '1fr';

return (
<Grid alignment="bottom" templateColumns={ templateColumns }>
{ withSelect && (
<FormGroup label={ label }>
<SelectDropdown
disabled={ disabled }
max={ 260 }
onChange={ onChange }
options={ options }
renderItem={ renderItem }
ref={ forwardedRef }
size={ size }
value={ value }
{ ...otherProps }
/>
</FormGroup>
) }
<Grid
alignment="bottom"
templateColumns={ subControlsTemplateColumns }
>
{ withNumberInput && (
<FormGroup label={ customLabel }>
<TextInput
disabled={ disabled }
max={ max }
min={ min }
onChange={ onInputChange }
size={ size }
type="number"
value={ inputValue }
/>
</FormGroup>
) }
<View>
<Button
disabled={ isDefaultValue }
isBlock
onClick={ onReset }
>
{ __( 'Reset' ) }
</Button>
</View>
</Grid>
</Grid>
);
}

export default contextConnect( FontSizeControlSelect, 'FontSizeControlSelect' );
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
/**
* External dependencies
*/
import { noop } from '@wp-g2/utils';

/**
* Internal dependencies
*/
import {
ControlLabel,
Grid,
Slider,
TextInput,
VStack,
} from '@wp-g2/components';
import { getSliderTemplateColumns } from './font-size-control-utils';

function FontSizeControlSlider( props ) {
const {
label = __( 'Custom size' ),
disabled,
min,
max,
onChange = noop,
size,
value,
withSlider,
} = props;

if ( ! withSlider ) return null;

const templateColumns = getSliderTemplateColumns();

const controlProps = {
disabled,
min,
max,
onChange,
size,
value,
};

return (
<VStack spacing={ 0 }>
<ControlLabel>{ label }</ControlLabel>
<Grid templateColumns={ templateColumns }>
<Slider { ...controlProps } />
<TextInput { ...controlProps } type="number" />
</Grid>
</VStack>
);
}

export default FontSizeControlSlider;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* External dependencies
*/
import { css } from '@wp-g2/styles';

export const FontSizeControl = css`
appearance: none;
border: none;
margin: 0;
padding: 0;
`;
Loading