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

Rich text: only consider a format active if active at every selected index #48789

Merged
merged 5 commits into from
Mar 20, 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
34 changes: 2 additions & 32 deletions packages/block-editor/src/components/rich-text/format-edit.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
/**
* WordPress dependencies
*/
import {
getActiveFormat,
getActiveObject,
isCollapsed,
} from '@wordpress/rich-text';
import { getActiveFormat, getActiveObject } from '@wordpress/rich-text';

export default function FormatEdit( {
formatTypes,
Expand All @@ -22,37 +18,11 @@ export default function FormatEdit( {
}

const activeFormat = getActiveFormat( value, name );
let isActive = activeFormat !== undefined;
const isActive = activeFormat !== undefined;
const activeObject = getActiveObject( value );
const isObjectActive =
activeObject !== undefined && activeObject.type === name;

// Edge case: un-collapsed link formats.
// If there is a missing link format at either end of the selection
// then we shouldn't show the Edit UI because the selection has exceeded
// the bounds of the link format.
// Also if the format objects don't match then we're dealing with two separate
// links so we should not allow the link to be modified over the top.
if ( name === 'core/link' && ! isCollapsed( value ) ) {
const formats = value.formats;

const linkFormatAtStart = formats[ value.start ]?.find(
( { type } ) => type === 'core/link'
);

const linkFormatAtEnd = formats[ value.end - 1 ]?.find(
( { type } ) => type === 'core/link'
);

if (
! linkFormatAtStart ||
! linkFormatAtEnd ||
linkFormatAtStart !== linkFormatAtEnd
) {
isActive = false;
}
}

return (
<Edit
key={ name }
Expand Down
2 changes: 1 addition & 1 deletion packages/rich-text/src/get-active-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { getActiveFormats } from './get-active-formats';
* type, or undefined.
*/
export function getActiveFormat( value, formatType ) {
return getActiveFormats( value )?.find(
return getActiveFormats( value ).find(
( { type } ) => type === formatType
);
}
57 changes: 52 additions & 5 deletions packages/rich-text/src/get-active-formats.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
/** @typedef {import('./create').RichTextValue} RichTextValue */
/** @typedef {import('./create').RichTextFormatList} RichTextFormatList */

/**
* Internal dependencies
*/
import { isFormatEqual } from './is-format-equal';

/**
* Gets the all format objects at the start of the selection.
*
Expand All @@ -10,10 +15,8 @@
*
* @return {RichTextFormatList} Active format objects.
*/
export function getActiveFormats(
{ formats, start, end, activeFormats },
EMPTY_ACTIVE_FORMATS = []
) {
export function getActiveFormats( value, EMPTY_ACTIVE_FORMATS = [] ) {
const { formats, start, end, activeFormats } = value;
if ( start === undefined ) {
return EMPTY_ACTIVE_FORMATS;
}
Expand All @@ -37,5 +40,49 @@ export function getActiveFormats(
return formatsAfter;
}

return formats[ start ] || EMPTY_ACTIVE_FORMATS;
// If there's no formats at the start index, there are not active formats.
if ( ! formats[ start ] ) {
return EMPTY_ACTIVE_FORMATS;
}

const selectedFormats = formats.slice( start, end );

// Clone the formats so we're not mutating the live value.
const _activeFormats = [ ...selectedFormats[ 0 ] ];
let i = selectedFormats.length;

// For performance reasons, start from the end where it's much quicker to
// realise that there are no active formats.
while ( i-- ) {
const formatsAtIndex = selectedFormats[ i ];

// If we run into any index without formats, we're sure that there's no
// active formats.
if ( ! formatsAtIndex ) {
return EMPTY_ACTIVE_FORMATS;
}

let ii = _activeFormats.length;

// Loop over the active formats and remove any that are not present at
// the current index.
while ( ii-- ) {
const format = _activeFormats[ ii ];

if (
! formatsAtIndex.find( ( _format ) =>
isFormatEqual( format, _format )
)
) {
_activeFormats.splice( ii, 1 );
}
}

// If there are no active formats, we can stop.
if ( _activeFormats.length === 0 ) {
return EMPTY_ACTIVE_FORMATS;
}
}

return _activeFormats || EMPTY_ACTIVE_FORMATS;
}
4 changes: 1 addition & 3 deletions packages/rich-text/src/test/__snapshots__/to-dom.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ exports[`recordToDom should create a value with image object and formatting 1`]

exports[`recordToDom should create a value with image object and text after 1`] = `
<body>
<em
data-rich-text-format-boundary="true"
>
<em>

<img
src=""
Expand Down
15 changes: 13 additions & 2 deletions packages/rich-text/src/test/get-active-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,28 @@ describe( 'getActiveFormat', () => {
expect( getActiveFormat( record, 'em' ) ).toBe( undefined );
} );

it( 'should return format at first character for uncollapsed selection', () => {
it( 'should return format when active over whole selection', () => {
const record = {
formats: [ [ em ], [ strong ], , ],
text: 'one',
start: 0,
end: 2,
end: 1,
};

expect( getActiveFormat( record, 'em' ) ).toBe( em );
} );

it( 'should return not return format when not active over whole selection', () => {
const record = {
formats: [ [ em ], [ strong ], , ],
text: 'one',
start: 0,
end: 2,
};

expect( getActiveFormat( record, 'em' ) ).toBe( undefined );
} );

it( 'should return undefined if at the boundary before', () => {
const record = {
formats: [ [ em ], , [ em ] ],
Expand Down
9 changes: 6 additions & 3 deletions packages/rich-text/src/test/toggle-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@ describe( 'toggleFormat', () => {
const strong = { type: 'strong' };
const em = { type: 'em' };

it( 'should remove format if it exists at start of selection', () => {
it( 'should remove format if it is active', () => {
const record = {
formats: [
,
,
,
[ strong ],
// In reality, formats at a different index are never the same
// value. Only formats that create the same tag are the same
// value.
[ { type: 'strong' } ],
[ em, strong ],
[ em, strong ],
[ em ],
[ em ],
,
,
Expand Down