BorderControl: Update labelling, tooltips and wrap with fieldset and legend (#42348)

Author: aaronrobertshaw

packages/components/src/base-control/styles/base-control-styles.ts

@@ -39,6 +39,11 @@ const labelStyles = css`
 
 export const StyledLabel = styled.label`
 	${ labelStyles }
+	/**
+	 * Removes Chrome/Safari/Firefox user agent stylesheet padding from
+	 * StyledLabel when it is rendered as a legend.
+	 */
+	padding: 0;
 `;
 
 const deprecatedMarginHelp = ( { __nextHasNoMarginBottom = false } ) => {

packages/components/src/border-control/border-control-dropdown/component.tsx

@@ -176,6 +176,8 @@ const BorderControlDropdown = (
 			variant="tertiary"
 			aria-label={ toggleAriaLabel }
 			position={ dropdownPosition }
+			label={ __( 'Border color and style picker' ) }
+			showTooltip={ true }
 		>
 			<span className={ indicatorWrapperClassName }>
 				<ColorIndicator

packages/components/src/border-control/border-control/component.tsx

@@ -1,3 +1,8 @@
+/**
+ * WordPress dependencies
+ */
+import { __ } from '@wordpress/i18n';
+
 /**
  * Internal dependencies
  */
@@ -21,9 +26,9 @@ const BorderLabel = ( props: LabelProps ) => {
 	}
 
 	return hideLabelFromVision ? (
-		<VisuallyHidden as="label">{ label }</VisuallyHidden>
+		<VisuallyHidden as="legend">{ label }</VisuallyHidden>
 	) : (
-		<StyledLabel>{ label }</StyledLabel>
+		<StyledLabel as="legend">{ label }</StyledLabel>
 	);
 };
 
@@ -59,7 +64,7 @@ const UnconnectedBorderControl = (
 	} = useBorderControl( props );
 
 	return (
-		<View { ...otherProps } ref={ forwardedRef }>
+		<View as="fieldset" { ...otherProps } ref={ forwardedRef }>
 			<BorderLabel
 				label={ label }
 				hideLabelFromVision={ hideLabelFromVision }
@@ -85,6 +90,8 @@ const UnconnectedBorderControl = (
 						__next36pxDefaultSize={ __next36pxDefaultSize }
 					/>
 					<UnitControl
+						label={ __( 'Border width' ) }
+						hideLabelFromVision
 						className={ widthControlClassName }
 						min={ 0 }
 						onChange={ onWidthChange }
@@ -94,6 +101,8 @@ const UnconnectedBorderControl = (
 				</HStack>
 				{ withSlider && (
 					<RangeControl
+						label={ __( 'Border width' ) }
+						hideLabelFromVision
 						className={ sliderClassName }
 						initialPosition={ 0 }
 						max={ 100 }

packages/components/src/border-control/styles.ts

@@ -31,6 +31,9 @@ const focusBoxShadow = css`
 
 export const borderControl = css`
 	position: relative;
+	border: 0;
+	padding: 0;
+	margin: 0;
 `;
 
 export const innerWrapper = () => css`

packages/components/src/border-control/test/index.js

@@ -58,8 +58,15 @@ const clickButton = ( name ) => {
 	fireEvent.click( getButton( name ) );
 };
 
+const getSliderInput = () => {
+	return screen.getByRole( 'slider', { name: 'Border width' } );
+};
+
+const getWidthInput = () => {
+	return screen.getByRole( 'spinbutton', { name: 'Border width' } );
+};
 const setWidthInput = ( value ) => {
-	const widthInput = screen.getByRole( 'spinbutton' );
+	const widthInput = getWidthInput();
 	widthInput.focus();
 	fireEvent.change( widthInput, { target: { value } } );
 };
@@ -73,9 +80,13 @@ describe( 'BorderControl', () => {
 
 			const label = screen.getByText( props.label );
 			const colorButton = screen.getByLabelText( toggleLabelRegex );
-			const widthInput = screen.getByRole( 'spinbutton' );
-			const unitSelect = screen.getByRole( 'combobox' );
-			const slider = screen.queryByRole( 'slider' );
+			const widthInput = getWidthInput();
+			const unitSelect = screen.getByRole( 'combobox', {
+				name: 'Select unit',
+			} );
+			const slider = screen.queryByRole( 'slider', {
+				name: 'Border width',
+			} );
 
 			expect( label ).toBeInTheDocument();
 			expect( colorButton ).toBeInTheDocument();
@@ -100,13 +111,13 @@ describe( 'BorderControl', () => {
 		it( 'should render with slider', () => {
 			renderBorderControl( { withSlider: true } );
 
-			const slider = screen.getByRole( 'slider' );
+			const slider = getSliderInput();
 			expect( slider ).toBeInTheDocument();
 		} );
 
 		it( 'should render placeholder in UnitControl', () => {
 			renderBorderControl( { placeholder: 'Mixed' } );
-			const widthInput = screen.getByRole( 'spinbutton' );
+			const widthInput = getWidthInput();
 
 			expect( widthInput ).toHaveAttribute( 'placeholder', 'Mixed' );
 		} );
@@ -262,7 +273,7 @@ describe( 'BorderControl', () => {
 		it( 'should update width with slider value', () => {
 			const { rerender } = renderBorderControl( { withSlider: true } );
 
-			const slider = screen.getByRole( 'slider' );
+			const slider = getSliderInput();
 			fireEvent.change( slider, { target: { value: '5' } } );
 
 			expect( props.onChange ).toHaveBeenNthCalledWith( 1, {
@@ -271,7 +282,7 @@ describe( 'BorderControl', () => {
 			} );
 
 			rerenderBorderControl( rerender, { withSlider: true } );
-			const widthInput = screen.getByRole( 'spinbutton' );
+			const widthInput = getWidthInput();
 
 			expect( widthInput.value ).toEqual( '5' );
 		} );