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.

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

Fix the headings hierarchy in the styles sidebar #43848

Merged
merged 8 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ function ColorGradientControlInner( {
clearable,
showTitle = true,
enableAlpha,
headingLevel,
} ) {
const canChooseAColor =
onColorChange && ( ! isEmpty( colors ) || ! disableCustomColors );
Expand Down Expand Up @@ -84,6 +85,7 @@ function ColorGradientControlInner( {
}
clearable={ clearable }
enableAlpha={ enableAlpha }
headingLevel={ headingLevel }
/>
),
[ TAB_GRADIENT.value ]: (
Expand All @@ -103,6 +105,7 @@ function ColorGradientControlInner( {
__experimentalIsRenderedInSidebar
}
clearable={ clearable }
headingLevel={ headingLevel }
/>
),
};
Expand Down
8 changes: 7 additions & 1 deletion packages/components/src/color-palette/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ function MultiplePalettes( {
onChange,
value,
actions,
headingLevel,
}: MultiplePalettesProps ) {
if ( colors.length === 0 ) {
return null;
Expand All @@ -117,7 +118,9 @@ function MultiplePalettes( {
{ colors.map( ( { name, colors: colorPalette }, index ) => {
return (
<VStack spacing={ 2 } key={ index }>
<ColorHeading>{ name }</ColorHeading>
<ColorHeading level={ headingLevel }>
{ name }
</ColorHeading>
<SinglePalette
clearColor={ clearColor }
colors={ colorPalette }
Expand Down Expand Up @@ -183,8 +186,10 @@ function UnforwardedColorPalette(
onChange,
value,
__experimentalIsRenderedInSidebar = false,
headingLevel = 2,
...otherProps
} = props;

const clearColor = useCallback( () => onChange( undefined ), [ onChange ] );

const hasMultipleColorOrigins = isMultiplePaletteArray( colors );
Expand Down Expand Up @@ -235,6 +240,7 @@ function UnforwardedColorPalette(
{ __( 'Clear' ) }
</CircularOptionPicker.ButtonAction>
),
headingLevel,
};

return (
Expand Down
8 changes: 8 additions & 0 deletions packages/components/src/color-palette/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { CSSProperties, ReactNode } from 'react';
* Internal dependencies
*/
import type { DropdownProps } from '../dropdown/types';
import type { HeadingSize } from '../heading/types';

export type ColorObject = {
name: string;
Expand All @@ -27,6 +28,7 @@ type PaletteProps = {
onChange: ( newColor?: string, index?: number ) => void;
value?: string;
actions?: ReactNode;
headingLevel?: HeadingSize;
};

export type SinglePaletteProps = PaletteProps & {
Expand Down Expand Up @@ -80,4 +82,10 @@ export type ColorPaletteProps = Pick< PaletteProps, 'onChange' > & {
* @default false
*/
__experimentalIsRenderedInSidebar?: boolean;
/**
* The heaeding level.
*
* @default 2
*/
headingLevel?: HeadingSize;
};
7 changes: 6 additions & 1 deletion packages/components/src/gradient-picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,16 @@ function MultipleOrigin( {
onChange,
value,
actions,
headingLevel,
} ) {
return (
<VStack spacing={ 3 } className={ className }>
{ gradients.map( ( { name, gradients: gradientSet }, index ) => {
return (
<VStack spacing={ 2 } key={ index }>
<ColorHeading>{ name }</ColorHeading>
<ColorHeading level={ headingLevel }>
{ name }
</ColorHeading>
<SingleOrigin
clearGradient={ clearGradient }
gradients={ gradientSet }
Expand Down Expand Up @@ -112,6 +115,7 @@ export default function GradientPicker( {
clearable = true,
disableCustomGradients = false,
__experimentalIsRenderedInSidebar,
headingLevel = 2,
} ) {
const clearGradient = useCallback(
() => onChange( undefined ),
Expand Down Expand Up @@ -168,6 +172,7 @@ export default function GradientPicker( {
</CircularOptionPicker.ButtonAction>
)
}
headingLevel={ headingLevel }
/>
) }
</VStack>
Expand Down
5 changes: 4 additions & 1 deletion packages/components/src/palette-edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ export default function PaletteEdit( {
colors = EMPTY_ARRAY,
onChange,
paletteLabel,
paletteLabelLevel = 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that this prop name conveys its meaning, since "level" only makes sense in the context of a "heading".

I suggest that we rename this prop to a more clear name, maybe paletteLabelHeadingLevel?

emptyMessage,
canOnlyChangeValues,
canReset,
Expand Down Expand Up @@ -347,7 +348,9 @@ export default function PaletteEdit( {
return (
<PaletteEditStyles>
<PaletteHStackHeader>
<PaletteHeading>{ paletteLabel }</PaletteHeading>
<PaletteHeading level={ paletteLabelLevel }>
{ paletteLabel }
</PaletteHeading>
<PaletteActionsContainer>
{ hasElements && isEditing && (
<DoneButton
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ const ToolsPanelHeader = (
dropdownMenuClassName,
hasMenuItems,
headingClassName,
headingLevel = 2,
label: labelText,
menuItems,
resetAll,
Expand Down Expand Up @@ -185,7 +186,7 @@ const ToolsPanelHeader = (

return (
<HStack { ...headerProps } ref={ forwardedRef }>
<Heading level={ 2 } className={ headingClassName }>
<Heading level={ headingLevel } className={ headingClassName }>
{ labelText }
</Heading>
{ hasMenuItems && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const ToolsPanel = (
panelContext,
resetAllItems,
toggleItem,
headingLevel,
...toolsPanelProps
} = useToolsPanel( props );

Expand All @@ -33,6 +34,7 @@ const ToolsPanel = (
label={ label }
resetAll={ resetAllItems }
toggleItem={ toggleItem }
headingLevel={ headingLevel }
/>
{ children }
</ToolsPanelContext.Provider>
Expand Down
13 changes: 13 additions & 0 deletions packages/components/src/tools-panel/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
*/
import type { ReactNode } from 'react';

/**
* Internal dependencies
*/
import type { HeadingSize } from '../heading/types';

type ResetAllFilter = () => void;
type ResetAll = ( filters?: ResetAllFilter[] ) => void;

Expand Down Expand Up @@ -47,6 +52,10 @@ export type ToolsPanelProps = {
* last visible `ToolsPanelItem` within the `ToolsPanel`.
*/
__experimentalLastVisibleItemClass?: string;
/**
* The heading level of the panel's header.
*/
headingLevel: HeadingSize;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this prop should be marked as optional as assigned a default value, both for ToolsPanel and ToolsPanelHeader. Any reason why it was marked as required instead?

};

export type ToolsPanelHeaderProps = {
Expand All @@ -67,6 +76,10 @@ export type ToolsPanelHeaderProps = {
* `onSelect` or `onDeselect` callbacks as appropriate.
*/
toggleItem: ( label: string ) => void;
/**
* The heading level of the panel's header.
*/
headingLevel: HeadingSize;
};

export type ToolsPanelItem = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default function ColorPalettePanel( { name } ) {
colors={ themeColors }
onChange={ setThemeColors }
paletteLabel={ __( 'Theme' ) }
paletteLabelLevel={ 3 }
/>
) }
{ !! defaultColors &&
Expand All @@ -66,12 +67,14 @@ export default function ColorPalettePanel( { name } ) {
colors={ defaultColors }
onChange={ setDefaultColors }
paletteLabel={ __( 'Default' ) }
paletteLabelLevel={ 3 }
/>
) }
<PaletteEdit
colors={ customColors }
onChange={ setCustomColors }
paletteLabel={ __( 'Custom' ) }
paletteLabelLevel={ 3 }
emptyMessage={ __(
'Custom colors are empty! Add some colors to create your own color palette.'
) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,11 @@ export default function DimensionsPanel( { name, variation = '' } ) {
};

return (
<ToolsPanel label={ __( 'Dimensions' ) } resetAll={ resetAll }>
<ToolsPanel
label={ __( 'Dimensions' ) }
resetAll={ resetAll }
headingLevel={ 3 }
>
{ ( showContentSizeControl || showWideSizeControl ) && (
<span className="span-columns">
{ __( 'Set the width of the main content area.' ) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export default function GradientPalettePanel( { name } ) {
gradients={ themeGradients }
onChange={ setThemeGradients }
paletteLabel={ __( 'Theme' ) }
paletteLabelLevel={ 3 }
/>
) }
{ !! defaultGradients &&
Expand All @@ -86,20 +87,22 @@ export default function GradientPalettePanel( { name } ) {
gradients={ defaultGradients }
onChange={ setDefaultGradients }
paletteLabel={ __( 'Default' ) }
paletteLabelLevel={ 3 }
/>
) }
<PaletteEdit
gradients={ customGradients }
onChange={ setCustomGradients }
paletteLabel={ __( 'Custom' ) }
paletteLabelLevel={ 3 }
emptyMessage={ __(
'Custom gradients are empty! Add some gradients to create your own palette.'
) }
slugPrefix="custom-"
/>
{ !! duotonePalette && !! duotonePalette.length && (
<div>
<Subtitle>{ __( 'Duotone' ) }</Subtitle>
<Subtitle level={ 3 }>{ __( 'Duotone' ) }</Subtitle>
<Spacer margin={ 3 } />
<DuotonePicker
duotonePalette={ duotonePalette }
Expand Down
8 changes: 7 additions & 1 deletion packages/edit-site/src/components/global-styles/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ function ScreenHeader( { title, description } ) {
aria-label={ __( 'Navigate to the previous view' ) }
/>
<Spacer>
<Heading level={ 5 }>{ title }</Heading>
<Heading
className="edit-site-global-styles-header"
level={ 2 }
size={ 13 }
>
{ title }
</Heading>
</Spacer>
</HStack>
</Spacer>
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/global-styles/palette.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function Palette( { name } ) {

return (
<VStack spacing={ 3 }>
<Subtitle>{ __( 'Palette' ) }</Subtitle>
<Subtitle level={ 3 }>{ __( 'Palette' ) }</Subtitle>
<ItemGroup isBordered isSeparated>
<NavigationButtonAsItem
path={ screenPath }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ function ScreenBackgroundColor( { name, variation = '' } ) {
showTitle={ false }
enableAlpha
__experimentalIsRenderedInSidebar
headingLevel={ 3 }
{ ...controlProps }
/>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ function ScreenButtonColor( { name, variation = '' } ) {
) }
/>

<h4 className="edit-site-global-styles-section-title">
<h3 className="edit-site-global-styles-section-title">
{ __( 'Text color' ) }
</h4>
</h3>

<ColorGradientControl
className="edit-site-screen-button-color__control"
Expand All @@ -78,11 +78,12 @@ function ScreenButtonColor( { name, variation = '' } ) {
colorValue={ buttonTextColor }
onColorChange={ setButtonTextColor }
clearable={ buttonTextColor === userButtonTextColor }
headingLevel={ 4 }
/>

<h4 className="edit-site-global-styles-section-title">
<h3 className="edit-site-global-styles-section-title">
{ __( 'Background color' ) }
</h4>
</h3>

<ColorGradientControl
className="edit-site-screen-button-color__control"
Expand All @@ -94,6 +95,7 @@ function ScreenButtonColor( { name, variation = '' } ) {
colorValue={ buttonBgColor }
onColorChange={ setButtonBgColor }
clearable={ buttonBgColor === userButtonBgColor }
headingLevel={ 4 }
/>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ function ScreenColors( { name, variation = '' } ) {
<Palette name={ name } />

<VStack spacing={ 3 }>
<Subtitle>{ __( 'Elements' ) }</Subtitle>
<Subtitle level={ 3 }>{ __( 'Elements' ) }</Subtitle>
<ItemGroup isBordered isSeparated>
<BackgroundColorItem
name={ name }
Expand Down
Loading