From 9078db12a316e4457bb4f19faf3cd512f12aed94 Mon Sep 17 00:00:00 2001
From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Date: Thu, 8 Apr 2021 15:53:40 +1000
Subject: [PATCH] Add top/bottom margins to separator block

This leverages the modified BoxControl component from `update/boxcontrol-with-configurable-sides` to allow only top and bottom margins to be set.
---
 .../block-library/src/separator/block.json    |   3 +
 packages/block-library/src/separator/edit.js  |  52 +++++++--
 .../src/separator/edit.native.js              |  48 ++++++++
 .../block-library/src/separator/editor.scss   |  10 +-
 packages/block-library/src/separator/save.js  |   2 +
 .../src/separator/separator-settings.js       |  80 ++++++++++---
 .../separator/separator-settings.native.js    | 107 +++++++++++++++++-
 .../block-library/src/separator/shared.js     |  72 ++++++++++++
 .../src/horizontal-rule/index.native.js       |  29 +++--
 9 files changed, 366 insertions(+), 37 deletions(-)
 create mode 100644 packages/block-library/src/separator/edit.native.js
 create mode 100644 packages/block-library/src/separator/shared.js

diff --git a/packages/block-library/src/separator/block.json b/packages/block-library/src/separator/block.json
index 011b9d3219f9bc..063111139aa8de 100644
--- a/packages/block-library/src/separator/block.json
+++ b/packages/block-library/src/separator/block.json
@@ -8,6 +8,9 @@
 		},
 		"customColor": {
 			"type": "string"
+		},
+		"style": {
+			"type": "object"
 		}
 	},
 	"supports": {
diff --git a/packages/block-library/src/separator/edit.js b/packages/block-library/src/separator/edit.js
index f611abde42a62b..8e951a4e1daa75 100644
--- a/packages/block-library/src/separator/edit.js
+++ b/packages/block-library/src/separator/edit.js
@@ -6,29 +6,59 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
-import { HorizontalRule } from '@wordpress/components';
+import {
+	HorizontalRule,
+	__experimentalBoxControl as BoxControl,
+} from '@wordpress/components';
+import { View } from '@wordpress/primitives';
 import { withColors, useBlockProps } from '@wordpress/block-editor';
+
 /**
  * Internal dependencies
  */
 import SeparatorSettings from './separator-settings';
+import { MARGIN_CONSTRAINTS, parseUnit } from './shared';
+
+const { __Visualizer: BoxControlVisualizer } = BoxControl;
+
+function SeparatorEdit( props ) {
+	const {
+		color,
+		attributes: { style },
+	} = props;
+
+	const { top, bottom } = style?.spacing?.margin || {};
+	const marginUnit = parseUnit( top || bottom );
+	const blockProps = useBlockProps();
 
-function SeparatorEdit( { color, setColor, className } ) {
 	return (
 		<>
-			<HorizontalRule
-				{ ...useBlockProps( {
-					className: classnames( className, {
+			<View
+				{ ...blockProps }
+				className={ blockProps.className?.replace(
+					'wp-block-separator',
+					'wp-block-separator-wrapper'
+				) }
+			>
+				<BoxControlVisualizer
+					values={ style?.spacing?.margin }
+					showValues={ style?.visualizers?.margin }
+				/>
+				<HorizontalRule
+					className={ classnames( blockProps.className, {
 						'has-background': color.color,
 						[ color.class ]: color.class,
-					} ),
-					style: {
+					} ) }
+					style={ {
 						backgroundColor: color.color,
 						color: color.color,
-					},
-				} ) }
-			/>
-			<SeparatorSettings color={ color } setColor={ setColor } />
+						marginTop: top || MARGIN_CONSTRAINTS[ marginUnit ].min,
+						marginBottom:
+							bottom || MARGIN_CONSTRAINTS[ marginUnit ].min,
+					} }
+				/>
+			</View>
+			<SeparatorSettings { ...props } />
 		</>
 	);
 }
diff --git a/packages/block-library/src/separator/edit.native.js b/packages/block-library/src/separator/edit.native.js
new file mode 100644
index 00000000000000..e42d73b663170c
--- /dev/null
+++ b/packages/block-library/src/separator/edit.native.js
@@ -0,0 +1,48 @@
+/**
+ * WordPress dependencies
+ */
+import { HorizontalRule, useConvertUnitToMobile } from '@wordpress/components';
+import { withColors, useBlockProps } from '@wordpress/block-editor';
+
+/**
+ * Internal dependencies
+ */
+import SeparatorSettings from './separator-settings';
+import { MARGIN_CONSTRAINTS, parseUnit } from './shared';
+
+function SeparatorEdit( props ) {
+	const {
+		color,
+		attributes: { style },
+	} = props;
+
+	const { top, bottom } = style?.spacing?.margin || {};
+	const marginUnit = parseUnit( top || bottom );
+
+	const convertedMarginTop = useConvertUnitToMobile(
+		parseFloat( top || 0 ) || MARGIN_CONSTRAINTS[ marginUnit ].min,
+		marginUnit
+	);
+
+	const convertedMarginBottom = useConvertUnitToMobile(
+		parseFloat( bottom || 0 ) || MARGIN_CONSTRAINTS[ marginUnit ].min,
+		marginUnit
+	);
+
+	return (
+		<>
+			<HorizontalRule
+				{ ...useBlockProps() }
+				style={ {
+					backgroundColor: color.color,
+					color: color.color,
+					marginBottom: convertedMarginBottom,
+					marginTop: convertedMarginTop,
+				} }
+			/>
+			<SeparatorSettings { ...props } />
+		</>
+	);
+}
+
+export default withColors( 'color', { textColor: 'color' } )( SeparatorEdit );
diff --git a/packages/block-library/src/separator/editor.scss b/packages/block-library/src/separator/editor.scss
index 24e940684279e8..f8e155a43e6e8d 100644
--- a/packages/block-library/src/separator/editor.scss
+++ b/packages/block-library/src/separator/editor.scss
@@ -1,5 +1,7 @@
-.block-editor-block-list__block[data-type="core/separator"] {
-	// Prevent margin collapsing so the area to select the separator is bigger.
-	padding-top: 0.1px;
-	padding-bottom: 0.1px;
+.wp-block-separator-wrapper {
+	display: flex;
+
+	.wp-block-separator {
+		width: 100%;
+	}
 }
diff --git a/packages/block-library/src/separator/save.js b/packages/block-library/src/separator/save.js
index 67d489bd611c3c..f913385b0b9e90 100644
--- a/packages/block-library/src/separator/save.js
+++ b/packages/block-library/src/separator/save.js
@@ -27,6 +27,8 @@ export default function separatorSave( { attributes } ) {
 	const style = {
 		backgroundColor: backgroundClass ? undefined : customColor,
 		color: colorClass ? undefined : customColor,
+		marginBottom: attributes.style?.spacing?.margin?.bottom,
+		marginTop: attributes.style?.spacing?.margin?.top,
 	};
 
 	return <hr { ...useBlockProps.save( { className, style } ) } />;
diff --git a/packages/block-library/src/separator/separator-settings.js b/packages/block-library/src/separator/separator-settings.js
index 28691fbabbd72d..f0df3e5ce3cf48 100644
--- a/packages/block-library/src/separator/separator-settings.js
+++ b/packages/block-library/src/separator/separator-settings.js
@@ -3,20 +3,74 @@
  */
 import { __ } from '@wordpress/i18n';
 import { InspectorControls, PanelColorSettings } from '@wordpress/block-editor';
+import {
+	PanelBody,
+	__experimentalBoxControl as BoxControl,
+} from '@wordpress/components';
 
-const SeparatorSettings = ( { color, setColor } ) => (
-	<InspectorControls>
-		<PanelColorSettings
-			title={ __( 'Color' ) }
-			colorSettings={ [
-				{
-					value: color.color,
-					onChange: setColor,
-					label: __( 'Color' ),
+/**
+ * Internal dependencies
+ */
+import { CSS_UNITS } from './shared';
+
+const SeparatorSettings = ( {
+	color,
+	setColor,
+	attributes: { style },
+	setAttributes,
+} ) => {
+	const updateMargins = ( { top, bottom } ) => {
+		setAttributes( {
+			style: {
+				...style,
+				spacing: {
+					...style?.spacing,
+					margin: { top, bottom },
 				},
-			] }
-		></PanelColorSettings>
-	</InspectorControls>
-);
+			},
+		} );
+	};
+
+	const onChangeShowVisualizer = ( { top, bottom } ) => {
+		setAttributes( {
+			style: {
+				...style,
+				visualizers: {
+					margin: { top, bottom },
+				},
+			},
+		} );
+	};
+
+	return (
+		<InspectorControls>
+			<PanelColorSettings
+				title={ __( 'Color settings' ) }
+				colorSettings={ [
+					{
+						value: color.color,
+						onChange: setColor,
+						label: __( 'Color' ),
+					},
+				] }
+			/>
+			<PanelBody title={ __( 'Separator settings' ) }>
+				<BoxControl
+					label={ __( 'Margin' ) }
+					onChange={ updateMargins }
+					sides={ {
+						top: true,
+						right: false,
+						bottom: true,
+						left: false,
+					} }
+					units={ CSS_UNITS }
+					values={ style?.spacing?.margin || {} }
+					onChangeShowVisualizer={ onChangeShowVisualizer }
+				/>
+			</PanelBody>
+		</InspectorControls>
+	);
+};
 
 export default SeparatorSettings;
diff --git a/packages/block-library/src/separator/separator-settings.native.js b/packages/block-library/src/separator/separator-settings.native.js
index d2bdd8ef6443a3..0f7295e486802b 100644
--- a/packages/block-library/src/separator/separator-settings.native.js
+++ b/packages/block-library/src/separator/separator-settings.native.js
@@ -1,3 +1,106 @@
-// Mobile has no separator settings at this time, so render nothing
-const SeparatorSettings = () => null;
+/**
+ * WordPress dependencies
+ */
+import { __ } from '@wordpress/i18n';
+import { InspectorControls, PanelColorSettings } from '@wordpress/block-editor';
+import { PanelBody, UnitControl } from '@wordpress/components';
+
+/**
+ * Internal dependencies
+ */
+import { CSS_UNITS, MARGIN_CONSTRAINTS, parseUnit } from './shared';
+
+const SeparatorSettings = ( {
+	color,
+	setColor,
+	attributes,
+	setAttributes,
+} ) => {
+	const { style } = attributes;
+	const { top, bottom } = style?.spacing?.margin || {};
+
+	const topUnit = parseUnit( top );
+	const bottomUnit = parseUnit( bottom );
+	const topValue = top
+		? parseFloat( top )
+		: MARGIN_CONSTRAINTS[ topUnit ].min;
+	const bottomValue = bottom
+		? parseFloat( bottom )
+		: MARGIN_CONSTRAINTS[ bottomUnit ].min;
+
+	const updateMargins = ( margins ) => {
+		setAttributes( {
+			style: {
+				...style,
+				spacing: {
+					...style?.spacing,
+					margin: margins,
+				},
+			},
+		} );
+	};
+
+	const createHandleMarginChange = ( side, unit ) => ( value ) => {
+		updateMargins( {
+			...style?.spacing?.margin,
+			[ side ]: `${ value }${ unit }`,
+		} );
+	};
+
+	const onUnitChange = ( unit ) => {
+		updateMargins( {
+			top: MARGIN_CONSTRAINTS[ unit ].default,
+			bottom: MARGIN_CONSTRAINTS[ unit ].default,
+		} );
+	};
+
+	return (
+		<InspectorControls>
+			<PanelColorSettings
+				title={ __( 'Color settings' ) }
+				colorSettings={ [
+					{
+						value: color.color,
+						onChange: setColor,
+						label: __( 'Color' ),
+					},
+				] }
+			/>
+			<PanelBody title={ __( 'Separator settings' ) }>
+				<UnitControl
+					label={ __( 'Top margin' ) }
+					key={ `separator-top-margin-${ bottomUnit }` }
+					min={ MARGIN_CONSTRAINTS[ topUnit ].min }
+					max={ MARGIN_CONSTRAINTS[ topUnit ].max }
+					value={ topValue || MARGIN_CONSTRAINTS[ topUnit ].min }
+					unit={ topUnit }
+					units={ CSS_UNITS }
+					onChange={ createHandleMarginChange( 'top', topUnit ) }
+					onUnitChange={ onUnitChange }
+					decimalNum={ 1 }
+					step={ topUnit === 'px' ? 1 : 0.1 }
+				/>
+				<UnitControl
+					label={ __( 'Bottom margin' ) }
+					key={ `separator-bottom-margin-${ bottomUnit }` }
+					min={ MARGIN_CONSTRAINTS[ bottomUnit ].min }
+					max={ MARGIN_CONSTRAINTS[ bottomUnit ].max }
+					value={
+						bottomValue || MARGIN_CONSTRAINTS[ bottomUnit ].min
+					}
+					unit={ bottomUnit }
+					units={ CSS_UNITS }
+					onChange={ createHandleMarginChange(
+						'bottom',
+						bottomUnit
+					) }
+					onUnitChange={ onUnitChange }
+					decimalNum={ 1 }
+					step={ bottomUnit === 'px' ? 1 : 0.1 }
+				/>
+			</PanelBody>
+		</InspectorControls>
+	);
+};
+
 export default SeparatorSettings;
diff --git a/packages/block-library/src/separator/shared.js b/packages/block-library/src/separator/shared.js
new file mode 100644
index 00000000000000..7f3ae05fdf0908
--- /dev/null
+++ b/packages/block-library/src/separator/shared.js
@@ -0,0 +1,72 @@
+/**
+ * WordPress dependencies
+ */
+import { Platform } from '@wordpress/element';
+import { __ } from '@wordpress/i18n';
+
+// Separator margin related constants.
+export const MIN_PX_MARGIN = 15;
+export const MIN_EM_MARGIN = 0.75;
+export const MIN_REM_MARGIN = 0.7;
+export const MAX_PX_MARGIN = 300;
+export const MAX_EM_MARGIN = 20;
+export const MAX_REM_MARGIN = 20;
+
+const isWeb = Platform.OS === 'web';
+
+/**
+ * Available CSS units for specifying separator block margins.
+ */
+export const CSS_UNITS = [
+	{ value: 'px', label: isWeb ? 'px' : __( 'Pixels (px)' ), default: 0 },
+	{
+		value: 'em',
+		label: isWeb ? 'em' : __( 'Relative to parent font size (em)' ),
+		default: 0,
+	},
+	{
+		value: 'rem',
+		label: isWeb ? 'rem' : __( 'Relative to root font size (rem)' ),
+		default: 0,
+	},
+];
+
+/**
+ * Separator margin constraints for available CSS units.
+ */
+export const MARGIN_CONSTRAINTS = {
+	px: {
+		min: MIN_PX_MARGIN,
+		max: MAX_PX_MARGIN,
+		default: `${ MIN_PX_MARGIN }px`,
+	},
+	em: {
+		min: MIN_EM_MARGIN,
+		max: MAX_EM_MARGIN,
+		default: '1em',
+	},
+	rem: {
+		min: MIN_REM_MARGIN,
+		max: MAX_REM_MARGIN,
+		default: '1rem',
+	},
+};
+
+/**
+ * Extracts CSS unit from string.
+ *
+ * @param { string } cssValue CSS string containing unit and value.
+ * @return { string }         CSS unit. Defaults to 'px'.
+ */
+export const parseUnit = ( cssValue ) => {
+	if ( ! cssValue ) {
+		return 'px';
+	}
+
+	const matches = cssValue.trim().match( /[\d.\-+]*\s*([a-zA-Z]*)$/ );
+	if ( ! matches ) {
+		return 'px';
+	}
+	const [ , unit ] = matches;
+	return ( unit || 'px' ).toLowerCase();
+};
diff --git a/packages/primitives/src/horizontal-rule/index.native.js b/packages/primitives/src/horizontal-rule/index.native.js
index 853b57e76d0c1a..3d338ad7d4ba7f 100644
--- a/packages/primitives/src/horizontal-rule/index.native.js
+++ b/packages/primitives/src/horizontal-rule/index.native.js
@@ -2,6 +2,7 @@
  * External dependencies
  */
 import Hr from 'react-native-hr';
+import { View } from 'react-native';
 
 /**
  * WordPress dependencies
@@ -13,16 +14,30 @@ import { withPreferredColorScheme } from '@wordpress/compose';
  */
 import styles from './styles.scss';
 
-const HR = ( { getStylesFromColorScheme, ...props } ) => {
+const HR = ( { getStylesFromColorScheme, style, ...props } ) => {
 	const lineStyle = getStylesFromColorScheme( styles.line, styles.lineDark );
+	const customBackground = style?.backgroundColor
+		? { backgroundColor: style.backgroundColor }
+		: {};
 
 	return (
-		<Hr
-			{ ...props }
-			lineStyle={ [ lineStyle, props.lineStyle ] }
-			marginLeft={ 0 }
-			marginRight={ 0 }
-		/>
+		<View
+			style={ {
+				marginTop: style?.marginTop,
+				marginBottom: style?.marginBottom,
+			} }
+		>
+			<Hr
+				{ ...props }
+				lineStyle={ {
+					...lineStyle,
+					...props.lineStyle,
+					...customBackground,
+				} }
+				marginLeft={ 0 }
+				marginRight={ 0 }
+			/>
+		</View>
 	);
 };