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

fix: skip set_selection onChange events #1443

Merged
merged 6 commits into from
Jul 12, 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
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
z0al marked this conversation as resolved.
Show resolved Hide resolved
*/
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]);
z0al marked this conversation as resolved.
Show resolved Hide resolved
};

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