Update: Refactor panel color gradient settings to use a tools panel. Unify color UI. #41091
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -604 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
aaronrobertshaw
requested changes
May 17, 2022
There was a problem hiding this comment.
Very nice work @jorgefilipecosta and a special thanks for the example use of batch! I wasn't aware of its availability. 🙇
In general, this is testing pretty well except for a couple of minor issues.
There is a more significant problem though with switching the color block supports panel to use the updated PanelColorGradientSettings. Doing so means the color block support controls are no longer rendered via the grouped InspectorControls slot which enables us to inject other color-related controls into that single panel.
We can revert the color block support to render its controls via the InspectorControls slot with only a subset of the code removed in this PR so we still benefit from your work here.
I've included a diff below that includes most of the suggestions from my review, reinstates the slot fill rendering for the color block support, fixes a linter issue etc.
Example diff with review suggestions
diff --git a/packages/block-editor/src/components/colors-gradients/dropdown.js b/packages/block-editor/src/components/colors-gradients/dropdown.js
index 5c76ccb6f9..b13e22b353 100644
--- a/packages/block-editor/src/components/colors-gradients/dropdown.js
+++ b/packages/block-editor/src/components/colors-gradients/dropdown.js
@@ -7,11 +7,11 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import {
+ Button,
ColorIndicator,
Dropdown,
FlexItem,
__experimentalHStack as HStack,
- __experimentalItem as Item,
__experimentalToolsPanelItem as ToolsPanelItem,
} from '@wordpress/components';
@@ -45,6 +45,9 @@ const WithToolsPanelItem = ( { setting, children, panelId, ...props } ) => {
{ ...props }
className="block-editor-tools-panel-color-gradient-settings__item"
panelId={ panelId }
+ // Pass resetAllFilter if supplied due to rendering via SlotFill
+ // into parent ToolsPanel.
+ resetAllFilter={ setting.resetAllFilter }
>
{ children }
</ToolsPanelItem>
@@ -70,16 +73,16 @@ const renderToggle = ( settings ) => ( { onToggle, isOpen } ) => {
const toggleProps = {
onClick: onToggle,
className: classnames(
- 'block-editor-tools-panel-color-gradient-settings__dropdown',
+ 'block-editor-panel-color-gradient-settings__dropdown',
{ 'is-open': isOpen }
),
'aria-expanded': isOpen,
};
return (
- <Item { ...toggleProps }>
+ <Button { ...toggleProps }>
<LabeledColorIndicator colorValue={ colorValue } label={ label } />
- </Item>
+ </Button>
);
};
diff --git a/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js b/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
index 1cc4b65ef7..797a6c9463 100644
--- a/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
+++ b/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
@@ -43,7 +43,7 @@ export const PanelColorGradientSettingsInner = ( {
__experimentalIsRenderedInSidebar,
enableAlpha,
} ) => {
- const panelId = useInstanceId( PanelColorGradientSettingsInner )
+ const panelId = useInstanceId( PanelColorGradientSettingsInner );
const { batch } = useRegistry();
if (
isEmpty( colors ) &&
@@ -90,8 +90,6 @@ export const PanelColorGradientSettingsInner = ( {
} );
} }
panelId={ panelId }
- hasInnerWrapper={ true }
- shouldRenderPlaceholderItems={ true } // Required to maintain fills ordering.
__experimentalFirstVisibleItemClass="first"
__experimentalLastVisibleItemClass="last"
>
diff --git a/packages/block-editor/src/components/colors-gradients/style.scss b/packages/block-editor/src/components/colors-gradients/style.scss
index abac92db1b..20c146ee71 100644
--- a/packages/block-editor/src/components/colors-gradients/style.scss
+++ b/packages/block-editor/src/components/colors-gradients/style.scss
@@ -112,3 +112,7 @@
}
}
}
+
+.block-editor-panel-color-gradient-settings__dropdown {
+ width: 100%;
+}
diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js
index 34a8645ea7..d0775abbc4 100644
--- a/packages/block-editor/src/hooks/color-panel.js
+++ b/packages/block-editor/src/hooks/color-panel.js
@@ -6,12 +6,11 @@ import { useState, useEffect } from '@wordpress/element';
/**
* Internal dependencies
*/
-import PanelColorGradientSettings from '../components/colors-gradients/panel-color-gradient-settings';
+import ColorGradientSettingsDropdown from '../components/colors-gradients/dropdown';
import ContrastChecker from '../components/contrast-checker';
import InspectorControls from '../components/inspector-controls';
import useMultipleOriginColorsAndGradients from '../components/colors-gradients/use-multiple-origin-colors-and-gradients';
import { __unstableUseBlockRef as useBlockRef } from '../components/block-list/use-block-props/use-block-refs';
-import { __ } from '@wordpress/i18n';
function getComputedStyle( node ) {
return node.ownerDocument.defaultView.getComputedStyle( node );
@@ -63,24 +62,24 @@ export default function ColorPanel( {
const colorGradientSettings = useMultipleOriginColorsAndGradients();
return (
- <InspectorControls>
- <PanelColorGradientSettings
+ <InspectorControls __experimentalGroup="color">
+ <ColorGradientSettingsDropdown
enableAlpha={ enableAlpha }
+ panelId={ clientId }
settings={ settings }
+ __experimentalIsItemGroup={ false }
__experimentalHasMultipleOrigins
__experimentalIsRenderedInSidebar
{ ...colorGradientSettings }
- title={ __( 'Color' ) }
- >
- { enableContrastChecking && (
- <ContrastChecker
- backgroundColor={ detectedBackgroundColor }
- textColor={ detectedColor }
- enableAlphaChecker={ enableAlpha }
- linkColor={ detectedLinkColor }
- />
- ) }
- </PanelColorGradientSettings>
+ />
+ { enableContrastChecking && (
+ <ContrastChecker
+ backgroundColor={ detectedBackgroundColor }
+ textColor={ detectedColor }
+ enableAlphaChecker={ enableAlpha }
+ linkColor={ detectedLinkColor }
+ />
+ ) }
</InspectorControls>
);
}
diff --git a/packages/block-editor/src/hooks/color.js b/packages/block-editor/src/hooks/color.js
index 52ade28f37..3219e01692 100644
--- a/packages/block-editor/src/hooks/color.js
+++ b/packages/block-editor/src/hooks/color.js
@@ -76,6 +76,60 @@ const hasTextColorSupport = ( blockType ) => {
return colorSupport && colorSupport.text !== false;
};
+/**
+ * Clears a single color property from a style object.
+ *
+ * @param {Array} path Path to color property to clear within styles object.
+ * @param {Object} style Block attributes style object.
+ * @return {Object} Styles with the color property omitted.
+ */
+const clearColorFromStyles = ( path, style ) =>
+ cleanEmptyObject( immutableSet( style, path, undefined ) );
+
+/**
+ * Clears text color related properties from supplied attributes.
+ *
+ * @param {Object} attributes Block attributes.
+ * @return {Object} Update block attributes with text color properties omitted.
+ */
+const resetAllTextFilter = ( attributes ) => ( {
+ textColor: undefined,
+ style: clearColorFromStyles( [ 'color', 'text' ], attributes.style ),
+} );
+
+/**
+ * Clears link color related properties from supplied attributes.
+ *
+ * @param {Object} attributes Block attributes.
+ * @return {Object} Update block attributes with link color properties omitted.
+ */
+const resetAllLinkFilter = ( attributes ) => ( {
+ style: clearColorFromStyles(
+ [ 'elements', 'link', 'color', 'text' ],
+ attributes.style
+ ),
+} );
+
+/**
+ * Clears all background color related properties including gradients from
+ * supplied block attributes.
+ *
+ * @param {Object} attributes Block attributes.
+ * @return {Object} Block attributes with background and gradient omitted.
+ */
+const clearBackgroundAndGradient = ( attributes ) => ( {
+ backgroundColor: undefined,
+ gradient: undefined,
+ style: {
+ ...attributes.style,
+ color: {
+ ...attributes.style?.color,
+ background: undefined,
+ gradient: undefined,
+ },
+ },
+} );
+
/**
* Filters registered block settings, extending attributes to include
* `backgroundColor` and `textColor` attribute.
@@ -414,6 +468,7 @@ export function ColorEdit( props ) {
style?.color?.text
).color,
isShownByDefault: defaultColorControls?.text,
+ resetAllFilter: resetAllTextFilter,
},
]
: [] ),
@@ -435,6 +490,7 @@ export function ColorEdit( props ) {
: undefined,
isShownByDefault:
defaultColorControls?.background,
+ resetAllFilter: clearBackgroundAndGradient,
},
]
: [] ),
@@ -450,6 +506,7 @@ export function ColorEdit( props ) {
clearable: !! style?.elements?.link?.color
?.text,
isShownByDefault: defaultColorControls?.link,
+ resetAllFilter: resetAllLinkFilter,
},
]
: [] ),
diff --git a/packages/block-editor/src/hooks/color.scss b/packages/block-editor/src/hooks/color.scss
index 2b79f03a50..f9b7898f94 100644
--- a/packages/block-editor/src/hooks/color.scss
+++ b/packages/block-editor/src/hooks/color.scss
@@ -20,4 +20,13 @@
row-gap: 0;
}
}
+
+ /**
+ * After converting PanelColorGradientSettings to render as a ToolsPanel
+ * we need to remove the top margin when wrapping inner content due to
+ * rendering via SlotFills.
+ */
+ .block-editor-tools-panel-color-gradient-settings__item.first {
+ margin-top: 0;
+ }
}
diff --git a/packages/block-library/src/social-links/edit.js b/packages/block-library/src/social-links/edit.js
index ed148909ad..f06f2ff42b 100644
--- a/packages/block-library/src/social-links/edit.js
+++ b/packages/block-library/src/social-links/edit.js
@@ -215,16 +215,17 @@ export function SocialLinksEdit( props ) {
__experimentalIsRenderedInSidebar
title={ __( 'Color' ) }
colorSettings={ colorSettings }
- />
- { ! logosOnly && (
- <ContrastChecker
- { ...{
- textColor: iconColorValue,
- backgroundColor: iconBackgroundColorValue,
- } }
- isLargeText={ false }
- />
- ) }
+ >
+ { ! logosOnly && (
+ <ContrastChecker
+ { ...{
+ textColor: iconColorValue,
+ backgroundColor: iconBackgroundColorValue,
+ } }
+ isLargeText={ false }
+ />
+ ) }
+ </PanelColorSettings>
</InspectorControls>
<ul { ...innerBlocksProps } />
</Fragment>
diff --git a/packages/components/src/tools-panel/styles.ts b/packages/components/src/tools-panel/styles.ts
index 715707a100..6dec56f1b5 100644
--- a/packages/components/src/tools-panel/styles.ts
+++ b/packages/components/src/tools-panel/styles.ts
@@ -54,8 +54,7 @@ export const ToolsPanelWithInnerWrapper = ( columns: number ) => {
};
export const ToolsPanelHiddenInnerWrapper = css`
- /* Required to meet specificity requirements */
- &&& > div:not( :first-of-type ) {
+ > div:not( :first-of-type ) {
display: none;
}
`;
Minor Issues
Cover Opacity
With the change to use ToolsPanel for PanelColorGradientSettings, the opacity control won't be included within the ToolsPanel menu and so "reset all" doesn't appear to reset all controls in the panel.
I have some code floating around locally where I tested out moving these Cover color related controls into the color block supports panel. I can create a PR doing just that I've created a draft PR for this ( #41102 ) assuming we revert the change this PR makes that prevents that. It will need some tweaks after this lands though.
Screen.Recording.2022-05-17.at.1.37.43.pm.mp4
Contrast Checker Outside Panel
After applying this PR, the contrast checker for the Social Links icon colors stretches the full width of the sidebar. No spacing or margin around it.
This can be fixed by rendering the contrast checker as a child of the PanelColorSettings in the Social Links block (change is also in the above diff).
packages/block-editor/src/components/colors-gradients/dropdown.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/colors-gradients/dropdown.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
Outdated
Show resolved
Hide resolved
jorgefilipecosta
force-pushed
the
update/refactor-panel-color-gradient-settings
branch
from
f8988ac to
e1ec461
Compare
|
Hi @aaronrobertshaw, thank you for the review and for providing a patch with your suggestions. I think your suggestions are in the right direction and I applied your patch 👍 Is there any other change you think we should do? I think your PR enhancing things on the cover block is nice and we should merge it as a follow-up. |
aaronrobertshaw
approved these changes
May 18, 2022
There was a problem hiding this comment.
I've re-tested this after the latest updates and it's working well for me 👍
✅ General color block supports still display and function correctly
✅ Cover block color panel works (opacity reset issue to be addressed via #41102)
✅ Social Links contrast checker displays within the related panel now
✅ Navigation block color controls work as before
✅ Tested injecting color controls into block supports panel via #41102
Is there any other change you think we should do?
I believe this PR covers all we need for now. Anything else should be able to be addressed via a follow-up.
jorgefilipecosta
force-pushed
the
update/refactor-panel-color-gradient-settings
branch
from
e1ec461 to
ed7c83c
Compare
jorgefilipecosta
deleted the
update/refactor-panel-color-gradient-settings
branch
|
Thank you a lot for the help in this PR @aaronrobertshaw! |
|
Looking at these latest changes, I can see This makes using it anywhere else impossible - I'd like to use this in a regular panel, in a custom block. My code is working with WP 5.9 and 6.0, but trying after this update nothing shows (naturally). Is it completely necessary to make the change preventing this from working outside a Would it make more sense to name the component (or offer a secondary version, wrapped in Its a really useful component and would love to carry on using it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates PanelColorGradientSettings to use the tools panel and makes the color support hook use PanelColorGradientSettings again.
PanelColorGradientSettings keeps almost the same API, only gets a new option isShownByDefault setting that allows one to hide a color option by default.
This refactor makes all the blocks using custom color colors core (cover, navigation, etc...) and the third-party blocks have the same color UI as the blocks relying on the color support hook unifying all the color picking interface.
With this change, we are also able to remove a considerable amount of code.
How?
I verified I could pick an overlay color for the cover, a color for a paragraph, and colors for navigation, remove the colors, and reset the color.