Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ToggleGroupControl: Improve styling for icon options #43060

Merged
merged 18 commits into from
Aug 11, 2022
Merged

ToggleGroupControl: Improve styling for icon options #43060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f213117
ToggleGroupControl: Add `isBorderless` variant
mirka Aug 6, 2022
73b30db
ToggleGroupControlOptionIcon: Improve sizing
mirka Aug 6, 2022
46a6ada
Add changelog
mirka Aug 8, 2022
7bc2bb4
Update snapshots
mirka Aug 8, 2022
fa743f7
Fix square size
mirka Aug 10, 2022
901814e
Fix indentation
mirka Aug 10, 2022
1736150
Fix a11y labeling
mirka Aug 11, 2022
37e6e45
Fix types
mirka Aug 11, 2022
0f21404
Rename `isBorderless` prop
mirka Aug 11, 2022
42d8192
Fixup
mirka Aug 11, 2022
02cc5d7
Update snapshots
mirka Aug 11, 2022
6b4f368
Fix up story control
mirka Aug 11, 2022
0d162a1
Privatize `size` context
mirka Aug 11, 2022
92260d8
Fixup docs
mirka Aug 11, 2022
684ec45
Update unit tests
mirka Aug 11, 2022
8cf4242
Add changelog about `label` prop
mirka Aug 11, 2022
7141b7d
Merge branch 'trunk' into toggle-group-control-icon-styles
mirka Aug 11, 2022
4efa533
Rename snapshot
mirka Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

### Enhancements

