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

LineHeightControl: Enhance interactions by migrating internals to NumberControl on Style options. #37160

Merged
merged 14 commits into from
Feb 16, 2022
4 changes: 4 additions & 0 deletions packages/block-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New Features

- `LineHeightControl`: Changes internal implementation to use `NumberControl`, which allows enhanced interactions such as dragging to change the value. To improve consistency with other control components, the bottom margin styles on the component has been deprecated, and will be removed in a future version. To opt into this simplified margin style, set the `__nextHasNoMarginBottom` prop to `true`.

## 8.1.1 (2022-02-10)

### Bug Fix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ _Note:_ It is worth noting that the line height setting option is an opt-in feat
Renders the markup for the line height setting option in the block inspector.

```jsx
import { KeyboardShortcuts } from '@wordpress/block-editor';
import { LineHeightControl } from '@wordpress/block-editor';
const MyLineHeightControl = () => (
<LineHeightControl value={ lineHeight } onChange={ onChange } />
<LineHeightControl
value={ lineHeight }
onChange={ onChange }
__nextHasNoMarginBottom={ true }
/>
);
```

Expand All @@ -38,6 +42,13 @@ The value of the line height.

A callback function that handles the application of the line height value.

#### `__nextHasNoMarginBottom`

- **Type:** `boolean`
- **Default:** `false`

Start opting into the new margin-free styles that will become the default in a future version, currently scheduled to be WordPress 6.4. (The prop can be safely removed once this happens.)

## Related components

Block Editor components are components that can be used to compose the UI of your block editor. Thus, they can only be used under a [`BlockEditorProvider`](https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-editor/src/components/provider/README.md) in the components tree.
103 changes: 63 additions & 40 deletions packages/block-editor/src/components/line-height-control/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/**
* WordPress dependencies
*/
import deprecated from '@wordpress/deprecated';
import { __ } from '@wordpress/i18n';
import { TextControl } from '@wordpress/components';
import { ZERO } from '@wordpress/keycodes';
import { __experimentalNumberControl as NumberControl } from '@wordpress/components';

