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.

Sign up for GitHub

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: consolidate applying from props #31635

Merged
merged 3 commits into from
May 9, 2021
Merged

Rich text: consolidate applying from props #31635

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72cdd1f
Rich text: consolidate applying from props
ellatrix May 9, 2021
f2172b7
Restore setting record on initial render
ellatrix May 9, 2021
1b818ec
Merge two effect hooks
ellatrix May 9, 2021
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
69 changes: 28 additions & 41 deletions packages/rich-text/src/component/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
useEffect,
useRef,
useState,
useMemo,
useLayoutEffect,
} from '@wordpress/element';
import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes';
Expand Down Expand Up @@ -104,14 +103,6 @@ function RichText(
multilineRootTag = TagName;
}

function getDoc() {
return ref.current.ownerDocument;
}

function getWin() {
return getDoc().defaultView;
}

/**
* Converts the outside data structure to our internal representation.
*
Expand Down Expand Up @@ -189,19 +180,11 @@ function RichText(
return toHTMLString( { value: val, multilineTag, preserveWhiteSpace } );
}

// Internal values are updated synchronously, unlike props and state.
const _value = useRef( value );
const record = useRef(
useMemo( () => {
const initialRecord = formatToValue( value );
initialRecord.start = selectionStart;
initialRecord.end = selectionEnd;
return initialRecord;
}, [] )
);

function createRecord() {
const selection = getWin().getSelection();
const {
ownerDocument: { defaultView },
} = ref.current;
const selection = defaultView.getSelection();
const range =
selection.rangeCount > 0 ? selection.getRangeAt( 0 ) : null;

Expand Down Expand Up @@ -328,8 +311,21 @@ function RichText(
handleEnter( event );
}

// Internal values are updated synchronously, unlike props and state.
const _value = useRef( value );
const record = useRef();
const lastHistoryValue = useRef( value );

function setRecordFromProps() {
record.current = formatToValue( value );
record.current.start = selectionStart;
record.current.end = selectionEnd;
}

if ( ! record.current ) {
setRecordFromProps();
}

function createUndoLevel() {
// If the content is the same, no level needs to be created.
if ( lastHistoryValue.current === _value.current ) {
Expand All @@ -340,8 +336,6 @@ function RichText(
lastHistoryValue.current = _value.current;
}

const didMount = useRef( false );

/**
* Sync the value to global state. The node tree and selection will also be
* updated if differences are found.
Expand Down Expand Up @@ -379,19 +373,23 @@ function RichText(
}
}

function applyFromProps() {
function applyFromProps( { domOnly } = {} ) {
_value.current = value;
record.current = formatToValue( value );
record.current.start = selectionStart;
record.current.end = selectionEnd;
applyRecord( record.current );
setRecordFromProps();
applyRecord( record.current, { domOnly } );
}

useEffect( () => {
const didMount = useRef( false );

useLayoutEffect( () => {
if ( didMount.current ) {
applyFromProps();
} else {
applyFromProps( { domOnly: true } );
}
}, [ TagName, placeholder ] );

didMount.current = true;
}, [ TagName, placeholder, ...dependencies ] );

useEffect( () => {
if ( didMount.current && value !== _value.current ) {
Expand Down Expand Up @@ -419,17 +417,6 @@ function RichText(
}
}, [ selectionStart, selectionEnd, isSelected ] );

useEffect( () => {
if ( didMount.current ) {
applyFromProps();
}
}, dependencies );

useLayoutEffect( () => {
applyRecord( record.current, { domOnly: true } );
didMount.current = true;
}, [] );

function focus() {
ref.current.focus();
applyRecord( record.current );
Expand Down