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

Global Styles Sidebar: Clean up button spacing and semantics #40533

Closed
wants to merge 6 commits into from
Closed

Global Styles Sidebar: Clean up button spacing and semantics #40533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e61bc34
Global Styles Sidebar: Tweak spacing
mirka Apr 22, 2022
d95fc51
Wrap lone NavigationButton in ItemGroup
mirka Apr 22, 2022
a45a37a
PR 📌 Rename NavigationButton so semantics are clearer
mirka Apr 22, 2022
d97a42b
Bandaid fix semantics in header back button
mirka Apr 22, 2022
6f325b4
PR 📌 Fix header styles and semantics
mirka Apr 22, 2022
a2be017
Add back description padding
mirka Apr 25, 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
14 changes: 7 additions & 7 deletions packages/edit-site/src/components/global-styles/context-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useHasBorderPanel } from './border-panel';
import { useHasColorPanel } from './color-utils';
import { useHasDimensionsPanel } from './dimensions-panel';
import { useHasTypographyPanel } from './typography-panel';
import { NavigationButton } from './navigation-button';
import { NavigationButtonAsItem } from './navigation-button';

function ContextMenu( { name, parentMenu = '' } ) {
const hasTypographyPanel = useHasTypographyPanel( name );
Expand All @@ -24,28 +24,28 @@ function ContextMenu( { name, parentMenu = '' } ) {
return (
<ItemGroup>
{ hasTypographyPanel && (
<NavigationButton
<NavigationButtonAsItem
icon={ typography }
path={ parentMenu + '/typography' }
>
{ __( 'Typography' ) }
</NavigationButton>
</NavigationButtonAsItem>
) }
{ hasColorPanel && (
<NavigationButton
<NavigationButtonAsItem
icon={ color }
path={ parentMenu + '/colors' }
>
{ __( 'Colors' ) }
</NavigationButton>
</NavigationButtonAsItem>
) }
{ hasLayoutPanel && (
<NavigationButton
<NavigationButtonAsItem
icon={ layout }
path={ parentMenu + '/layout' }
>
{ __( 'Layout' ) }
</NavigationButton>
</NavigationButtonAsItem>
) }
</ItemGroup>
);
Expand Down
37 changes: 20 additions & 17 deletions packages/edit-site/src/components/global-styles/header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,33 @@ import {
__experimentalSpacer as Spacer,
__experimentalHeading as Heading,
__experimentalView as View,
__experimentalNavigatorBackButton as NavigatorBackButton,
} from '@wordpress/components';
import { isRTL, __ } from '@wordpress/i18n';
import { chevronRight, chevronLeft } from '@wordpress/icons';

/**
* Internal dependencies
*/
import { NavigationBackButton } from './navigation-button';

function ScreenHeader( { title, description } ) {
return (
<VStack spacing={ 2 }>
<HStack spacing={ 2 }>
<View>
<NavigationBackButton
icon={ isRTL() ? chevronRight : chevronLeft }
size="small"
aria-label={ __( 'Navigate to the previous view' ) }
/>
</View>
<Spacer>
<Heading level={ 5 }>{ title }</Heading>
<VStack spacing={ 0 }>
<View>
<Spacer marginBottom={ 0 } paddingX={ 4 } paddingY={ 3 }>
<HStack spacing={ 2 }>
<NavigatorBackButton
style={
// TODO: This style override is also used in ToolsPanelHeader.
// It should be supported out-of-the-box by Button.
{ minWidth: 24, padding: 0 }
}
icon={ isRTL() ? chevronRight : chevronLeft }
isSmall
aria-label={ __( 'Navigate to the previous view' ) }
/>
<Spacer>
<Heading level={ 5 }>{ title }</Heading>
</Spacer>
</HStack>
</Spacer>
</HStack>
</View>
{ description && (
<p className="edit-site-global-styles-header__description">
{ description }
Expand Down
6 changes: 3 additions & 3 deletions packages/edit-site/src/components/global-styles/navigation-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ function GenericNavigationButton( { icon, children, ...props } ) {
);
}

function NavigationButton( props ) {
function NavigationButtonAsItem( props ) {
return <NavigatorButton as={ GenericNavigationButton } { ...props } />;
}

function NavigationBackButton( props ) {
function NavigationBackButtonAsItem( props ) {
return <NavigatorBackButton as={ GenericNavigationButton } { ...props } />;
}

export { NavigationButton, NavigationBackButton };
export { NavigationButtonAsItem, NavigationBackButtonAsItem };
6 changes: 3 additions & 3 deletions packages/edit-site/src/components/global-styles/palette.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { useMemo } from '@wordpress/element';
* Internal dependencies
*/
import Subtitle from './subtitle';
import { NavigationButton } from './navigation-button';
import { NavigationButtonAsItem } from './navigation-button';
import { useSetting } from './hooks';
import ColorIndicatorWrapper from './color-indicator-wrapper';

Expand Down Expand Up @@ -58,7 +58,7 @@ function Palette( { name } ) {
<VStack spacing={ 3 }>
<Subtitle>{ __( 'Palette' ) }</Subtitle>
<ItemGroup isBordered isSeparated>
<NavigationButton path={ screenPath }>
<NavigationButtonAsItem path={ screenPath }>
<HStack
direction={
colors.length === 0 ? 'row-reverse' : 'row'
Expand All @@ -73,7 +73,7 @@ function Palette( { name } ) {
</ZStack>
<FlexItem>{ paletteButtonText }</FlexItem>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
</ItemGroup>
</VStack>
);
Expand Down
6 changes: 3 additions & 3 deletions packages/edit-site/src/components/global-styles/screen-block-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { useHasColorPanel } from './color-utils';
import { useHasDimensionsPanel } from './dimensions-panel';
import { useHasTypographyPanel } from './typography-panel';
import ScreenHeader from './header';
import { NavigationButton } from './navigation-button';
import { NavigationButtonAsItem } from './navigation-button';

function useSortedBlockTypes() {
const blockItems = useSelect(
Expand Down Expand Up @@ -61,12 +61,12 @@ function BlockMenuItem( { block } ) {
}

return (
<NavigationButton path={ '/blocks/' + block.name }>
<NavigationButtonAsItem path={ '/blocks/' + block.name }>
<HStack justify="flex-start">
<BlockIcon icon={ block.icon } />
<FlexItem>{ block.title }</FlexItem>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
);
}

Expand Down
14 changes: 7 additions & 7 deletions packages/edit-site/src/components/global-styles/screen-colors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
*/
import ScreenHeader from './header';
import Palette from './palette';
import { NavigationButton } from './navigation-button';
import { NavigationButtonAsItem } from './navigation-button';
import { getSupportedGlobalStylesPanels, useStyle } from './hooks';
import Subtitle from './subtitle';
import ColorIndicatorWrapper from './color-indicator-wrapper';
Expand All @@ -33,7 +33,7 @@ function BackgroundColorItem( { name, parentMenu } ) {
}

return (
<NavigationButton path={ parentMenu + '/colors/background' }>
<NavigationButtonAsItem path={ parentMenu + '/colors/background' }>
<HStack justify="flex-start">
<ColorIndicatorWrapper expanded={ false }>
<ColorIndicator
Expand All @@ -42,7 +42,7 @@ function BackgroundColorItem( { name, parentMenu } ) {
</ColorIndicatorWrapper>
<FlexItem>{ __( 'Background' ) }</FlexItem>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
);
}

Expand All @@ -56,14 +56,14 @@ function TextColorItem( { name, parentMenu } ) {
}

return (
<NavigationButton path={ parentMenu + '/colors/text' }>
<NavigationButtonAsItem path={ parentMenu + '/colors/text' }>
<HStack justify="flex-start">
<ColorIndicatorWrapper expanded={ false }>
<ColorIndicator colorValue={ color } />
</ColorIndicatorWrapper>
<FlexItem>{ __( 'Text' ) }</FlexItem>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
);
}

Expand All @@ -77,14 +77,14 @@ function LinkColorItem( { name, parentMenu } ) {
}

return (
<NavigationButton path={ parentMenu + '/colors/link' }>
<NavigationButtonAsItem path={ parentMenu + '/colors/link' }>
<HStack justify="flex-start">
<ColorIndicatorWrapper expanded={ false }>
<ColorIndicator colorValue={ color } />
</ColorIndicatorWrapper>
<FlexItem>{ __( 'Links' ) }</FlexItem>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
);
}

Expand Down
58 changes: 34 additions & 24 deletions packages/edit-site/src/components/global-styles/screen-root.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
*/
import {
__experimentalItemGroup as ItemGroup,
__experimentalItem as Item,
__experimentalHStack as HStack,
__experimentalSpacer as Spacer,
__experimentalVStack as VStack,
FlexItem,
CardBody,
Expand All @@ -21,7 +21,7 @@ import { store as coreStore } from '@wordpress/core-data';
* Internal dependencies
*/
import { IconWithCurrentColor } from './icon-with-current-color';
import { NavigationButton } from './navigation-button';
import { NavigationButtonAsItem } from './navigation-button';
import ContextMenu from './context-menu';
import StylesPreview from './preview';

Expand All @@ -37,48 +37,58 @@ function ScreenRoot() {
return (
<Card size="small">
<CardBody>
<VStack spacing={ 2 }>
<VStack spacing={ 4 }>
<Card>
<CardMedia>
<StylesPreview />
</CardMedia>
</Card>
{ !! variations?.length && (
<NavigationButton path="/variations">
<HStack justify="space-between">
<FlexItem>{ __( 'Browse styles' ) }</FlexItem>
<IconWithCurrentColor
icon={
isRTL() ? chevronLeft : chevronRight
}
/>
</HStack>
</NavigationButton>
<ItemGroup>
<NavigationButtonAsItem path="/variations">
<HStack justify="space-between">
<FlexItem>
{ __( 'Browse styles' ) }
</FlexItem>
<IconWithCurrentColor
icon={
isRTL() ? chevronLeft : chevronRight
}
/>
</HStack>
</NavigationButtonAsItem>
</ItemGroup>
) }
<ContextMenu />
</VStack>
</CardBody>

<CardBody>
<ContextMenu />
</CardBody>

<CardDivider />

<CardBody>
<Spacer
as="p"
paddingTop={ 2 }
/*
* 13px matches the text inset of the NavigationButton (12px padding, plus the width of the button's border).
* This is an ad hoc override for this particular instance only and should be reconsidered before making into a pattern.
*/
paddingX="13px"
marginBottom={ 4 }
>
{ __(
'Customize the appearance of specific blocks for the whole site.'
) }
</Spacer>
<ItemGroup>
<Item>
{ __(
'Customize the appearance of specific blocks for the whole site.'
) }
</Item>
<NavigationButton path="/blocks">
<NavigationButtonAsItem path="/blocks">
<HStack justify="space-between">
<FlexItem>{ __( 'Blocks' ) }</FlexItem>
<IconWithCurrentColor
icon={ isRTL() ? chevronLeft : chevronRight }
/>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
</ItemGroup>
</CardBody>
</Card>
Expand Down
6 changes: 3 additions & 3 deletions packages/edit-site/src/components/global-styles/screen-typography.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
* Internal dependencies
*/
import ScreenHeader from './header';
import { NavigationButton } from './navigation-button';
import { NavigationButtonAsItem } from './navigation-button';
import { useStyle } from './hooks';
import Subtitle from './subtitle';
import TypographyPanel from './typography-panel';
Expand Down Expand Up @@ -44,7 +44,7 @@ function Item( { name, parentMenu, element, label } ) {
}

return (
<NavigationButton path={ parentMenu + '/typography/' + element }>
<NavigationButtonAsItem path={ parentMenu + '/typography/' + element }>
<HStack justify="flex-start">
<FlexItem
className="edit-site-global-styles-screen-typography__indicator"
Expand All @@ -62,7 +62,7 @@ function Item( { name, parentMenu, element, label } ) {
</FlexItem>
<FlexItem>{ label }</FlexItem>
</HStack>
</NavigationButton>
</NavigationButtonAsItem>
);
}

Expand Down