/**
* Internal dependencies
Expand All @@ -15,64 +15,87 @@ import {
isLineHeightDefined,
} from './utils';

export default function LineHeightControl( { value: lineHeight, onChange } ) {
export default function LineHeightControl( {
value: lineHeight,
onChange,
/** Start opting into the new margin-free styles that will become the default in a future version. */
__nextHasNoMarginBottom = false,
__unstableInputWidth = '60px',
} ) {
const isDefined = isLineHeightDefined( lineHeight );

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 ) => {
const adjustNextValue = ( nextValue, wasTypedOrPasted ) => {
// Set the next value without modification if lineHeight has been defined
if ( isDefined ) {
onChange( nextValue );
return;
}
if ( isDefined ) return nextValue;

// 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.
* The following logic handles the initial step up/down action
* (from an undefined value state) so that the next values are better suited for
* line-height rendering. For example, the first step up should immediately
* go to 1.6, rather than the normally expected 0.1.
*
* Step up/down actions can be triggered by keydowns of the up/down arrow keys,
* or by clicking the spin buttons.
*/
let adjustedNextValue = nextValue;

switch ( nextValue ) {
case `${ STEP }`:
// Increment by step value
adjustedNextValue = BASE_DEFAULT_VALUE + STEP;
break;
case '0':
return BASE_DEFAULT_VALUE + STEP;
case '0': {
// This means the user explicitly input '0', rather than stepped down
// from an undefined value state
if ( wasTypedOrPasted ) return nextValue;
// Decrement by step value
adjustedNextValue = BASE_DEFAULT_VALUE - STEP;
break;
return BASE_DEFAULT_VALUE - STEP;
}
case '':
return BASE_DEFAULT_VALUE;
default:
return nextValue;
}
};

onChange( adjustedNextValue );
const stateReducer = ( state, action ) => {
// Be careful when changing this — cross-browser behavior of the
// `inputType` field in `input` events are inconsistent.
// For example, Firefox emits an input event with inputType="insertReplacementText"
// on spin button clicks, while other browsers do not even emit an input event.
const wasTypedOrPasted = [ 'insertText', 'insertFromPaste' ].includes(
action.payload.event.nativeEvent.inputType
);
state.value = adjustNextValue( state.value, wasTypedOrPasted );
return state;
};

const value = isDefined ? lineHeight : RESET_VALUE;

if ( ! __nextHasNoMarginBottom ) {
deprecated(
'Bottom margin styles for wp.blockEditor.LineHeightControl',
{
since: '6.0',
version: '6.4',
hint:
'Set the `__nextHasNoMarginBottom` prop to true to start opting into the new styles, which will become the default in a future version',
}
);
}
const deprecatedStyles = __nextHasNoMarginBottom
? undefined
: { marginBottom: 24 };
Comment on lines +83 to +85
Copy link
Member Author

Choose a reason for hiding this comment

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

I hope this is simple and ephemeral enough to put in here as an inline style. The @wordpress/block-editor package itself doesn't really use Emotion yet, and we can avoid a hardcoded classname here if we just do it inline.


return (
<div className="block-editor-line-height-control">
<TextControl
autoComplete="off"
onKeyDown={ handleOnKeyDown }
onChange={ handleOnChange }
<div
className="block-editor-line-height-control"
style={ deprecatedStyles }
>
<NumberControl
__unstableInputWidth={ __unstableInputWidth }
__unstableStateReducer={ stateReducer }
onChange={ onChange }
label={ __( 'Line height' ) }
placeholder={ BASE_DEFAULT_VALUE }
step={ STEP }
type="number"
value={ value }
min={ 0 }
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';

/**
* Internal dependencies
*/
import LineHeightControl from '../';

export default {
component: LineHeightControl,
title: 'BlockEditor/LineHeightControl',
};

const Template = ( props ) => {
const [ value, setValue ] = useState();
return (
<LineHeightControl onChange={ setValue } value={ value } { ...props } />
);
};

export const Default = Template.bind( {} );
Default.args = {
__nextHasNoMarginBottom: true,
__unstableInputWidth: '60px',
};

export const UnconstrainedWidth = Template.bind( {} );
UnconstrainedWidth.args = {
...Default.args,
__unstableInputWidth: '100%',
};

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* External dependencies
*/
import { fireEvent, render, screen } from '@testing-library/react';

/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';
import { UP, DOWN } from '@wordpress/keycodes';

/**
* Internal dependencies
*/
import LineHeightControl from '../';
import { BASE_DEFAULT_VALUE, STEP } from '../utils';

const ControlledLineHeightControl = () => {
const [ value, setValue ] = useState();
return (
<LineHeightControl
value={ value }
onChange={ setValue }
__nextHasNoMarginBottom={ true }
/>
);
};

describe( 'LineHeightControl', () => {
it( 'should immediately step up from the default value if up-arrowed from an unset state', () => {
render( <ControlledLineHeightControl /> );
const input = screen.getByRole( 'spinbutton' );
input.focus();
fireEvent.keyDown( input, { keyCode: UP } );
expect( input ).toHaveValue( BASE_DEFAULT_VALUE + STEP );
} );

it( 'should immediately step down from the default value if down-arrowed from an unset state', () => {
render( <ControlledLineHeightControl /> );
const input = screen.getByRole( 'spinbutton' );
input.focus();
fireEvent.keyDown( input, { keyCode: DOWN } );
expect( input ).toHaveValue( BASE_DEFAULT_VALUE - STEP );
} );

it( 'should immediately step up from the default value if spin button up was clicked from an unset state', () => {
render( <ControlledLineHeightControl /> );
const input = screen.getByRole( 'spinbutton' );
input.focus();
fireEvent.change( input, { target: { value: 0.1 } } ); // simulates click on spin button up
expect( input ).toHaveValue( BASE_DEFAULT_VALUE + STEP );
} );

it( 'should immediately step down from the default value if spin button down was clicked from an unset state', () => {
render( <ControlledLineHeightControl /> );
const input = screen.getByRole( 'spinbutton' );
input.focus();
fireEvent.change( input, { target: { value: 0 } } ); // simulates click on spin button down
expect( input ).toHaveValue( BASE_DEFAULT_VALUE - STEP );
} );
} );
2 changes: 2 additions & 0 deletions packages/block-editor/src/hooks/line-height.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export function LineHeightEdit( props ) {
};
return (
<LineHeightControl
__unstableInputWidth="100%"
__nextHasNoMarginBottom={ true }
ciampo marked this conversation as resolved.
Show resolved Hide resolved
value={ style?.typography?.lineHeight }
onChange={ onChange }
/>
Expand Down
4 changes: 0 additions & 4 deletions packages/block-editor/src/hooks/typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
margin-bottom: 0;
}

.block-editor-line-height-control input {
max-width: 100%;
}

.single-column {
grid-column: span 1;
}
Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
@import "./components/inner-blocks/style.scss";
@import "./components/inserter-list-item/style.scss";
@import "./components/justify-content-control/style.scss";
@import "./components/line-height-control/style.scss";
@import "./components/link-control/style.scss";
@import "./components/list-view/style.scss";
@import "./components/media-replace-flow/style.scss";
Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/input-control/reducer/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export const composeStateReducers = (
): StateReducer => {
return ( ...args ) => {
return fns.reduceRight( ( state, fn ) => {
// TODO: Assess whether this can be replaced with a more standard `compose` implementation
// like wp.data.compose() (aka lodash flowRight) or Redux compose().
// The current implementation only works by functions mutating the original state object.
const fnState = fn( ...args );
return isEmpty( fnState ) ? state : { ...state, ...fnState };
}, {} as InputState );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ export const TypographyPanel = () => {
<LineHeightControl
value={ lineHeight }
onChange={ setLineHeight }
__unstableInputWidth="100%"
__nextHasNoMarginBottom={ true }
/>
</ToolsPanelItem>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import {
__experimentalFontAppearanceControl as FontAppearanceControl,
__experimentalLetterSpacingControl as LetterSpacingControl,
} from '@wordpress/block-editor';
import { PanelBody, FontSizePicker } from '@wordpress/components';
import {
PanelBody,
FontSizePicker,
__experimentalSpacer as Spacer,
} from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -143,10 +147,13 @@ export default function TypographyPanel( { name, element } ) {
/>
) }
{ hasLineHeightEnabled && (
<LineHeightControl
value={ lineHeight }
onChange={ setLineHeight }
/>
<Spacer marginBottom={ 6 }>
<LineHeightControl
__nextHasNoMarginBottom={ true }
value={ lineHeight }
onChange={ setLineHeight }
/>
</Spacer>
mirka marked this conversation as resolved.
Show resolved Hide resolved
) }
{ hasAppearanceControl && (
<FontAppearanceControl
Expand Down