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

Paragraph: Add LineHeightControl + Attribute #20775

Merged
merged 30 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b9c4827
Update emotion library to latest. Adds to block-library package.
Mar 9, 2020
58d57f7
Add LineHeightControl component to block-editor
Mar 9, 2020
d7fba5e
Export LineHeightControl as experimental from block-editor
Mar 9, 2020
9319076
Hook up LineHeightControl with core Paragraph block
Mar 9, 2020
38c4404
Improve undefined/reset interactions for line-height attribute
Mar 9, 2020
7867960
Refactor store hook usage with LineHeightControl
Mar 9, 2020
bb61179
Refactor with withLineHeight HOC for Edit and Save components
Mar 9, 2020
0facc59
Improve reset logic. Simplifies stlye value output.
Mar 9, 2020
601a1f2
Refactor function names and comments
Mar 10, 2020
d455c17
Fix import/export of new experimental attribute hooks
Mar 10, 2020
ce678af
Export LineHeightControls and withLineHeight for native
Mar 11, 2020
499d59e
Adjust mechanism to get block attributes in useBlockAttributes hook
Mar 11, 2020
c329f21
Add theme support for disable-custom-line-height
Mar 11, 2020
2d6beb4
Clarify logic + add comments for lineHeight attribute -> value evalua…
Mar 12, 2020
2e99301
Simplify CSS output for lineHeight in paragraph block
Mar 12, 2020
864e8ea
Add experimental prefix for disableCustomLineHeight setting
Mar 12, 2020
cc18a2b
Add comment for :root based CSS rule
Mar 16, 2020
0838ee0
Removes @emotion from block-library. Refactor to use scss
Mar 16, 2020
002e753
Remove withLineHeight HOC abstraction.
Mar 17, 2020
3bd0457
Revert className implementation
Mar 17, 2020
bda4f0e
Revert @emotion/native from 10.0.23 -> 10.0.22 (to match master)
Mar 17, 2020
5900583
Updates package-lock.json
Mar 18, 2020
35794cc
Fix e2e test to target correct text input
Mar 18, 2020
bf7ef83
Reverting package-lock.json
Mar 19, 2020
96d4158
Provide clientId in dependency array in useSetAttributes hook
Mar 19, 2020
5f43e25
Use CSS variables and a support flag for the line height customization
youknowriad Mar 25, 2020
c449aad
Improves handling of "zero" key press
Mar 25, 2020
5923ffc
Update packages/block-editor/src/components/line-height-control/utils.js
Mar 25, 2020
777f079
Remove unused import
Mar 25, 2020
a129ab8
Remove useless hooks file
youknowriad Mar 27, 2020
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
13 changes: 13 additions & 0 deletions lib/client-assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,19 @@ function gutenberg_extend_settings_block_patterns( $settings ) {
}
add_filter( 'block_editor_settings', 'gutenberg_extend_settings_block_patterns', 0 );

/**
* Extends block editor settings to determine whether to use custom line height controls.
* Currently experimental.
*
* @param array $settings Default editor settings.
*
* @return array Filtered editor settings.
*/
function gutenberg_extend_settings_custom_line_height( $settings ) {
$settings['__experimentalDisableCustomLineHeight'] = get_theme_support( 'disable-custom-line-height' );
return $settings;
}
add_filter( 'block_editor_settings', 'gutenberg_extend_settings_custom_line_height' );

