Apply Marco's suggestions for TS verbatim

#44632 (review)
WordPress · Oct 12, 2022 · 4a602dd · 4a602dd
1 parent c9f5d8a
commit 4a602dd
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 44 deletions.
diff --git a/packages/components/src/border-control/border-control-dropdown/component.tsx b/packages/components/src/border-control/border-control-dropdown/component.tsx
@@ -198,7 +198,6 @@ const BorderControlDropdown = (
 							/>
 						</HStack>
 					) : undefined }
-					{ /* @ts-ignore colors prop: Type 'Colors | undefined' is not assignable to type 'Color[] | MultipleColors' */ }
 					<ColorPalette
 						className={ popoverContentClassName }
 						value={ color }

diff --git a/packages/components/src/border-control/types.ts b/packages/components/src/border-control/types.ts
@@ -16,7 +16,7 @@ export type Border = {
 
 export type Color = {
 	name: string;
-	color: CSSProperties[ 'color' ];
+	color: NonNullable< CSSProperties[ 'color' ] >;
 };
 
 export type ColorOrigin = {

diff --git a/packages/components/src/color-palette/index.tsx b/packages/components/src/color-palette/index.tsx
@@ -81,6 +81,11 @@ function SinglePalette( {
 			);
 		} );
 	}, [ colors, value, onChange, clearColor ] );
