-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Extract the three color panels to their own global styles view #35400
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
|
||
import { getSupportedGlobalStylesPanels } from './hooks'; | ||
|
||
export function useHasColorPanel( name ) { | ||
const supports = getSupportedGlobalStylesPanels( name ); | ||
return ( | ||
supports.includes( 'color' ) || | ||
supports.includes( 'backgroundColor' ) || | ||
supports.includes( 'background' ) || | ||
supports.includes( 'linkColor' ) | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,90 @@ | |
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { | ||
__experimentalItemGroup as ItemGroup, | ||
__experimentalHStack as HStack, | ||
__experimentalVStack as VStack, | ||
FlexItem, | ||
ColorIndicator, | ||
} from '@wordpress/components'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import ColorPanel from './color-panel'; | ||
import ScreenHeader from './header'; | ||
import Palette from './palette'; | ||
import NavigationButton from './navigation-button'; | ||
import { getSupportedGlobalStylesPanels, useStyle } from './hooks'; | ||
import Subtitle from './subtitle'; | ||
|
||
function BackgroundColorItem( { name, parentMenu } ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have considered merging the three items (background, text and link) into a single component with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems fine to me. We might want to create a "BlockElement" higher level component eventually, but we need to see how the typography ones evolve, and how we want to render this in the block inspector where it doesn't "navigate". |
||
const supports = getSupportedGlobalStylesPanels( name ); | ||
const hasSupport = | ||
supports.includes( 'backgroundColor' ) || | ||
supports.includes( 'background' ); | ||
const [ backgroundColor ] = useStyle( 'color.background', name ); | ||
const [ gradientValue ] = useStyle( 'color.gradient', name ); | ||
|
||
if ( ! hasSupport ) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<NavigationButton path={ parentMenu + '/colors/background' }> | ||
<HStack justify="flex-start"> | ||
<FlexItem> | ||
<ColorIndicator | ||
colorValue={ gradientValue ?? backgroundColor } | ||
/> | ||
</FlexItem> | ||
<FlexItem>{ __( 'Background' ) }</FlexItem> | ||
</HStack> | ||
</NavigationButton> | ||
); | ||
} | ||
|
||
function TextColorItem( { name, parentMenu } ) { | ||
const supports = getSupportedGlobalStylesPanels( name ); | ||
const hasSupport = supports.includes( 'color' ); | ||
const [ color ] = useStyle( 'color.text', name ); | ||
|
||
if ( ! hasSupport ) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<NavigationButton path={ parentMenu + '/colors/text' }> | ||
<HStack justify="flex-start"> | ||
<FlexItem> | ||
<ColorIndicator colorValue={ color } /> | ||
</FlexItem> | ||
<FlexItem>{ __( 'Text' ) }</FlexItem> | ||
</HStack> | ||
</NavigationButton> | ||
); | ||
} | ||
|
||
function LinkColorItem( { name, parentMenu } ) { | ||
const supports = getSupportedGlobalStylesPanels( name ); | ||
const hasSupport = supports.includes( 'linkColor' ); | ||
const [ color ] = useStyle( 'elements.link.color.text', name ); | ||
|
||
if ( ! hasSupport ) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<NavigationButton path={ parentMenu + '/colors/link' }> | ||
<HStack justify="flex-start"> | ||
<FlexItem> | ||
<ColorIndicator colorValue={ color } /> | ||
</FlexItem> | ||
<FlexItem>{ __( 'Links' ) }</FlexItem> | ||
</HStack> | ||
</NavigationButton> | ||
); | ||
} | ||
|
||
function ScreenColors( { name } ) { | ||
const parentMenu = name === undefined ? '' : '/blocks/' + name; | ||
|
@@ -21,7 +99,30 @@ function ScreenColors( { name } ) { | |
'Manage color palettes and how they affect the different elements of the site.' | ||
) } | ||
/> | ||
<ColorPanel name={ name } /> | ||
|
||
<div className="edit-site-global-styles-screen-colors"> | ||
<VStack spacing={ 10 }> | ||
<Palette contextName={ name } /> | ||
|
||
<VStack spacing={ 3 }> | ||
<Subtitle>{ __( 'Elements' ) }</Subtitle> | ||
<ItemGroup isBordered isSeparated> | ||
<BackgroundColorItem | ||
name={ name } | ||
parentMenu={ parentMenu } | ||
/> | ||
<TextColorItem | ||
name={ name } | ||
parentMenu={ parentMenu } | ||
/> | ||
<LinkColorItem | ||
name={ name } | ||
parentMenu={ parentMenu } | ||
/> | ||
</ItemGroup> | ||
</VStack> | ||
</VStack> | ||
</div> | ||
</> | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have considered merging the three screens (background, text and link) into a single screen with a
colorType
prop to change its behavior but the code felt less readable and full with if/else. So I thought for now it's better to have separate screen (even with some code duplication).