/*
* Register default patterns if not registered in Core already.
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export { default as InnerBlocks } from './inner-blocks';
export { default as InspectorAdvancedControls } from './inspector-advanced-controls';
export { default as InspectorControls } from './inspector-controls';
export { default as __experimentalLinkControl } from './link-control';
export { default as __experimentalLineHeightControl } from './line-height-control';
export { default as MediaReplaceFlow } from './media-replace-flow';
export { default as MediaPlaceholder } from './media-placeholder';
export { default as MediaUpload } from './media-upload';
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export * from './font-sizes';
export { default as AlignmentToolbar } from './alignment-toolbar';
export { default as InnerBlocks } from './inner-blocks';
export { default as InspectorControls } from './inspector-controls';
export { default as __experimentalLineHeightControl } from './line-height-control';
export { default as PlainText } from './plain-text';
export {
default as RichText,
Expand Down
88 changes: 88 additions & 0 deletions packages/block-editor/src/components/line-height-control/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { TextControl } from '@wordpress/components';
import { ZERO } from '@wordpress/keycodes';

/**
* Internal dependencies
*/
import {
BASE_DEFAULT_VALUE,
RESET_VALUE,
STEP,
useIsLineHeightControlsDisabled,
isLineHeightDefined,
} from './utils';

export default function LineHeightControl( { value: lineHeight, onChange } ) {
const isDisabled = useIsLineHeightControlsDisabled();
const isDefined = isLineHeightDefined( lineHeight );

// Don't render the controls if disabled by editor settings
if ( isDisabled ) {
return null;
}

const handleOnKeyDown = ( event ) => {
const { keyCode } = event;

if ( keyCode === ZERO && ! isDefined ) {
/**
* Prevents the onChange callback from firing, which prevents
* the logic from assuming the change was triggered from
* an input arrow CLICK.
*/
event.preventDefault();
onChange( '0' );
}
};

const handleOnChange = ( nextValue ) => {
// Set the next value without modification if lineHeight has been defined
if ( isDefined ) {
onChange( nextValue );
return;
}

// Otherwise...
/**
* The following logic handles the initial up/down arrow CLICK of the
* input element. This is so that the next values (from an undefined value state)
* are more better suited for line-height rendering.
*/
let adjustedNextValue = nextValue;

switch ( nextValue ) {
case `${ STEP }`:
// Increment by step value
adjustedNextValue = BASE_DEFAULT_VALUE + STEP;
break;
case '0':
Copy link
Member

Choose a reason for hiding this comment

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

If I write a value of 0 directly we assume right away another value as if the user was decreasing the value.
If I write a value of 0.1 directly we assume right away another value as if the user was increasing the value.

Maybe in the event that is triggered, we can check if the controls up/down are being used instead of using the value as a way to check?

Copy link
Author

Choose a reason for hiding this comment

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

Oh, good call. This one is tricky. We can capture UP/DOWN on keyboard, but not so much clicking up/down in the input control.

Investigating...

Copy link
Author

Choose a reason for hiding this comment

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

@jorgefilipecosta Haii! I just pushed a change to account for this logic.

Typing in 0 (zero) has now been accounted for.

Some good news is, the user can't physically trigger the 0.1 case by typing. This flow can only be triggered by either pressing UP or clicking the up arrow in the input.

// Decrement by step value
adjustedNextValue = BASE_DEFAULT_VALUE - STEP;
break;
}

onChange( adjustedNextValue );
};

const value = isDefined ? lineHeight : RESET_VALUE;

return (
<div className="block-editor-line-height-control">
<TextControl
autoComplete="off"
onKeyDown={ handleOnKeyDown }
onChange={ handleOnChange }
label={ __( 'Line height' ) }
placeholder={ BASE_DEFAULT_VALUE }
step={ STEP }
type="number"
value={ value }
min={ 0 }
/>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.block-editor-line-height-control {
margin-bottom: 24px;

input {
display: block;
max-width: 60px;
}
}
44 changes: 44 additions & 0 deletions packages/block-editor/src/components/line-height-control/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';

export const BASE_DEFAULT_VALUE = 1.5;
export const STEP = 0.1;
/**
* There are varying value types within LineHeightControl:
*
* {undefined} Initial value. No changes from the user.
* {string} Input value. Value consumed/outputted by the input. Empty would be ''.
* {number} Block attribute type. Input value needs to be converted for attribute setting.
*
* Note: If the value is undefined, the input requires it to be an empty string ('')
* in order to be considered "controlled" by props (rather than internal state).
*/
export const RESET_VALUE = '';

/**
* Retrieves whether custom lineHeight controls should be disabled from editor settings.
*
* @return {boolean} Whether lineHeight controls should be disabled.
*/
export function useIsLineHeightControlsDisabled() {
const isDisabled = useSelect( ( select ) => {
const { getSettings } = select( 'core/block-editor' );

return !! getSettings().__experimentalDisableCustomLineHeight;
}, [] );

return isDisabled;
}

/**
* Determines if the lineHeight attribute has been properly defined.
*
* @param {any} lineHeight The value to check.
*
* @return {boolean} Whether the lineHeight attribute is valid.
*/
export function isLineHeightDefined( lineHeight ) {
return lineHeight !== undefined && lineHeight !== RESET_VALUE;
}
15 changes: 1 addition & 14 deletions packages/block-editor/src/hooks/color.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* External dependencies
*/
import classnames from 'classnames';
import { pickBy, isEqual, isObject, identity, mapValues } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -26,22 +25,10 @@ import PanelColorSettings from '../components/panel-color-settings';
import ContrastChecker from '../components/contrast-checker';
import InspectorControls from '../components/inspector-controls';
import { getBlockDOMNode } from '../utils/dom';
import { cleanEmptyObject } from './utils';

export const COLOR_SUPPORT_KEY = '__experimentalColor';

export const cleanEmptyObject = ( object ) => {
if ( ! isObject( object ) ) {
return object;
}
const cleanedNestedObjects = pickBy(
mapValues( object, cleanEmptyObject ),
identity
);
return isEqual( cleanedNestedObjects, {} )
? undefined
: cleanedNestedObjects;
};

/**
* Filters registered block settings, extending attributes to include
* `backgroundColor` and `textColor` attribute.
Expand Down
64 changes: 64 additions & 0 deletions packages/block-editor/src/hooks/line-height.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* WordPress dependencies
*/
import { addFilter } from '@wordpress/hooks';
import { hasBlockSupport } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
import { PanelBody } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import LineHeightControl from '../components/line-height-control';
import InspectorControls from '../components/inspector-controls';
import { cleanEmptyObject } from './utils';

export const LINE_HEIGHT_SUPPRT_KEY = '__experimentalLineHeight';

/**
* Override the default edit UI to include new inspector controls for block
* color, if block defines support.
*
* @param {Function} BlockEdit Original component
* @return {Function} Wrapped component
*/
export const withBlockControls = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const { name: blockName } = props;
if ( ! hasBlockSupport( blockName, LINE_HEIGHT_SUPPRT_KEY ) ) {
return <BlockEdit key="edit" { ...props } />;
}
const { style } = props.attributes;
const onChange = ( newLineHeightValue ) => {
const newStyle = {
...style,
typography: {
lineHeight: newLineHeightValue,
},
};
props.setAttributes( {
style: cleanEmptyObject( newStyle ),
} );
};

return [
<InspectorControls key="control">
<PanelBody title={ __( 'Typography' ) }>
<LineHeightControl
value={ style?.typography?.lineHeight }
onChange={ onChange }
/>
</PanelBody>
</InspectorControls>,
<BlockEdit key="edit" { ...props } />,
];
},
'withToolbarControls'
);

