Fix annotations text tracking

docs/data/data-core-annotations.md

@@ -75,6 +75,16 @@ Removes an annotation with a specific ID.
 
  * annotationId: The annotation to remove.
 
+### __experimentalUpdateAnnotationRange
+
+Updates the range of an annotation.
+
+*Parameters*
+
+ * annotationId: ID of the annotation to update.
+ * start: The start of the new range.
+ * end: The end of the new range.
+
 ### __experimentalRemoveAnnotationsBySource
 
 Removes all annotations of a specific source.

packages/annotations/src/format/annotation.js

@@ -2,13 +2,12 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
+import { applyFormat, removeFormat } from '@wordpress/rich-text';
 
-const name = 'core/annotation';
+const FORMAT_NAME = 'core/annotation';
 
-/**
- * WordPress dependencies
- */
-import { applyFormat, removeFormat } from '@wordpress/rich-text';
+const ANNOTATION_ATTRIBUTE_PREFIX = 'annotation-text-';
+const STORE_KEY = 'core/annotations';
 
 /**
  * Applies given annotations to the given record.
@@ -29,11 +28,17 @@ export function applyAnnotations( record, annotations = [] ) {
 			end = record.text.length;
 		}
 
-		const className = 'annotation-text-' + annotation.source;
+		const className = ANNOTATION_ATTRIBUTE_PREFIX + annotation.source;
+		const id = ANNOTATION_ATTRIBUTE_PREFIX + annotation.id;
 
 		record = applyFormat(
 			record,
-			{ type: 'core/annotation', attributes: { className } },
+			{
+				type: FORMAT_NAME, attributes: {
+					className,
+					id,
+				},
+			},
 			start,
 			end
 		);
@@ -52,31 +57,104 @@ export function removeAnnotations( record ) {
 	return removeFormat( record, 'core/annotation', 0, record.text.length );
 }
 
+/**
+ * Retrieves the positions of annotations inside an array of formats.
+ *
+ * @param {Array} formats Formats with annotations in there.
+ * @return {Object} ID keyed positions of annotations.
+ */
+function retrieveAnnotationPositions( formats ) {
+	const positions = {};
+
+	formats.forEach( ( characterFormats, i ) => {
+		characterFormats = characterFormats || [];
+		characterFormats = characterFormats.filter( ( format ) => format.type === FORMAT_NAME );
+		characterFormats.forEach( ( format ) => {
+			let { id } = format.attributes;
+			id = id.replace( ANNOTATION_ATTRIBUTE_PREFIX, '' );
+
+			if ( ! positions.hasOwnProperty( id ) ) {
+				positions[ id ] = {
+					start: i,
+				};
+			}
+
+			// Annotations refer to positions between characters.
+			// Formats refer to the character themselves.
+			// So we need to adjust for that here.
+			positions[ id ].end = i + 1;
+		} );
+	} );
+
+	return positions;
+}
+
+/**
+ * Updates annotations in the state based on positions retrieved from RichText.
+ *
+ * @param {Array}    annotations           The annotations that are currently applied.
+ * @param {Array}    positions             The current positions of the given annotations.
+ * @param {Function} removeAnnotation      Function to remove an annotation from the state.
+ * @param {Function} updateAnnotationRange Function to update an annotation range in the state.
+ */
+function updateAnnotationsWithPositions( annotations, positions, { removeAnnotation, updateAnnotationRange } ) {
+	annotations.forEach( ( currentAnnotation ) => {
+		const position = positions[ currentAnnotation.id ];
+		// If we cannot find an annotation, delete it.
+		if ( ! position ) {
+			// Apparently the annotation has been removed, so remove it from the state:
+			// Remove...
+			removeAnnotation( currentAnnotation.id );
+			return;
+		}
+
+		const { start, end } = currentAnnotation;
+		if ( start !== position.start || end !== position.end ) {
+			updateAnnotationRange( currentAnnotation.id, position.start, position.end );
+		}
+	} );
+}
+
 export const annotation = {
-	name,
+	name: FORMAT_NAME,
 	title: __( 'Annotation' ),
 	tagName: 'mark',
 	className: 'annotation-text',
 	attributes: {
 		className: 'class',
+		id: 'id',
 	},
 	edit() {
 		return null;
 	},
 	__experimentalGetPropsForEditableTreePreparation( select, { richTextIdentifier, blockClientId } ) {
 		return {
-			annotations: select( 'core/annotations' ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ),
+			annotations: select( STORE_KEY ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ),
 		};
 	},
-	__experimentalCreatePrepareEditableTree( props ) {
+	__experimentalCreatePrepareEditableTree( { annotations } ) {
 		return ( formats, text ) => {
-			if ( props.annotations.length === 0 ) {
+			if ( annotations.length === 0 ) {
 				return formats;
 			}
 
 			let record = { formats, text };
-			record = applyAnnotations( record, props.annotations );
+			record = applyAnnotations( record, annotations );
 			return record.formats;
 		};
 	},
+	__experimentalGetPropsForEditableTreeChangeHandler( dispatch ) {
+		return {
+			removeAnnotation: dispatch( STORE_KEY ).__experimentalRemoveAnnotation,
+			updateAnnotationRange: dispatch( STORE_KEY ).__experimentalUpdateAnnotationRange,
+		};
+	},
+	__experimentalCreateOnChangeEditableValue( props ) {
+		return ( formats ) => {
+			const positions = retrieveAnnotationPositions( formats );
+			const { removeAnnotation, updateAnnotationRange, annotations } = props;
+
+			updateAnnotationsWithPositions( annotations, positions, { removeAnnotation, updateAnnotationRange } );
+		};
+	},
 };

packages/annotations/src/store/actions.js

@@ -57,6 +57,24 @@ export function __experimentalRemoveAnnotation( annotationId ) {
 	};
 }
 
+/**
+ * Updates the range of an annotation.
+ *
+ * @param {string} annotationId ID of the annotation to update.
+ * @param {number} start The start of the new range.
+ * @param {number} end The end of the new range.
+ *
+ * @return {Object} Action object.
+ */
+export function __experimentalUpdateAnnotationRange( annotationId, start, end ) {
+	return {
+		type: 'ANNOTATION_UPDATE_RANGE',
+		annotationId,
+		start,
+		end,
+	};
+}
+
 /**
  * Removes all annotations of a specific source.
  *

packages/annotations/src/store/reducer.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { isNumber, mapValues } from 'lodash';
+import { get, isNumber, mapValues } from 'lodash';
 
 /**
  * Filters an array based on the predicate, but keeps the reference the same if
@@ -38,7 +38,7 @@ function isValidAnnotationRange( annotation ) {
  *
  * @return {Array} Updated state.
  */
-export function annotations( state = { all: [], byBlockClientId: {} }, action ) {
+export function annotations( state = {}, action ) {
 	switch ( action.type ) {
 		case 'ANNOTATION_ADD':
 			const blockClientId = action.blockClientId;
@@ -55,52 +55,48 @@ export function annotations( state = { all: [], byBlockClientId: {} }, action )
 				return state;
 			}
 
-			const previousAnnotationsForBlock = state.byBlockClientId[ blockClientId ] || [];
+			const previousAnnotationsForBlock = get( state, blockClientId, [] );
 
 			return {
-				all: [
-					...state.all,
-					newAnnotation,
-				],
-				byBlockClientId: {
-					...state.byBlockClientId,
-					[ blockClientId ]: [ ...previousAnnotationsForBlock, action.id ],
-				},
+				...state,
+				[ blockClientId ]: [ ...previousAnnotationsForBlock, newAnnotation ],
 			};
 
 		case 'ANNOTATION_REMOVE':
-			return {
-				all: state.all.filter( ( annotation ) => annotation.id !== action.annotationId ),
+			return mapValues( state, ( annotationsForBlock ) => {
+				return filterWithReference( annotationsForBlock, ( annotation ) => {
+					return annotation.id !== action.annotationId;
+				} );
+			} );
 
-				// We use filterWithReference to not refresh the reference if a block still has
-				// the same annotations.
-				byBlockClientId: mapValues( state.byBlockClientId, ( annotationForBlock ) => {
-					return filterWithReference( annotationForBlock, ( annotationId ) => {
-						return annotationId !== action.annotationId;
-					} );
-				} ),
-			};
+		case 'ANNOTATION_UPDATE_RANGE':
+			return mapValues( state, ( annotationsForBlock ) => {
+				let hasChangedRange = false;
 
-		case 'ANNOTATION_REMOVE_SOURCE':
-			const idsToRemove = [];
+				const newAnnotations = annotationsForBlock.map( ( annotation ) => {
+					if ( annotation.id === action.annotationId ) {
+						hasChangedRange = true;
+						return {
+							...annotation,
+							range: {
+								start: action.start,
+								end: action.end,
+							},
+						};
+					}
 
-			const allAnnotations = state.all.filter( ( annotation ) => {
-				if ( annotation.source === action.source ) {
-					idsToRemove.push( annotation.id );
-					return false;
-				}
+					return annotation;
+				} );
 
-				return true;
+				return hasChangedRange ? newAnnotations : annotationsForBlock;
 			} );
 
-			return {
-				all: allAnnotations,
-				byBlockClientId: mapValues( state.byBlockClientId, ( annotationForBlock ) => {
-					return filterWithReference( annotationForBlock, ( annotationId ) => {
-						return ! idsToRemove.includes( annotationId );
-					} );
-				} ),
-			};
+		case 'ANNOTATION_REMOVE_SOURCE':
+			return mapValues( state, ( annotationsForBlock ) => {
+				return filterWithReference( annotationsForBlock, ( annotation ) => {
+					return annotation.source !== action.source;
+				} );
+			} );
 	}
 
 	return state;

packages/annotations/src/store/selectors.js

@@ -2,6 +2,7 @@
  * External dependencies
  */
 import createSelector from 'rememo';
+import { get, flatMap } from 'lodash';
 
 /**
  * Returns the annotations for a specific client ID.
@@ -13,15 +14,19 @@ import createSelector from 'rememo';
  */
 export const __experimentalGetAnnotationsForBlock = createSelector(
 	( state, blockClientId ) => {
-		return state.all.filter( ( annotation ) => {
-			return annotation.selector === 'block' && annotation.blockClientId === blockClientId;
+		return get( state, blockClientId, [] ).filter( ( annotation ) => {
+			return annotation.selector === 'block';
 		} );
 	},
 	( state, blockClientId ) => [
-		state.byBlockClientId[ blockClientId ],
+		get( state, blockClientId, [] ),
 	]
 );
 
+export const __experimentalGetAllAnnotationsForBlock = function( state, blockClientId ) {
+	return get( state, blockClientId, [] );
+};
+
 /**
  * Returns the annotations that apply to the given RichText instance.
  *
@@ -36,9 +41,8 @@ export const __experimentalGetAnnotationsForBlock = createSelector(
  */
 export const __experimentalGetAnnotationsForRichText = createSelector(
 	( state, blockClientId, richTextIdentifier ) => {
-		return state.all.filter( ( annotation ) => {
+		return get( state, blockClientId, [] ).filter( ( annotation ) => {
 			return annotation.selector === 'range' &&
-				annotation.blockClientId === blockClientId &&
 				richTextIdentifier === annotation.richTextIdentifier;
 		} ).map( ( annotation ) => {
 			const { range, ...other } = annotation;
@@ -50,7 +54,7 @@ export const __experimentalGetAnnotationsForRichText = createSelector(
 		} );
 	},
 	( state, blockClientId ) => [
-		state.byBlockClientId[ blockClientId ],
+		get( state, blockClientId, [] ),
 	]
 );
 
@@ -61,5 +65,7 @@ export const __experimentalGetAnnotationsForRichText = createSelector(
  * @return {Array} All annotations currently applied.
  */
 export function __experimentalGetAnnotations( state ) {
-	return state.all;
+	return flatMap( state, ( annotations ) => {
+		return annotations;
+	} );
 }