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 4 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

### Enhancements

- `ToggleGroupControl`: Add `isBorderless` variant ([#43060](https://github.com/WordPress/gutenberg/pull/43060/)).
- `ToggleGroupControlOptionIcon`: Maintain square proportions ([#43060](https://github.com/WordPress/gutenberg/pull/43060/)).
- `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)).
- Add `box-sizing` reset style mixin to utils ([#42754](https://github.com/WordPress/gutenberg/pull/42754)).
Expand Down
6 changes: 6 additions & 0 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 @@ -24,6 +24,7 @@ export default {
component: ToggleGroupControl,
title: 'Components (Experimental)/ToggleGroupControl',
argTypes: {
isBorderless: { control: { type: 'boolean' } },
size: {
control: 'select',
options: [ 'default', '__unstable-large' ],
Expand Down Expand Up @@ -138,6 +139,10 @@ WithAriaLabel.args = {
],
};

/**
* The `<ToggleGroupControlOptionIcon>` component can be used for icon options.
* In this case, the `isBorderless` style is preferred.
*/
export const WithIcons = ( props ) => {
const [ state, setState ] = useState();
return (
Expand Down Expand Up @@ -165,6 +170,7 @@ export const WithIcons = ( props ) => {
};
WithIcons.args = {
...Default.args,
isBorderless: true,
};

export const WithReset = ( props ) => {
Expand Down
42 changes: 24 additions & 18 deletions packages/components/src/toggle-group-control/test/__snapshots__/index.js.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: 32px;
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
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: '32px',
'__unstable-large': '36px',
};

return css`
width: ${ iconButtonSizes[ size ] };
padding-left: 0;
padding-right: 0;
`;
};
2 changes: 1 addition & 1 deletion 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
Expand Up @@ -15,7 +15,7 @@ export default function ToggleGroupControlOptionIcon(
) {
const { icon, ...restProps } = props;
return (
<ToggleGroupControlOptionBase { ...restProps }>
<ToggleGroupControlOptionBase { ...restProps } isIcon>
<Icon icon={ icon } />
</ToggleGroupControlOptionBase>
);
Expand Down
14 changes: 12 additions & 2 deletions packages/components/src/toggle-group-control/toggle-group-control/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
*/
import {
contextConnect,
ContextSystemProvider,
useContextSystem,
WordPressComponentProps,
} from '../../ui/context';
Expand All @@ -43,6 +44,7 @@ function ToggleGroupControl(
className,
isAdaptiveWidth = false,
isBlock = false,
isBorderless = false,
label,
hideLabelFromVision = false,
help,
Expand Down Expand Up @@ -81,14 +83,20 @@ function ToggleGroupControl(
}
}, [ value ] );

const optionsContext = useMemo(
() => ( { ToggleGroupControlOptionBase: { size } } ),
[ size ]
);

const classes = useMemo(
() =>
cx(
styles.ToggleGroupControl( { size } ),
! isBorderless && styles.border,
isBlock && styles.block,
className
),
[ className, cx, isBlock, size ]
[ className, cx, isBlock, isBorderless, size ]
);
return (
<BaseControl help={ help }>
Expand Down Expand Up @@ -117,7 +125,9 @@ function ToggleGroupControl(
containerWidth={ sizes.width }
isAdaptiveWidth={ isAdaptiveWidth }
/>
{ children }
<ContextSystemProvider value={ optionsContext }>
{ children }
</ContextSystemProvider>
</RadioGroup>
</ToggleGroupControlContext.Provider>
</BaseControl>
Expand Down
17 changes: 10 additions & 7 deletions packages/components/src/toggle-group-control/toggle-group-control/styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ export const ToggleGroupControl = ( {
size: NonNullable< ToggleGroupControlProps[ 'size' ] >;
} ) => css`
background: ${ COLORS.ui.background };
border: 1px solid;
border-color: ${ COLORS.ui.border };
border: 1px solid transparent;
border-radius: ${ CONFIG.controlBorderRadius };
display: inline-flex;
min-width: 0;
Expand All @@ -28,18 +27,22 @@ export const ToggleGroupControl = ( {

${ toggleGroupControlSize( size ) }

&:hover {
border-color: ${ COLORS.ui.borderHover };
}

&:focus-within {
&:focus-within {
mirka marked this conversation as resolved.
Show resolved Hide resolved
border-color: ${ COLORS.ui.borderFocus };
box-shadow: ${ CONFIG.controlBoxShadowFocus };
outline: none;
z-index: 1;
}
`;

export const border = css`
border-color: ${ COLORS.ui.border };

&:hover {
border-color: ${ COLORS.ui.borderHover };
}
`;

export const toggleGroupControlSize = (
size: NonNullable< ToggleGroupControlProps[ 'size' ] >
) => {
Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/toggle-group-control/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ export type ToggleGroupControlProps = Omit<
* @default false
*/
isBlock?: boolean;
/**
* Removes the outer border from the control. A good style to use with `ToggleGroupControlOptionIcon`s.
*
* @default false
*/
isBorderless?: boolean;
/**
* Callback when a segment is selected.
*/
Expand Down