-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from 5 commits
49dc3b6
9ef47ce
93c697d
d6ef394
4f6a34e
4e28a34
fa706a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
||
|
@@ -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 } />; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@gziolo I believe so. |
||
} | ||
|
||
/** | ||
|
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; | ||
} |
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'; |
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 ); | ||
} |
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; | ||
`; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
🎉