address feedback

WordPress · Mar 31, 2022 · 05e1573 · 05e1573
1 parent 8adb214
commit 05e1573
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 12 deletions.
diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -5,7 +5,6 @@
 ### Enhancements
 
 -   Update `BorderControl` and `BorderBoxControl` to allow the passing of custom class names to popovers ([#39753](https://github.com/WordPress/gutenberg/pull/39753)).
--   `ToggleGroupControlOption`: Add `icon` support ([#39760](https://github.com/WordPress/gutenberg/pull/39760)).
 
 ### Internal
 
@@ -15,6 +14,7 @@
 
 -   Add `BorderControl` component ([#37769](https://github.com/WordPress/gutenberg/pull/37769)).
 -   Add `BorderBoxControl` component ([#38876](https://github.com/WordPress/gutenberg/pull/38876)).
+-   Add `ToggleGroupControlOptionIcon` component ([#39760](https://github.com/WordPress/gutenberg/pull/39760)).
 
 ### Bug Fix
 
@@ -27,7 +27,7 @@
 -   `CustomSelectControl`: Add `__next36pxDefaultSize` flag for larger default size ([#39401](https://github.com/WordPress/gutenberg/pull/39401)).
 -   `BaseControl`: Add `__nextHasNoMarginBottom` prop for opting into the new margin-free styles ([#39325](https://github.com/WordPress/gutenberg/pull/39325)).
 -   `Divider`: Make the divider visible by default (`display: inline`) in flow layout containers when the divider orientation is vertical ([#39316](https://github.com/WordPress/gutenberg/pull/39316)).
--   Stop using deprecated `event.keyCode` in favor of `event.key` for keyboard events in `UnitControl` and `InputControl`.  ([#39360](https://github.com/WordPress/gutenberg/pull/39360))
+-   Stop using deprecated `event.keyCode` in favor of `event.key` for keyboard events in `UnitControl` and `InputControl`. ([#39360](https://github.com/WordPress/gutenberg/pull/39360))
 -   `ColorPalette`: refine custom color button's label. ([#39386](https://github.com/WordPress/gutenberg/pull/39386))
 -   Add `onClick` prop on `FormFileUpload`. ([#39268](https://github.com/WordPress/gutenberg/pull/39268))
 -   `FocalPointPicker`: stop using `UnitControl`'s deprecated `unit` prop ([#39504](https://github.com/WordPress/gutenberg/pull/39504)).

diff --git a/packages/components/src/toggle-group-control/stories/index.js b/packages/components/src/toggle-group-control/stories/index.js
@@ -143,11 +143,13 @@ export const WithIcons = () => {
 				value="uppercase"
 				icon={ formatUppercase }
 				showTooltip={ true }
+				aria-label="Uppercase"
 			/>
 			<ToggleGroupControlOptionIcon
 				value="lowercase"
 				icon={ formatLowercase }
 				showTooltip={ true }
+				aria-label="Lowercase"
 			/>
 		</ToggleGroupControl>
 	);

diff --git a/packages/components/src/toggle-group-control/test/__snapshots__/index.js.snap b/packages/components/src/toggle-group-control/test/__snapshots__/index.js.snap
@@ -157,6 +157,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
       >
         <button
           aria-checked="true"
+          aria-label="Uppercase"
           class="emotion-10 components-toggle-group-control-option-base emotion-11"
           data-value="uppercase"
           data-wp-c16t="true"
@@ -189,6 +190,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
       >
         <button
           aria-checked="false"
+          aria-label="Lowercase"
           class="emotion-10 components-toggle-group-control-option-base"
           data-value="lowercase"
           data-wp-c16t="true"

diff --git a/packages/components/src/toggle-group-control/test/index.js b/packages/components/src/toggle-group-control/test/index.js
@@ -60,11 +60,13 @@ describe( 'ToggleGroupControl', () => {
 						value="uppercase"
 						icon={ formatUppercase }
 						showTooltip={ true }
+						aria-label="Uppercase"
 					/>
 					<ToggleGroupControlOptionIcon
 						value="lowercase"
 						icon={ formatLowercase }
 						showTooltip={ true }
+						aria-label="Lowercase"
 					/>
 				</ToggleGroupControl>
 			);

diff --git a/.../components/src/toggle-group-control/toggle-group-control-option-base/README.md b/.../components/src/toggle-group-control/toggle-group-control-option-base/README.md
@@ -4,11 +4,13 @@
 This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes.
 </div>
 
-`ToggleGroupControlOptionBase` is a form component and is meant to be used as a child of [`ToggleGroupControl`](<(/packages/components/src/toggle-group-control/toggle-group-control/README.md)>).
+`ToggleGroupControlOptionBase` is a form component and is meant to be used as an internal, generic component for any children of [`ToggleGroupControl`](<(/packages/components/src/toggle-group-control/toggle-group-control/README.md)>).
 
 ## Props
 
-### `children`: ReactNode
+### `children`: `ReactNode`
+
+The children elements.
 
 -   Required: Yes
 
@@ -20,6 +22,6 @@ The value of the `ToggleGroupControlOptionBase`.
 
 ### `showTooltip`: `boolean`
 
-Whether to show a tooltip when hovering over the option. The tooltip will use the `aria-label` prop.
+Whether to show a tooltip when hovering over the option. The tooltip will only show if a label for it is provided using the `aria-label` prop.
 
 -   Required: No
diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx
@@ -43,7 +43,8 @@ const WithToolTip = ( { showTooltip, text, children }: WithToolTipProps ) => {
 function ToggleGroupControlOptionBase(
 	props: WordPressComponentProps<
 		ToggleGroupControlOptionBaseProps,
-		'button'
+		'button',
+		false
 	>,
 	forwardedRef: ForwardedRef< any >
 ) {
@@ -100,8 +101,8 @@ function ToggleGroupControlOptionBase(
 }
 
 /**
- * `ToggleGroupControlOptionBase` is a form component and is used to be used as a
- * child of `ToggleGroupControl`.
+ * `ToggleGroupControlOptionBase` is a form component and is meant to be used as an internal,
+ * generic component for any children of `ToggleGroupControl`.
  *
  * @example
  * ```jsx

diff --git a/.../components/src/toggle-group-control/toggle-group-control-option-icon/README.md b/.../components/src/toggle-group-control/toggle-group-control-option-icon/README.md
@@ -22,11 +22,13 @@ function Example() {
 				value="uppercase"
 				icon={ formatUppercase }
 				showTooltip={ true }
+				aria-label="Uppercase"
 			/>
 			<ToggleGroupControlOptionIcon
 				value="lowercase"
 				icon={ formatLowercase }
 				showTooltip={ true }
+				aria-label="Lowercase"
 			/>
 		</ToggleGroupControl>
 	);
@@ -43,12 +45,12 @@ The value of the `ToggleGroupControlOption`.
 
 ### `icon`: `WPComponent`
 
-Icon for the option.
+Icon displayed as the content of the the `ToggleGroupControl` option.
 
 -   Required: Yes
 
 ### `showTooltip`: `boolean`
 
-Whether to show a tooltip when hovering over the option. The tooltip will use the `aria-label` prop text.
+Whether to show a tooltip when hovering over the option. The tooltip will only show if a label for it is provided using the `aria-label` prop.
 
 -   Required: No
diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-icon/component.tsx b/packages/components/src/toggle-group-control/toggle-group-control-option-icon/component.tsx
@@ -13,7 +13,8 @@ import { ToggleGroupControlOptionBase } from '../toggle-group-control-option-bas
 export default function ToggleGroupControlOptionIcon(
 	props: WordPressComponentProps<
 		ToggleGroupControlOptionIconProps,
-		'button'
+		'button',
+		false
 	>
 ) {
 	const { icon, ...restProps } = props;

diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option/component.tsx b/packages/components/src/toggle-group-control/toggle-group-control-option/component.tsx
@@ -6,7 +6,11 @@ import type { ToggleGroupControlOptionProps } from '../types';
 import { ToggleGroupControlOptionBase } from '../toggle-group-control-option-base';
 
 export default function ToggleGroupControlOption(
-	props: WordPressComponentProps< ToggleGroupControlOptionProps, 'button' >
+	props: WordPressComponentProps<
+		ToggleGroupControlOptionProps,
+		'button',
+		false
+	>
 ) {
 	const { label, ...restProps } = props;
 	const optionLabel = restProps[ 'aria-label' ] || label;