addFilter(
'editor.BlockEdit',
'core/color/with-block-controls',
withBlockControls
);
3 changes: 2 additions & 1 deletion packages/block-editor/src/hooks/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import { hasBlockSupport } from '@wordpress/blocks';
* Internal dependencies
*/
import { COLOR_SUPPORT_KEY } from './color';
import { LINE_HEIGHT_SUPPRT_KEY } from './line-height';

const styleSupportKeys = [ COLOR_SUPPORT_KEY ];
const styleSupportKeys = [ COLOR_SUPPORT_KEY, LINE_HEIGHT_SUPPRT_KEY ];

const hasStyleSupport = ( blockType ) =>
styleSupportKeys.some( ( key ) => hasBlockSupport( blockType, key ) );
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/test/color.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Internal dependencies
*/
import { cleanEmptyObject } from '../color';
import { cleanEmptyObject } from '../utils';

describe( 'cleanEmptyObject', () => {
it( 'should remove nested keys', () => {
Expand Down
23 changes: 23 additions & 0 deletions packages/block-editor/src/hooks/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* External dependencies
*/
import { pickBy, isEqual, isObject, identity, mapValues } from 'lodash';

/**
* Removed undefined values from nested object.
*
* @param {*} object
* @return {*} Object cleaned from undefined values
*/
export const cleanEmptyObject = ( object ) => {
if ( ! isObject( object ) ) {
return object;
}
const cleanedNestedObjects = pickBy(
mapValues( object, cleanEmptyObject ),
identity
);
return isEqual( cleanedNestedObjects, {} )
? undefined
: cleanedNestedObjects;
};
1 change: 1 addition & 0 deletions packages/block-editor/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
@import "./components/contrast-checker/style.scss";
@import "./components/default-block-appender/style.scss";
@import "./components/link-control/style.scss";
@import "./components/line-height-control/style.scss";
@import "./components/image-size-control/style.scss";
@import "./components/inner-blocks/style.scss";
@import "./components/inserter-list-item/style.scss";
Expand Down
5 changes: 4 additions & 1 deletion packages/block-library/src/paragraph/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
},
"direction": {
"type": "string",
"enum": [ "ltr", "rtl" ]
"enum": [
"ltr",
Copy link
Author

Choose a reason for hiding this comment

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

Prettier autofixed these ones

"rtl"
]
}
}
}
14 changes: 8 additions & 6 deletions packages/block-library/src/paragraph/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,19 @@ function ParagraphBlock( {
setAttributes,
setFontSize,
} ) {
const { align, content, dropCap, placeholder, direction } = attributes;
const { align, content, direction, dropCap, placeholder } = attributes;

const ref = useRef();
const dropCapMinimumHeight = useDropCapMinimumHeight( dropCap, [
fontSize.size,
] );

const styles = {
Copy link
Author

Choose a reason for hiding this comment

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

Created a variable from inline implementation

fontSize: fontSize.size ? `${ fontSize.size }px` : undefined,
direction,
minHeight: dropCapMinimumHeight,
};

return (
<>
<BlockControls>
Expand Down Expand Up @@ -132,11 +138,7 @@ function ParagraphBlock( {
[ `has-text-align-${ align }` ]: align,
[ fontSize.class ]: fontSize.class,
} ) }
style={ {
fontSize: fontSize.size ? fontSize.size + 'px' : undefined,
direction,
minHeight: dropCapMinimumHeight,
} }
style={ styles }
value={ content }
onChange={ ( newContent ) =>
setAttributes( { content: newContent } )
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/paragraph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const settings = {
__unstablePasteTextInline: true,
lightBlockWrapper: true,
__experimentalColor: true,
__experimentalLineHeight: true,
},
__experimentalLabel( attributes, { context } ) {
if ( context === 'accessibility' ) {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/paragraph/style.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
:root p {
background-color: var(--wp--color--background);
color: var(--wp--color--text);
line-height: var(--wp--typography--line-height);
}

.is-small-text {
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/editor/various/embedding.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ describe( 'Embedding content', () => {
await page.keyboard.type( 'Hello there!' );
await publishPost();
const postUrl = await page.$eval(
'[id^=inspector-text-control-]',
'.editor-post-publish-panel [id^=inspector-text-control-]',
Copy link
Member

Choose a reason for hiding this comment

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

This change does not seem related, what was the reason for it? Should we include a comment or maybe discuss it in another PR?

Copy link
Author

Choose a reason for hiding this comment

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

That's the interesting thing! For the longest time, this test was failing for this PR.

The reason for that is because the Paragraph block adds a TextControl component.
When this test $eval for an input, it finds the new LineHeightControl, not the publish URL input.

To resolve this, I made the test more specific to the Publish Panel

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for clarification @ItsJonQ!

( el ) => el.value
);

Expand Down
Loading