Skip to content

Commit

Permalink
fix: skip set_selection onChange events (#1443)
Browse files Browse the repository at this point in the history
  • Loading branch information
z0al authored Jul 12, 2023
1 parent 92c8dbd commit fb835a2
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 111 deletions.
7 changes: 3 additions & 4 deletions cypress/e2e/rich-text/RichTextEditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('Rich Text Editor', { viewportHeight: 2000 }, () => {
];

beforeEach(() => {
cy.viewport(1000, 2000);
cy.viewport(1280, 720);
richText = new RichTextPage();
richText.visit();
});
Expand Down Expand Up @@ -145,6 +145,7 @@ describe('Rich Text Editor', { viewportHeight: 2000 }, () => {
});

it('correctly undoes after drag&drop', () => {
cy.viewport(1200, 1200);
cy.shouldConfirm(true);
const paragraph = block(BLOCKS.PARAGRAPH, {}, text('some text.'));
const docBeforeDragAndDrop = doc(paragraph, entryBlock(), emptyParagraph());
Expand Down Expand Up @@ -390,9 +391,7 @@ describe('Rich Text Editor', { viewportHeight: 2000 }, () => {
getIframe().find('li').contains('some text more text');
});

// temporarily skipped. Snapshots don't match. Will be fixed in a follow up PR
// eslint-disable-next-line
it.skip('runs initial normalization without triggering a value change', () => {
it('runs initial normalization without triggering a value change', () => {
cy.setInitialValue(validDocumentThatRequiresNormalization);

cy.reload();
Expand Down
17 changes: 2 additions & 15 deletions packages/rich-text/src/RichTextEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,19 @@ import { FieldExtensionSDK } from '@contentful/app-sdk';
import { EntityProvider } from '@contentful/field-editor-reference';
import { FieldConnector } from '@contentful/field-editor-shared';
import * as Contentful from '@contentful/rich-text-types';
import { Document } from '@contentful/rich-text-types';
import { Plate, PlateProvider } from '@udecode/plate-core';
import { css, cx } from 'emotion';
import deepEquals from 'fast-deep-equal';
import noop from 'lodash/noop';

import { ContentfulEditorIdProvider, getContentfulEditorId } from './ContentfulEditorProvider';
import { createOnChangeCallback } from './helpers/callbacks';
import { toSlateValue } from './helpers/toSlateValue';
import { normalizeInitialValue } from './internal/misc';
import { getPlugins, disableCorePlugins } from './plugins';
import { RichTextTrackingActionHandler } from './plugins/Tracking';
import { styles } from './RichTextEditor.styles';
import { SdkProvider } from './SdkProvider';
import { SyncEditorValue } from './SyncEditorValue';
import { SyncEditorChanges } from './SyncEditorChanges';
import Toolbar from './Toolbar';
import StickyToolbarWrapper from './Toolbar/components/StickyToolbarWrapper';

Expand All @@ -44,8 +42,6 @@ export const ConnectedRichTextEditor = (props: ConnectedProps) => {
[sdk, onAction, restrictedMarks]
);

const handleChange = props.onChange;

const initialValue = React.useMemo(() => {
return normalizeInitialValue(
{
Expand All @@ -56,14 +52,6 @@ export const ConnectedRichTextEditor = (props: ConnectedProps) => {
);
}, [props.value, plugins]);

const onChange = React.useMemo(
() =>
createOnChangeCallback((document: Document) => {
handleChange?.(document);
}),
[handleChange]
);

const classNames = cx(
styles.editor,
props.minHeight !== undefined ? css({ minHeight: props.minHeight }) : undefined,
Expand All @@ -81,14 +69,13 @@ export const ConnectedRichTextEditor = (props: ConnectedProps) => {
initialValue={initialValue}
plugins={plugins}
disableCorePlugins={disableCorePlugins}
onChange={onChange}
>
{!props.isToolbarHidden && (
<StickyToolbarWrapper isDisabled={props.isDisabled}>
<Toolbar isDisabled={props.isDisabled} />
</StickyToolbarWrapper>
)}
<SyncEditorValue incomingValue={initialValue} />
<SyncEditorChanges incomingValue={initialValue} onChange={props.onChange} />
<Plate
id={id}
editableProps={{
Expand Down
78 changes: 78 additions & 0 deletions packages/rich-text/src/SyncEditorChanges.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import * as React from 'react';

import * as Contentful from '@contentful/rich-text-types';
import { usePlateActions } from '@udecode/plate-core';
import equal from 'fast-deep-equal';

import { createOnChangeCallback } from './helpers/callbacks';
import { usePlateSelectors } from './internal/hooks';
import { setEditorValue } from './internal/transforms';
import { Value } from './internal/types';

/**
* A hook responsible for keeping the editor state in sync with incoming
* changes (aka. external updates
*/
const useAcceptIncomingChanges = (incomingValue?: Value) => {
const editor = usePlateSelectors().editor();

// Cache latest editor value to avoid unnecessary updates
const lastIncomingValue = React.useRef(incomingValue);

React.useEffect(() => {
if (equal(lastIncomingValue.current, incomingValue)) {
return;
}

lastIncomingValue.current = incomingValue;
setEditorValue(editor, incomingValue);
}, [editor, incomingValue]);
};

/**
* Attaches a custom onChange callback that
*/
const useOnValueChanged = (onChange?: (doc: Contentful.Document) => unknown) => {
const editor = usePlateSelectors().editor();
const setEditorOnChange = usePlateActions().onChange();

React.useEffect(() => {
const cb = createOnChangeCallback(onChange);

setEditorOnChange({
fn: (document) => {
// Skip irrelevant events e.g. mouse selection
const operations = editor?.operations.filter((op) => {
return op.type !== 'set_selection';
});

if (operations.length === 0) {
return;
}

cb(document);
},
});
}, [editor, onChange, setEditorOnChange]);
};

export type SyncEditorStateProps = {
incomingValue?: Value;
onChange?: (doc: Contentful.Document) => unknown;
};

/**
* A void component that's responsible for keeping the editor
* state in sync with incoming changes (aka. external updates) and
* triggering onChange events.
*
* This component is a hack to work around the limitation of Plate v17+
* where we can no longer access the editor instance outside the Plate
* provider.
*/
export const SyncEditorChanges = ({ incomingValue, onChange }: SyncEditorStateProps) => {
useAcceptIncomingChanges(incomingValue);
useOnValueChanged(onChange);

return null;
};
66 changes: 0 additions & 66 deletions packages/rich-text/src/SyncEditorValue.tsx

This file was deleted.

23 changes: 4 additions & 19 deletions packages/rich-text/src/helpers/callbacks.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,20 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { toContentfulDocument } from '@contentful/contentful-slatejs-adapter';
import { Document } from '@contentful/rich-text-types';
import equal from 'fast-deep-equal';
import debounce from 'lodash/debounce';

import schema from '../constants/Schema';
import { removeInternalMarks } from './removeInternalMarks';

export const createOnChangeCallback = (handler?: (value: Document) => void) => {
// Cache previous value to avoid firing the handler unnecessarily
//
// Note: We are not using lodash/memoize here to avoid memory leaks
// due to having an infinite cache while we only care about the last
// value.
let cache: unknown = null;

return debounce((document: unknown) => {
if (equal(document, cache)) {
return;
}

cache = document;
export const createOnChangeCallback = (handler?: (value: Document) => void) =>
debounce((document: unknown) => {
const doc = removeInternalMarks(
toContentfulDocument({
// eslint-disable-next-line -- parameter type is not exported @typescript-eslint/no-explicit-any
document: document as any,
schema: schema,
// eslint-disable-next-line -- parameter type is not exported @typescript-eslint/no-explicit-any
}) as any
);
// eslint-disable-next-line -- correct parameter type is not defined @typescript-eslint/no-explicit-any

const cleanedDocument = removeInternalMarks(doc as Record<string, any>);
handler?.(cleanedDocument);
}, 500);
};
4 changes: 0 additions & 4 deletions packages/rich-text/src/internal/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ export const normalizeInitialValue = (
return editor.children;
};

export const withoutNormalizing = (editor: PlateEditor, fn: () => boolean | void) => {
return p.withoutNormalizing(editor, fn);
};

export const focusEditor = (editor: PlateEditor, target?: Location) => {
p.focusEditor(editor, target);
};
Expand Down
32 changes: 32 additions & 0 deletions packages/rich-text/src/internal/transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as p from '@udecode/plate-core';
import * as s from 'slate';
import { Except } from 'type-fest';

import { getEndPoint, isNode } from './queries';
import {
PlateEditor,
Node,
Expand Down Expand Up @@ -29,6 +30,10 @@ export const normalize = (
return p.normalizeEditor(editor, options);
};

export const withoutNormalizing = (editor: PlateEditor, fn: () => boolean | void) => {
return p.withoutNormalizing(editor, fn);
};

/**
* Set the selection to a location
*/
Expand Down Expand Up @@ -149,3 +154,30 @@ export const deleteFragment = (
) => {
return p.deleteFragment(editor, options);
};

/**
* Plate api doesn't allow to modify (easily) the editor value
* programmatically after the editor instance is created.
*
* This function is inspired by:
* https://github.com/udecode/plate/issues/1269#issuecomment-1057643622
*/
export const setEditorValue = (editor: PlateEditor, nodes?: Node[]): void => {
// Replaces editor content while keeping change history
withoutNormalizing(editor, () => {
const children = [...editor.children];
children.forEach((node) => editor.apply({ type: 'remove_node', path: [0], node }));

if (nodes) {
const nodesArray = isNode(nodes) ? [nodes] : nodes;
nodesArray.forEach((node, i) => {
editor.apply({ type: 'insert_node', path: [i], node });
});
}

const point = getEndPoint(editor, []);
if (point) {
select(editor, point);
}
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
import { getListTypes } from '@udecode/plate-list';

import { withoutNormalizing } from '../../../internal/misc';
import { withoutNormalizing } from '../../../internal';
import {
getNodeEntry,
getNodeChildren,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
import { isListNested, ELEMENT_LIC, getListItemEntry, moveListItemUp } from '@udecode/plate-list';

import { withoutNormalizing } from '../../../internal/misc';
import { withoutNormalizing } from '../../../internal';
import {
getNodeEntries,
getPluginType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { BLOCKS } from '@contentful/rich-text-types';
import { ELEMENT_LIC } from '@udecode/plate-list';
import { getListItemEntry } from '@udecode/plate-list';

import { withoutNormalizing } from '../../../internal';
import { ELEMENT_DEFAULT } from '../../../internal/constants';
import { withoutNormalizing } from '../../../internal/misc';
import {
findNode,
getNodeEntries,
Expand Down

0 comments on commit fb835a2

Please sign in to comment.