Remove unnecessary color attributes from navigation block (#18540)

WordPress · Nov 15, 2019 · fbd7fb8 · fbd7fb8
1 parent 58e8c9e
commit fbd7fb8
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 119 deletions.
diff --git a/packages/block-library/src/navigation-menu/block-colors-selector.js b/packages/block-library/src/navigation-menu/block-colors-selector.js
@@ -1,7 +1,6 @@
 /**
  * External dependencies
  */
-import classnames from 'classnames';
 import { noop } from 'lodash';
 
 /**
@@ -24,20 +23,12 @@ const ColorSelectorSVGIcon = () => (
  * @param {Object} colorControlProps colorControl properties.
  * @return {*} React Icon component.
  */
-const ColorSelectorIcon = ( { textColor, textColorValue } ) => {
-	const iconClasses = classnames(
-		'block-library-colors-selector__state-selection',
-		'editor-styles-wrapper', {
-			'has-text-color': textColor && textColor.color,
-			[ textColor.class ]: textColor && textColor.class,
-		}
-	);
-
+const ColorSelectorIcon = ( { color } ) => {
 	return (
 		<div className="block-library-colors-selector__icon-container">
 			<div
-				className={ iconClasses }
-				style={ { ...( textColorValue && { color: textColorValue } ) } }
+				className="block-library-colors-selector__state-selection"
+				style={ { ...( color && { color } ) } }
 			>
 				<ColorSelectorSVGIcon />
 			</div>
@@ -51,7 +42,7 @@ const ColorSelectorIcon = ( { textColor, textColorValue } ) => {
  * @param {Object} colorControlProps colorControl properties.
  * @return {*} React toggle button component.
  */
-const renderToggleComponent = ( { textColor, textColorValue } ) => ( { onToggle, isOpen } ) => {
+const renderToggleComponent = ( { value } ) => ( { onToggle, isOpen } ) => {
 	const openOnArrowDown = ( event ) => {
 		if ( ! isOpen && event.keyCode === DOWN ) {
 			event.preventDefault();
@@ -67,24 +58,19 @@ const renderToggleComponent = ( { textColor, textColorValue } ) => ( { onToggle,
 				label={ __( 'Open Colors Selector' ) }
 				onClick={ onToggle }
 				onKeyDown={ openOnArrowDown }
-				icon={ <ColorSelectorIcon
-					textColor={ textColor }
-					textColorValue={ textColorValue }
-				/> }
+				icon={ <ColorSelectorIcon color={ value } /> }
 			/>
 		</Toolbar>
 	);
 };
 
-const renderContent = ( { textColor, onColorChange = noop } ) => ( () => {
-	const setColor = ( attr ) => ( value ) => onColorChange( { attr, value } );
-
+const renderContent = ( { value, onChange = noop } ) => ( () => {
 	return (
 		<>
 			<div className="color-palette-controller-container">
 				<ColorPaletteControl
-					value={ textColor.color }
-					onChange={ setColor( 'textColor' ) }
+					value={ value }
+					onChange={ onChange }
 					label={ __( 'Text Color' ) }
 				/>
 			</div>

diff --git a/packages/block-library/src/navigation-menu/edit.js b/packages/block-library/src/navigation-menu/edit.js
@@ -1,21 +1,15 @@
-/**
- * External dependencies
- */
-import classnames from 'classnames';
-
 /**
  * WordPress dependencies
  */
 import {
 	useMemo,
-	useEffect,
 	Fragment,
 } from '@wordpress/element';
 import {
 	InnerBlocks,
 	InspectorControls,
 	BlockControls,
-	withColors,
+	__experimentalUseColors,
 } from '@wordpress/block-editor';
 
 import { createBlock } from '@wordpress/blocks';
@@ -45,24 +39,20 @@ function NavigationMenu( {
 	pages,
 	isRequestingPages,
 	hasResolvedPages,
-	textColor,
-	setTextColor,
 	setAttributes,
 	hasExistingNavItems,
 	updateNavItemBlocks,
 } ) {
 	//
 	// HOOKS
 	//
+	/* eslint-disable @wordpress/no-unused-vars-before-return */
+	const { TextColor } = __experimentalUseColors(
+		[ { name: 'textColor', property: 'color' } ],
+	);
+	/* eslint-enable @wordpress/no-unused-vars-before-return */
 	const { navigatorToolbarButton, navigatorModal } = useBlockNavigator( clientId );
 
-	useEffect( () => {
-		// Set/Unset colors CSS classes.
-		setAttributes( {
-			textColorCSSClass: textColor.class ? textColor.class : null,
-		} );
-	}, [ textColor.class ] );
-
 	// Builds menu items from default Pages
 	const defaultPagesMenuItems = useMemo(
 		() => {
@@ -153,22 +143,6 @@ function NavigationMenu( {
 		);
 	}
 
-	// Build Inline Styles
-	const navigationMenuInlineStyles = {
-		...( textColor && {
-			color: textColor.color,
-			borderColor: textColor.color,
-		} ),
-	};
-
-	// Build ClassNames
-	const navigationMenuClasses = classnames(
-		'wp-block-navigation-menu', {
-			'has-text-color': textColor.color,
-			[ attributes.textColorCSSClass ]: attributes && attributes.textColorCSSClass,
-		}
-	);
-
 	// UI State: rendered Block UI
 	return (
 		<Fragment>
@@ -177,12 +151,8 @@ function NavigationMenu( {
 					{ navigatorToolbarButton }
 				</Toolbar>
 				<BlockColorsStyleSelector
-					textColor={ textColor }
-					textColorValue={ attributes.textColorValue }
-					onColorChange={ ( { value } ) => {
-						setTextColor( value );
-						setAttributes( { textColorValue: value } );
-					} }
+					value={ TextColor.color }
+					onChange={ TextColor.setColor }
 				/>
 			</BlockControls>
 			{ navigatorModal }
@@ -205,23 +175,23 @@ function NavigationMenu( {
 					<BlockNavigationList clientId={ clientId } />
 				</PanelBody>
 			</InspectorControls>
-
-			<div className={ navigationMenuClasses } style={ navigationMenuInlineStyles }>
-				{ ! hasExistingNavItems && isRequestingPages && <><Spinner /> { __( 'Loading Navigation…' ) } </> }
-
-				<InnerBlocks
-					allowedBlocks={ [ 'core/navigation-link' ] }
-					templateInsertUpdatesSelection={ false }
-					__experimentalMoverDirection={ 'horizontal' }
-				/>
-
-			</div>
+			<TextColor>
+				<div className="wp-block-navigation-menu">
+					{ ! hasExistingNavItems && isRequestingPages && <><Spinner /> { __( 'Loading Navigation…' ) } </> }
+
+					<InnerBlocks
+						allowedBlocks={ [ 'core/navigation-link' ] }
+						templateInsertUpdatesSelection={ false }
+						__experimentalMoverDirection={ 'horizontal' }
+					/>
+
+				</div>
+			</TextColor>
 		</Fragment>
 	);
 }
 
 export default compose( [
-	withColors( { textColor: 'color' } ),
 	withSelect( ( select, { clientId } ) => {
 		const innerBlocks = select( 'core/block-editor' ).getBlocks( clientId );
 		const hasExistingNavItems = !! innerBlocks.length;

diff --git a/packages/block-library/src/navigation-menu/index.php b/packages/block-library/src/navigation-menu/index.php
@@ -15,28 +15,30 @@
 function build_css_colors( $attributes ) {
 	// CSS classes.
 	$colors = array(
-		'text_css_classes'   => '',
-		'text_inline_styles' => '',
+		'css_classes'   => '',
+		'inline_styles' => '',
 	);
 
-	// Text color.
-	// Text color - has text color.
-	if ( array_key_exists( 'textColor', $attributes ) ) {
-		$colors['text_css_classes'] .= ' has-text-color';
-	}
-	// Text color - add custom CSS class.
-	if ( array_key_exists( 'textColorCSSClass', $attributes ) ) {
-		$colors['text_css_classes'] .= " {$attributes['textColorCSSClass']}";
+	$has_named_text_color  = array_key_exists( 'textColor', $attributes );
+	$has_custom_text_color = array_key_exists( 'customTextColor', $attributes );
 
-	} elseif ( array_key_exists( 'customTextColor', $attributes ) ) {
-		// Text color - or add inline `color` style.
-		$colors['text_inline_styles'] = ' style="color: ' . esc_attr( $attributes['customTextColor'] ) . ';"';
+	// If has text color.
+	if ( $has_custom_text_color || $has_named_text_color ) {
+		// Add has-text-color class.
+		$colors['css_classes'] .= 'has-text-color';
 	}
 
-	$colors['text_css_classes'] = esc_attr( trim( $colors['text_css_classes'] ) );
+	if ( $has_named_text_color ) {
+		// Add the color class.
+		$colors['css_classes'] .= sprintf( ' has-%s-color', $attributes['textColor'] );
+	} elseif ( $has_custom_text_color ) {
+		// Add the custom color inline style.
+		$colors['inline_styles'] = sprintf( 'color: %s;', $attributes['customTextColor'] );
+	}
 
 	return $colors;
 }
+
 /**
  * Renders the `core/navigation-menu` block on server.
  *
@@ -47,21 +49,22 @@ function build_css_colors( $attributes ) {
  * @return string Returns the post content with the legacy widget added.
  */
 function render_block_navigation_menu( $attributes, $content, $block ) {
-	// Inline computed colors.
-	$comp_inline_styles = '';
-
-	if ( array_key_exists( 'textColorValue', $attributes ) ) {
-		$comp_inline_styles .= ' color: ' . esc_attr( $attributes['textColorValue'] ) . ';';
-	}
-	$comp_inline_styles = ! empty( $comp_inline_styles )
-		? ' style="' . esc_attr( trim( $comp_inline_styles ) ) . '"'
-		: '';
-
-	$colors = build_css_colors( $attributes );
-
-	return "<nav class='wp-block-navigation-menu' {$comp_inline_styles}>" .
-		build_navigation_menu_html( $block, $colors ) .
-	'</nav>';
+	$colors          = build_css_colors( $attributes );
+	$class_attribute = sprintf( ' class="%s"', esc_attr( $colors['css_classes'] ? 'wp-block-navigation-menu ' . $colors['css_classes'] : 'wp-block-navigation-menu' ) );
+	$style_attribute = $colors['inline_styles'] ? sprintf( ' style="%s"', esc_attr( $colors['inline_styles'] ) ) : '';
+	return sprintf(
+		implode(
+			"\n",
+			array(
+				'<nav%s%s>',
+				'	%s',
+				'</nav>',
+			)
+		),
+		$class_attribute,
+		$style_attribute,
+		build_navigation_menu_html( $block, $colors )
+	);
 }
 
 /**
@@ -74,12 +77,14 @@ function render_block_navigation_menu( $attributes, $content, $block ) {
  */
 function build_navigation_menu_html( $block, $colors ) {
 	$html = '';
+
+	$class_attribute = sprintf( ' class="%s"', esc_attr( $colors['css_classes'] ? 'wp-block-navigation-menu-item__link ' . $colors['css_classes'] : 'wp-block-navigation-menu-item__link' ) );
+	$style_attribute = $colors['inline_styles'] ? sprintf( ' style="%s"', esc_attr( $colors['inline_styles'] ) ) : '';
+
 	foreach ( (array) $block['innerBlocks'] as $key => $block ) {
 
 		$html .= '<li class="wp-block-navigation-menu-item">' .
-			'<a
-				class="wp-block-navigation-menu-item__link ' . $colors['text_css_classes'] . '"
-				' . $colors['text_inline_styles'];
+			'<a' . $class_attribute . $style_attribute;
 
 		// Start appending HTML attributes to anchor tag.
 		if ( isset( $block['attrs']['url'] ) ) {
@@ -124,28 +129,17 @@ function register_block_core_navigation_menu() {
 		array(
 			'category'        => 'layout',
 			'attributes'      => array(
-				'className'         => array(
+				'className'        => array(
 					'type' => 'string',
 				),
-
-				'automaticallyAdd'  => array(
+				'automaticallyAdd' => array(
 					'type'    => 'boolean',
 					'default' => false,
 				),
-
-				'textColor'         => array(
-					'type' => 'string',
-				),
-
-				'textColorValue'    => array(
+				'textColor'        => array(
 					'type' => 'string',
 				),
-
-				'customTextColor'   => array(
-					'type' => 'string',
-				),
-
-				'textColorCSSClass' => array(
+				'customTextColor'  => array(
 					'type' => 'string',
 				),
 			),