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

Add option to add text color to specific text inside RichText #16014

Merged
merged 1 commit into from
Feb 7, 2020
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
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ const FormatToolbar = () => {
return (
<div className="block-editor-format-toolbar">
<Toolbar>
{ [ 'bold', 'italic', 'link' ].map( ( format ) => (
<Slot
name={ `RichText.ToolbarControls.${ format }` }
key={ format }
/>
) ) }
{ [ 'bold', 'italic', 'link', 'text-color' ].map(
( format ) => (
<Slot
name={ `RichText.ToolbarControls.${ format }` }
key={ format }
/>
)
) }
<Slot name="RichText.ToolbarControls">
{ ( fills ) =>
fills.length !== 0 && (
Expand Down
1 change: 1 addition & 0 deletions packages/format-library/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"@babel/runtime": "^7.8.3",
"@wordpress/block-editor": "file:../block-editor",
"@wordpress/components": "file:../components",
"@wordpress/data": "file:../data",
"@wordpress/dom": "file:../dom",
"@wordpress/element": "file:../element",
"@wordpress/html-entities": "file:../html-entities",
Expand Down
12 changes: 11 additions & 1 deletion packages/format-library/src/default-formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,15 @@ import { italic } from './italic';
import { link } from './link';
import { strikethrough } from './strikethrough';
import { underline } from './underline';
import { textColor } from './text-color';

export default [ bold, code, image, italic, link, strikethrough, underline ];
export default [
bold,
code,
image,
italic,
link,
strikethrough,
underline,
textColor,
];
1 change: 1 addition & 0 deletions packages/format-library/src/style.scss
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
@import "./image/style.scss";
@import "./link/style.scss";
@import "./text-color/style.scss";
94 changes: 94 additions & 0 deletions packages/format-library/src/text-color/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* External dependencies
*/
import { get } from 'lodash';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { useCallback, useMemo, useState } from '@wordpress/element';
import { RichTextToolbarButton } from '@wordpress/block-editor';
import { Dashicon } from '@wordpress/components';

/**
* Internal dependencies
*/
import { default as InlineColorUI, getActiveColor } from './inline';

const name = 'core/text-color';
const title = __( 'Text Color' );

const EMPTY_ARRAY = [];

function TextColorEdit( { value, onChange, isActive, activeAttributes } ) {
const colors = useSelect( ( select ) => {
const { getSettings } = select( 'core/block-editor' );
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way not to depend on the the block editor package?

Copy link
Member

Choose a reason for hiding this comment

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

I tried to make at least nothing crashes if core/block-editor is not available. I guess as a follow up we may try to see if a better solution is possible.

if ( getSettings ) {
return get( getSettings(), [ 'colors' ], EMPTY_ARRAY );
}
return EMPTY_ARRAY;
} );
const [ isAddingColor, setIsAddingColor ] = useState( false );
const enableIsAddingColor = useCallback( () => setIsAddingColor( true ), [
setIsAddingColor,
] );
const disableIsAddingColor = useCallback( () => setIsAddingColor( false ), [
setIsAddingColor,
] );
const colorIndicatorStyle = useMemo( () => {
const activeColor = getActiveColor( name, value, colors );
if ( ! activeColor ) {
return undefined;
}
return {
backgroundColor: activeColor,
};
}, [ value, colors ] );
return (
<>
<RichTextToolbarButton
key={ isActive ? 'text-color' : 'text-color-not-active' }
className="format-library-text-color-button"
name={ isActive ? 'text-color' : undefined }
Copy link
Member

Choose a reason for hiding this comment

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

@jorgefilipecosta - what's even more surprising is that the name and key discussed here is going to be RichText.ToolbarControls.undefined from time to time. It all needs to be further investigated.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no Slot for RichText.ToolbarControls.undefined but we should just not render this component instead.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @gziolo, @epiqueras, the key is not going to be RichText.ToolbarControls.undefined, because the key is the fillName and fill name has the following logic:

	let fillName = 'RichText.ToolbarControls';

	if ( name ) {
		fillName += `.${ name }`;
	}

So here when we pass undefined the key is going to be RichText.ToolbarControl.

Copy link
Member

Choose a reason for hiding this comment

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

We need this logic because we want to show the format on the main toolbar when there is a color set, but we want to hide it from the main toolbar when color is not set.

Copy link
Member

Choose a reason for hiding this comment

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

@epiqueras, @gziolo I think we may have a bug in slot fill, this case basically passes a dynamic name to the fill and maybe slot fill when the name changes does not remove previous fills. Setting a key seems to be a workaround for the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

So here when we pass undefined the key is going to be RichText.ToolbarControl.

So it will be in the dropdown.

I think we may have a bug in slot fill, this case basically passes a dynamic name to the fill and maybe slot fill when the name changes does not remove previous fills. Setting a key seems to be a workaround for the issue.

Yes, see #16014 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

We should address the bug in slot&fill. But I think using a key is an ok workaround so we can merge this PR in WordPress 5.4. A key in this situation does not seem to cause problems. Any thoughts on this @gziolo, @epiqueras?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but I’d rather have the key in the button itself to avoid this pattern from masking the need for a proper fix.

icon={
<>
<Dashicon icon="editor-textcolor" />
{ isActive && (
<span
className="format-library-text-color-button__indicator"
style={ colorIndicatorStyle }
/>
) }
</>
}
title={ title }
onClick={ enableIsAddingColor }
/>
{ isAddingColor && (
<InlineColorUI
name={ name }
addingColor={ isAddingColor }
onClose={ disableIsAddingColor }
isActive={ isActive }
activeAttributes={ activeAttributes }
value={ value }
onChange={ onChange }
/>
) }
</>
);
}

export const textColor = {
name,
title,
tagName: 'span',
className: 'has-inline-color',
Copy link
Member

Choose a reason for hiding this comment

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

I was hoping a class wouldn't be needed when we do this:
#15478

But I don't think it should block this feature. Let's work that out later.

attributes: {
style: 'style',
class: 'class',
},
edit: TextColorEdit,
};
137 changes: 137 additions & 0 deletions packages/format-library/src/text-color/inline.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/**
* External dependencies
*/
import { get } from 'lodash';

/**
* WordPress dependencies
*/
import { useCallback, useMemo } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { withSpokenMessages } from '@wordpress/components';
import { getRectangleFromRange } from '@wordpress/dom';
import {
applyFormat,
removeFormat,
getActiveFormat,
} from '@wordpress/rich-text';
import {
ColorPalette,
URLPopover,
getColorClassName,
getColorObjectByColorValue,
getColorObjectByAttributeValues,
} from '@wordpress/block-editor';

export function getActiveColor( formatName, formatValue, colors ) {
const activeColorFormat = getActiveFormat( formatValue, formatName );
if ( ! activeColorFormat ) {
return;
}
const styleColor = activeColorFormat.attributes.style;
if ( styleColor ) {
return styleColor.replace( new RegExp( `^color:\\s*` ), '' );
}
const currentClass = activeColorFormat.attributes.class;
if ( currentClass ) {
const colorSlug = currentClass.replace( /.*has-(.*?)-color.*/, '$1' );
return getColorObjectByAttributeValues( colors, colorSlug ).color;
}
}

const ColorPopoverAtLink = ( { isActive, addingColor, value, ...props } ) => {
const anchorRect = useMemo( () => {
const selection = window.getSelection();
const range =
selection.rangeCount > 0 ? selection.getRangeAt( 0 ) : null;
if ( ! range ) {
return;
}

if ( addingColor ) {
return getRectangleFromRange( range );
}

let element = range.startContainer;

// If the caret is right before the element, select the next element.
element = element.nextElementSibling || element;

while ( element.nodeType !== window.Node.ELEMENT_NODE ) {
element = element.parentNode;
}

const closest = element.closest( 'span' );
if ( closest ) {
return closest.getBoundingClientRect();
}
}, [ isActive, addingColor, value.start, value.end ] );

if ( ! anchorRect ) {
return null;
}

return <URLPopover anchorRect={ anchorRect } { ...props } />;
};

const ColorPicker = ( { name, value, onChange } ) => {
const colors = useSelect( ( select ) => {
const { getSettings } = select( 'core/block-editor' );
return get( getSettings(), [ 'colors' ], [] );
} );
const onColorChange = useCallback(
( color ) => {
if ( color ) {
const colorObject = getColorObjectByColorValue( colors, color );
onChange(
applyFormat( value, {
type: name,
attributes: colorObject
? {
class: getColorClassName(
'color',
colorObject.slug
),
}
: {
style: `color:${ color }`,
},
} )
);
} else {
onChange( removeFormat( value, name ) );
}
},
[ colors, onChange ]
);
const activeColor = useMemo( () => getActiveColor( name, value, colors ), [
name,
value,
colors,
] );

return <ColorPalette value={ activeColor } onChange={ onColorChange } />;
};

const InlineColorUI = ( {
name,
value,
onChange,
onClose,
isActive,
addingColor,
} ) => {
return (
<ColorPopoverAtLink
value={ value }
isActive={ isActive }
addingColor={ addingColor }
onClose={ onClose }
className="components-inline-color-popover"
>
<ColorPicker name={ name } value={ value } onChange={ onChange } />
</ColorPopoverAtLink>
);
};

export default withSpokenMessages( InlineColorUI );
43 changes: 43 additions & 0 deletions packages/format-library/src/text-color/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
.components-inline-color__indicator {
position: absolute;
background: #000;
height: 3px;
width: 20px;
bottom: 6px;
left: auto;
right: auto;
margin: 0 5px;
}

.components-inline-color-popover {

.components-popover__content {
padding: 20px 18px;

.components-color-palette {
margin-top: 0.6rem;
}

.components-base-control__title {
display: block;
margin-bottom: 16px;
font-weight: 600;
color: #191e23;
}

.component-color-indicator {
vertical-align: text-bottom;
}
}
}

.format-library-text-color-button {
position: relative;
}
.format-library-text-color-button__indicator {
height: 4px;
width: 20px;
position: absolute;
bottom: 6px;
left: 8px;
}