+
+	if ( colors.length === 0 ) {
+		return null;
+	}
+
 	return (
 		<CircularOptionPicker
 			className={ className }
@@ -98,6 +103,10 @@ function MultiplePalettes( {
 	value,
 	actions,
 }: MultiplePalettesProps ) {
+	if ( colors.length === 0 ) {
+		return null;
+	}
+
 	return (
 		<VStack spacing={ 3 } className={ className }>
 			{ colors.map( ( { name, colors: colorPalette }, index ) => {
@@ -155,9 +164,9 @@ export function CustomColorPickerDropdown( {
 }
 
 export const extractColorNameFromCurrentValue = (
-	currentValue?: string,
-	colors: Color[] | MultipleColors = [],
-	showMultiplePalettes: boolean = false
+	currentValue?: ColorPaletteProps[ 'value' ],
+	colors: ColorPaletteProps[ 'colors' ] = [],
+	showMultiplePalettes: ColorPaletteProps[ '__experimentalHasMultipleOrigins' ] = false
 ) => {
 	if ( ! currentValue ) {
 		return '';
@@ -169,8 +178,9 @@ export const extractColorNameFromCurrentValue = (
 		: colord( currentValue ).toHex();
 
 	// Normalize format of `colors` to simplify the following loop
-	const colorPalettes = showMultiplePalettes
-		? ( colors as MultipleColors )
+	type normalizedPaletteObject = { colors: Color[] };
+	const colorPalettes: normalizedPaletteObject[] = showMultiplePalettes
+		? ( colors as MultipleColors[] )
 		: [ { colors: colors as Color[] } ];
 	for ( const { colors: paletteColors } of colorPalettes ) {
 		for ( const { name: colorName, color: colorValue } of paletteColors ) {
@@ -195,10 +205,18 @@ export const showTransparentBackground = ( currentValue?: string ) => {
 	return colord( currentValue ).alpha() === 0;
 };
 
+const areColorsMultiplePalette = (
+	colors: NonNullable< ColorPaletteProps[ 'colors' ] >
+): colors is MultipleColors[] => {
+	return colors.every( ( colorObj ) =>
+		Array.isArray( ( colorObj as MultipleColors ).colors )
+	);
+};
+
 export default function ColorPalette( {
 	clearable = true,
 	className,
-	colors,
+	colors = [],
 	disableCustomColors = false,
 	enableAlpha,
 	onChange,
@@ -207,9 +225,31 @@ export default function ColorPalette( {
 	__experimentalIsRenderedInSidebar = false,
 }: ColorPaletteProps ) {
 	const clearColor = useCallback( () => onChange( undefined ), [ onChange ] );
-	const showMultiplePalettes =
-		__experimentalHasMultipleOrigins && !! colors?.length;
-	const Component = showMultiplePalettes ? MultiplePalettes : SinglePalette;
+
+	const buttonLabelName = useMemo(
+		() =>
+			extractColorNameFromCurrentValue(
+				value,
+				colors,
+				__experimentalHasMultipleOrigins
+			),
+		[ value, colors, __experimentalHasMultipleOrigins ]
+	);
+
+	// Make sure that the `colors` array has a format (single/multiple) that is
+	// compatible with the `__experimentalHasMultipleOrigins` flag. This is true
+	// when __experimentalHasMultipleOrigins and areColorsMultiplePalette() are
+	// either both `true` or both `false`.
+	if (
+		colors.length > 0 &&
+		__experimentalHasMultipleOrigins !== areColorsMultiplePalette( colors )
+	) {
+		// eslint-disable-next-line no-console
+		console.warn(
+			'wp.components.ColorPalette: please specify a format for the `colors` prop that is compatible with the `__experimentalHasMultipleOrigins` prop.'
+		);
+		return null;
+	}
 
 	const renderCustomColorPicker = () => (
 		<DropdownContentWrapper paddingSize="none">
@@ -226,15 +266,6 @@ export default function ColorPalette( {
 	const valueWithoutLeadingHash = value?.startsWith( '#' )
 		? value.substring( 1 )
 		: value ?? '';
-	const buttonLabelName = useMemo(
-		() =>
-			extractColorNameFromCurrentValue(
-				value,
-				colors,
-				showMultiplePalettes
-			),
-		[ value, colors, showMultiplePalettes ]
-	);
 
 	const customColorAccessibleLabel = !! valueWithoutLeadingHash
 		? sprintf(
@@ -247,6 +278,19 @@ export default function ColorPalette( {
 		  )
 		: __( 'Custom color picker.' );
 
+	const paletteCommonProps = {
+		clearable,
+		clearColor,
+		onChange,
+		value,
+		actions: !! clearable && (
+			// @ts-ignore Required className property.
+			<CircularOptionPicker.ButtonAction onClick={ clearColor }>
+				{ __( 'Clear' ) }
+			</CircularOptionPicker.ButtonAction>
+		),
+	};
+
 	return (
 		<VStack spacing={ 3 } className={ className }>
 			{ ! disableCustomColors && (
@@ -293,24 +337,17 @@ export default function ColorPalette( {
 					) }
 				/>
 			) }
-			<Component
-				clearable={ clearable }
-				clearColor={ clearColor }
-				// @ts-ignore Component can be MultiplePalettes or SinglePalette, which have different colors prop types.
-				colors={ colors }
-				onChange={ onChange }
-				value={ value }
-				actions={
-					!! clearable && (
-						// @ts-ignore Required className property.
-						<CircularOptionPicker.ButtonAction
-							onClick={ clearColor }
-						>
-							{ __( 'Clear' ) }
-						</CircularOptionPicker.ButtonAction>
-					)
-				}
-			/>
+			{ __experimentalHasMultipleOrigins ? (
+				<MultiplePalettes
+					{ ...paletteCommonProps }
+					colors={ colors as MultipleColors[] }
+				/>
+			) : (
+				<SinglePalette
+					{ ...paletteCommonProps }
+					colors={ colors as Color[] }
+				/>
+			) }
 		</VStack>
 	);
 }
diff --git a/packages/components/src/color-palette/stories/index.tsx b/packages/components/src/color-palette/stories/index.tsx
@@ -51,7 +51,7 @@ export default meta;
 const Template: ComponentStory< typeof ColorPalette > = ( args ) => {
 	const firstColor =
 		( args.colors as Color[] )[ 0 ].color ||
-		( args.colors as MultipleColors )[ 0 ].colors[ 0 ].color;
+		( args.colors as MultipleColors[] )[ 0 ].colors[ 0 ].color;
 	const [ color, setColor ] = useState< string | undefined >( firstColor );
 
 	return (

diff --git a/packages/components/src/color-palette/types.ts b/packages/components/src/color-palette/types.ts
@@ -1,19 +1,19 @@
 /**
  * External dependencies
  */
-import type { MouseEventHandler, ReactNode } from 'react';
+import type { CSSProperties, MouseEventHandler, ReactNode } from 'react';
 
 type OnColorChange = ( newColor?: string ) => void;
 
 export type Color = {
 	name: string;
-	color: string;
+	color: NonNullable< CSSProperties[ 'color' ] >;
 };
 
 export type MultipleColors = {
 	name: string;
 	colors: Color[];
-}[];
+};
 
 type PaletteProps = {
 	className?: string;
@@ -28,7 +28,7 @@ export type SinglePaletteProps = PaletteProps & {
 };
 
 export type MultiplePalettesProps = PaletteProps & {
-	colors: MultipleColors;
+	colors: MultipleColors[];
 };
 
 export type CustomColorPickerDropdownProps = {
@@ -52,8 +52,10 @@ export type ColorPaletteProps = {
 	className?: string;
 	/**
 	 * Array with the colors to be shown.
+	 *
+	 * @default []
 	 */
-	colors: MultipleColors | Color[];
+	colors: ( MultipleColors | Color )[];
 	/**
 	 * Whether to allow custom color or not.
 	 */