- `ToggleGroupControlOptionIcon`: Maintain square proportions ([#43060](https://github.com/WordPress/gutenberg/pull/43060/)).
- `ToggleGroupControlOptionIcon`: Add a required `label` prop so the button is always accessibly labeled. Also removes `showTooltip` from the accepted prop types, as the tooltip will now always be shown. ([#43060](https://github.com/WordPress/gutenberg/pull/43060/)).
- `SelectControl`, `CustomSelectControl`: Refresh and refactor chevron down icon ([#42962](https://github.com/WordPress/gutenberg/pull/42962)).
- `FontSizePicker`: Add large size variant ([#42716](https://github.com/WordPress/gutenberg/pull/42716/)).
- `Popover`: tidy up code, add more comments ([#42944](https://github.com/WordPress/gutenberg/pull/42944)).
Expand All @@ -30,6 +32,7 @@

### Internal

- `ToggleGroupControl`: Add `__experimentalIsIconGroup` prop ([#43060](https://github.com/WordPress/gutenberg/pull/43060/)).
- `Flex`, `FlexItem`, `FlexBlock`: Convert to TypeScript ([#42537](https://github.com/WordPress/gutenberg/pull/42537)).
- `InputControl`: Fix incorrect `size` prop passing ([#42793](https://github.com/WordPress/gutenberg/pull/42793)).
- `Placeholder`: Convert to TypeScript ([#42990](https://github.com/WordPress/gutenberg/pull/42990)).
Expand Down
15 changes: 10 additions & 5 deletions packages/components/src/toggle-group-control/stories/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ import Button from '../../button';
export default {
component: ToggleGroupControl,
title: 'Components (Experimental)/ToggleGroupControl',
subcomponents: { ToggleGroupControlOption, ToggleGroupControlOptionIcon },
argTypes: {
__experimentalIsIconGroup: { control: { type: 'boolean' } },
size: {
control: 'select',
control: 'radio',
options: [ 'default', '__unstable-large' ],
},
},
Expand Down Expand Up @@ -138,6 +140,10 @@ WithAriaLabel.args = {
],
};

/**
* The `<ToggleGroupControlOptionIcon>` component can be used for icon options.
* In this case, the `__experimentalIsIconGroup` style is preferred.
*/
export const WithIcons = ( props ) => {
const [ state, setState ] = useState();
return (
Expand All @@ -151,20 +157,19 @@ export const WithIcons = ( props ) => {
<ToggleGroupControlOptionIcon
value="uppercase"
icon={ formatUppercase }
showTooltip={ true }
aria-label="Uppercase"
label="Uppercase"
/>
<ToggleGroupControlOptionIcon
value="lowercase"
icon={ formatLowercase }
showTooltip={ true }
aria-label="Lowercase"
label="Lowercase"
mirka marked this conversation as resolved.
Show resolved Hide resolved
/>
</ToggleGroupControl>
);
};
WithIcons.args = {
...Default.args,
__experimentalIsIconGroup: true,
};

export const WithReset = ( props ) => {
Expand Down
42 changes: 24 additions & 18 deletions ...-control/test/__snapshots__/index.js.snap → ...control/test/__snapshots__/index.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `

.emotion-6 {
background: #fff;
border: 1px solid;
border-color: #757575;
border: 1px solid transparent;
border-radius: 2px;
display: -webkit-inline-box;
display: -webkit-inline-flex;
Expand All @@ -46,6 +45,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
-webkit-transition: -webkit-transform 100ms linear;
transition: transform 100ms linear;
min-height: 36px;
border-color: #757575;
}

@media ( prefers-reduced-motion: reduce ) {
Expand All @@ -54,17 +54,17 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
}
}

.emotion-6:hover {
border-color: #757575;
}

.emotion-6:focus-within {
border-color: var( --wp-admin-theme-color-darker-10, #007cba);
box-shadow: 0 0 0 0.5px var( --wp-admin-theme-color, #00669b);
outline: none;
z-index: 1;
}

.emotion-6:hover {
border-color: #757575;
}

.emotion-8 {
background: #1e1e1e;
border-radius: 2px;
Expand Down Expand Up @@ -152,14 +152,20 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
}

.emotion-13 {
width: 30px;
padding-left: 0;
padding-right: 0;
}

.emotion-14 {
color: #fff;
}

.emotion-13:active {
.emotion-14:active {
background: transparent;
}

.emotion-14 {
.emotion-15 {
font-size: 13px;
line-height: 1;
}
Expand Down Expand Up @@ -201,7 +207,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
<button
aria-checked="true"
aria-label="Uppercase"
class="emotion-12 components-toggle-group-control-option-base emotion-13"
class="emotion-12 emotion-13 components-toggle-group-control-option-base emotion-14"
data-value="uppercase"
data-wp-c16t="true"
data-wp-component="ToggleGroupControlOptionBase"
Expand All @@ -210,7 +216,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
tabindex="0"
>
<div
class="emotion-14 emotion-15"
class="emotion-15 emotion-16"
>
<svg
aria-hidden="true"
Expand All @@ -234,7 +240,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
<button
aria-checked="false"
aria-label="Lowercase"
class="emotion-12 components-toggle-group-control-option-base"
class="emotion-12 emotion-13 components-toggle-group-control-option-base"
data-value="lowercase"
data-wp-c16t="true"
data-wp-component="ToggleGroupControlOptionBase"
Expand All @@ -243,7 +249,7 @@ exports[`ToggleGroupControl should render correctly with icons 1`] = `
tabindex="-1"
>
<div
class="emotion-14 emotion-15"
class="emotion-15 emotion-16"
>
<svg
aria-hidden="true"
Expand Down Expand Up @@ -298,8 +304,7 @@ exports[`ToggleGroupControl should render correctly with text options 1`] = `

.emotion-6 {
background: #fff;
border: 1px solid;
border-color: #757575;
border: 1px solid transparent;
border-radius: 2px;
display: -webkit-inline-box;
display: -webkit-inline-flex;
Expand All @@ -311,6 +316,7 @@ exports[`ToggleGroupControl should render correctly with text options 1`] = `
-webkit-transition: -webkit-transform 100ms linear;
transition: transform 100ms linear;
min-height: 36px;
border-color: #757575;
}

@media ( prefers-reduced-motion: reduce ) {
Expand All @@ -319,17 +325,17 @@ exports[`ToggleGroupControl should render correctly with text options 1`] = `
}
}

.emotion-6:hover {
border-color: #757575;
}

.emotion-6:focus-within {
border-color: var( --wp-admin-theme-color-darker-10, #007cba);
box-shadow: 0 0 0 0.5px var( --wp-admin-theme-color, #00669b);
outline: none;
z-index: 1;
}

.emotion-6:hover {
border-color: #757575;
}

.emotion-8 {
display: -webkit-inline-box;
display: -webkit-inline-flex;
Expand Down
6 changes: 2 additions & 4 deletions ...ts/src/toggle-group-control/test/index.js → ...s/src/toggle-group-control/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,12 @@ describe( 'ToggleGroupControl', () => {
<ToggleGroupControlOptionIcon
value="uppercase"
icon={ formatUppercase }
showTooltip={ true }
aria-label="Uppercase"
label="Uppercase"
/>
<ToggleGroupControlOptionIcon
value="lowercase"
icon={ formatLowercase }
showTooltip={ true }
aria-label="Lowercase"
label="Lowercase"
/>
</ToggleGroupControl>
);
Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ function ToggleGroupControlOptionBase(
const {
className,
isBlock = false,
isIcon = false,
value,
children,
size = 'default',
Comment on lines +63 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that TypeScript is allowing this — technically isIcon and size are not defined on neither toggleGroupControlContext nor buttonProps.

I'm not sure if we can pass props that are not part of ToggleGroupControlOptionBase's props via the "context system". If we want to pass "private", internal props, we may need to use useToggleGroupControlContext (and declare these props in the ToggleGroupControlContextProps type)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that TypeScript is allowing this

I don't quite understand it either, good catch.

I fixed this up. size is now private to the context system, and isIcon is defined on OptionBase but not the final Option components. Final props tables of the Option components can now be verified in Storybook.

showTooltip = false,
...radioProps
} = {
Expand All @@ -74,6 +76,7 @@ function ToggleGroupControlOptionBase(
const labelViewClasses = cx( isBlock && styles.labelBlock );
const classes = cx(
styles.buttonView,
isIcon && styles.isIcon( { size } ),
className,
isActive && styles.buttonActive
);
Expand Down
18 changes: 18 additions & 0 deletions packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import styled from '@emotion/styled';
* Internal dependencies
*/
import { CONFIG, COLORS, reduceMotion } from '../../utils';
import type { ToggleGroupControlProps } from '../types';

export const LabelView = styled.div`
display: inline-flex;
Expand Down Expand Up @@ -69,3 +70,20 @@ export const ButtonContentView = styled.div`
export const separatorActive = css`
background: transparent;
`;

export const isIcon = ( {
size,
}: {
size: NonNullable< ToggleGroupControlProps[ 'size' ] >;
} ) => {
const iconButtonSizes = {
default: '30px',
'__unstable-large': '34px',
};

return css`
width: ${ iconButtonSizes[ size ] };
padding-left: 0;
padding-right: 0;
`;
};
14 changes: 6 additions & 8 deletions .../components/src/toggle-group-control/toggle-group-control-option-icon/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,16 @@ import { formatLowercase, formatUppercase } from '@wordpress/icons';

function Example() {
return (
<ToggleGroupControl label="my label" value="lowercase" isBlock>
<ToggleGroupControl __experimentalIsIconGroup>
<ToggleGroupControlOptionIcon
value="uppercase"
icon={ formatUppercase }
showTooltip={ true }
aria-label="Uppercase"
label="Uppercase"
/>
<ToggleGroupControlOptionIcon
value="lowercase"
icon={ formatLowercase }
showTooltip={ true }
aria-label="Lowercase"
label="Lowercase"
/>
</ToggleGroupControl>
);
Expand All @@ -49,8 +47,8 @@ Icon displayed as the content of the option. Usually one of the icons from the `

- Required: Yes

### `showTooltip`: `boolean`
### `label`: `string`

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.
The text to accessibly label the icon option. Will also be shown in a tooltip.

- Required: No
- Required: Yes
37 changes: 30 additions & 7 deletions packages/components/src/toggle-group-control/toggle-group-control-option-icon/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
/**
* External dependencies
*/
import type { ForwardedRef } from 'react';

/**
* WordPress dependencies
*/
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
Expand All @@ -6,16 +16,23 @@ import type { ToggleGroupControlOptionIconProps } from '../types';
import { ToggleGroupControlOptionBase } from '../toggle-group-control-option-base';
import Icon from '../../icon';

export default function ToggleGroupControlOptionIcon(
function UnforwardedToggleGroupControlOptionIcon(
props: WordPressComponentProps<
ToggleGroupControlOptionIconProps,
'button',
false
>
>,
ref: ForwardedRef< any >
) {
const { icon, ...restProps } = props;
const { icon, label, ...restProps } = props;
return (
<ToggleGroupControlOptionBase { ...restProps }>
<ToggleGroupControlOptionBase
{ ...restProps }
isIcon
aria-label={ label }
showTooltip
ref={ ref }
>
<Icon icon={ icon } />
</ToggleGroupControlOptionBase>
);
Expand All @@ -25,7 +42,6 @@ export default function ToggleGroupControlOptionIcon(
* `ToggleGroupControlOptionIcon` is a form component which is meant to be used as a
* child of `ToggleGroupControl` and displays an icon.
*
* @example
* ```jsx
*
* import {
Expand All @@ -36,17 +52,24 @@ export default function ToggleGroupControlOptionIcon(
*
* function Example() {
* return (
* <ToggleGroupControl label="my label" value="vertical" isBlock>
* <ToggleGroupControl __experimentalIsIconGroup>
* <ToggleGroupControlOptionIcon
* value="uppercase"
* label="Uppercase"
* icon={ formatUppercase }
* />
* <ToggleGroupControlOptionIcon
* value="lowercase"
* label="Lowercase"
* icon={ formatLowercase }
* />
* </ToggleGroupControl>
* );
* }
** ```
* ```
*/
export const ToggleGroupControlOptionIcon = forwardRef(
UnforwardedToggleGroupControlOptionIcon
);

export default ToggleGroupControlOptionIcon;