SelectControl: Fix multiple prop styling (#47893)

* SelectControl: Fix `multiple` prop styling * Update readme * Add changelog * Refactor padding code * Add comment * Allow overflow scrolling when `multiple` * Update snapshot
WordPress · Feb 13, 2023 · a4e7a4b · a4e7a4b
1 parent 80aaf64
commit a4e7a4b
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 39 deletions.
diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Bug Fix
+
+-   `SelectControl`: Fix styling when `multiple` prop is enabled ([#47893](https://github.com/WordPress/gutenberg/pull/43213)).
+
 ### Enhancements
 
 -   `ColorPalette`: ensure text label contrast checking works with CSS variables ([#47373](https://github.com/WordPress/gutenberg/pull/47373)).

diff --git a/packages/components/src/dimension-control/test/__snapshots__/index.test.js.snap b/packages/components/src/dimension-control/test/__snapshots__/index.test.js.snap
@@ -116,7 +116,6 @@ exports[`DimensionControl rendering renders with custom sizes 1`] = `
   width: 100%;
   max-width: none;
   cursor: pointer;
-  overflow: hidden;
   white-space: nowrap;
   text-overflow: ellipsis;
   font-size: 16px;
@@ -126,6 +125,7 @@ exports[`DimensionControl rendering renders with custom sizes 1`] = `
   padding-bottom: 0;
   padding-left: 8px;
   padding-right: 26px;
+  overflow: hidden;
 }
 
 @media ( min-width: 600px ) {
@@ -387,7 +387,6 @@ exports[`DimensionControl rendering renders with defaults 1`] = `
   width: 100%;
   max-width: none;
   cursor: pointer;
-  overflow: hidden;
   white-space: nowrap;
   text-overflow: ellipsis;
   font-size: 16px;
@@ -397,6 +396,7 @@ exports[`DimensionControl rendering renders with defaults 1`] = `
   padding-bottom: 0;
   padding-left: 8px;
   padding-right: 26px;
+  overflow: hidden;
 }
 
 @media ( min-width: 600px ) {
@@ -668,7 +668,6 @@ exports[`DimensionControl rendering renders with icon and custom icon label 1`]
   width: 100%;
   max-width: none;
   cursor: pointer;
-  overflow: hidden;
   white-space: nowrap;
   text-overflow: ellipsis;
   font-size: 16px;
@@ -678,6 +677,7 @@ exports[`DimensionControl rendering renders with icon and custom icon label 1`]
   padding-bottom: 0;
   padding-left: 8px;
   padding-right: 26px;
+  overflow: hidden;
 }
 
 @media ( min-width: 600px ) {
@@ -961,7 +961,6 @@ exports[`DimensionControl rendering renders with icon and default icon label 1`]
   width: 100%;
   max-width: none;
   cursor: pointer;
-  overflow: hidden;
   white-space: nowrap;
   text-overflow: ellipsis;
   font-size: 16px;
@@ -971,6 +970,7 @@ exports[`DimensionControl rendering renders with icon and default icon label 1`]
   padding-bottom: 0;
   padding-left: 8px;
   padding-right: 26px;
+  overflow: hidden;
 }
 
 @media ( min-width: 600px ) {

diff --git a/packages/components/src/select-control/README.md b/packages/components/src/select-control/README.md
@@ -187,7 +187,9 @@ If this property is added, a help text will be generated using help property as
 
 #### multiple
 
-If this property is added, multiple values can be selected. The value passed should be an array.
+If this property is added, multiple values can be selected. The `value` passed should be an array.
+
+In most cases, it is preferable to use the `FormTokenField` or `CheckboxControl` components instead.
 
 -   Type: `Boolean`
 -   Required: No

diff --git a/packages/components/src/select-control/index.tsx b/packages/components/src/select-control/index.tsx
@@ -101,7 +101,9 @@ function UnforwardedSelectControl(
 				isFocused={ isFocused }
 				label={ label }
 				size={ size }
-				suffix={ suffix || <SelectControlChevronDown /> }
+				suffix={
+					suffix || ( ! multiple && <SelectControlChevronDown /> )
+				}
 				prefix={ prefix }
 				labelPosition={ labelPosition }
 				__next36pxDefaultSize={ __next36pxDefaultSize }

diff --git a/packages/components/src/select-control/style.scss b/packages/components/src/select-control/style.scss
@@ -1,16 +1,6 @@
 .components-select-control__input {
-	background: $white;
-	height: 36px;
-	line-height: 36px;
-	margin: 1px;
 	outline: 0;
-	width: 100%;
 	-webkit-tap-highlight-color: rgba(0, 0, 0, 0) !important;
-
-	@include break-medium() {
-		height: 28px;
-		line-height: 28px;
-	}
 }
 
 @media (max-width: #{ ($break-medium) }) {

diff --git a/packages/components/src/select-control/styles/select-control-styles.ts b/packages/components/src/select-control/styles/select-control-styles.ts
@@ -13,7 +13,10 @@ import type { SelectControlProps } from '../types';
 import InputControlSuffixWrapper from '../../input-control/input-suffix-wrapper';
 
 interface SelectProps
-	extends Pick< SelectControlProps, '__next36pxDefaultSize' | 'disabled' > {
+	extends Pick<
+		SelectControlProps,
+		'__next36pxDefaultSize' | 'disabled' | 'multiple'
+	> {
 	// Using `selectSize` instead of `size` to avoid a type conflict with the
 	// `size` HTML attribute of the `select` element.
 	selectSize?: SelectControlProps[ 'size' ];
@@ -50,8 +53,15 @@ const fontSizeStyles = ( { selectSize = 'default' }: SelectProps ) => {
 
 const sizeStyles = ( {
 	__next36pxDefaultSize,
+	multiple,
 	selectSize = 'default',
 }: SelectProps ) => {
+	if ( multiple ) {
+		// When `multiple`, just use the native browser styles
+		// without setting explicit height.
+		return;
+	}
+
 	const sizes = {
 		default: {
 			height: 36,
@@ -91,33 +101,37 @@ export const chevronIconSize = 18;
 
 const sizePaddings = ( {
 	__next36pxDefaultSize,
+	multiple,
 	selectSize = 'default',
 }: SelectProps ) => {
-	const iconWidth = chevronIconSize;
-
-	const sizes = {
-		default: {
-			paddingLeft: 16,
-			paddingRight: 16 + iconWidth,
-		},
-		small: {
-			paddingLeft: 8,
-			paddingRight: 8 + iconWidth,
-		},
-		'__unstable-large': {
-			paddingLeft: 16,
-			paddingRight: 16 + iconWidth,
-		},
+	const padding = {
+		default: 16,
+		small: 8,
+		'__unstable-large': 16,
 	};
 
 	if ( ! __next36pxDefaultSize ) {
-		sizes.default = {
-			paddingLeft: 8,
-			paddingRight: 8 + iconWidth,
-		};
+		padding.default = 8;
 	}
 
-	return rtl( sizes[ selectSize ] || sizes.default );
+	const selectedPadding = padding[ selectSize ] || padding.default;
+
+	return rtl( {
+		paddingLeft: selectedPadding,
+		paddingRight: selectedPadding + chevronIconSize,
+		...( multiple
+			? {
+					paddingTop: selectedPadding,
+					paddingBottom: selectedPadding,
+			  }
+			: {} ),
+	} );
+};
+
+const overflowStyles = ( { multiple }: SelectProps ) => {
+	return {
+		overflow: multiple ? 'auto' : 'hidden',
+	};
 };
 
 // TODO: Resolve need to use &&& to increase specificity
@@ -137,14 +151,14 @@ export const Select = styled.select< SelectProps >`
 		width: 100%;
 		max-width: none;
 		cursor: pointer;
-		overflow: hidden;
 		white-space: nowrap;
 		text-overflow: ellipsis;
 
 		${ disabledStyles };
 		${ fontSizeStyles };
 		${ sizeStyles };
 		${ sizePaddings };
+		${ overflowStyles }
 	}
 `;
 

diff --git a/packages/components/src/select-control/types.ts b/packages/components/src/select-control/types.ts
@@ -23,7 +23,9 @@ export interface SelectControlProps
 		>,
 		Pick< BaseControlProps, 'help' | '__nextHasNoMarginBottom' > {
 	/**
-	 * If this property is added, multiple values can be selected. The value passed should be an array.
+	 * If this property is added, multiple values can be selected. The `value` passed should be an array.
+	 *
+	 * In most cases, it is preferable to use the `FormTokenField` or `CheckboxControl` components instead.
 	 *
 	 * @default false
 	 */