From e57138730ce85622ac65ae02b70e7e9e7fbafee0 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 12 Nov 2021 16:11:12 +0900 Subject: [PATCH] Cleanup --- .../header-toolbar/index.js | 3 +- .../use-yjs/__tests__/undo.js | 2 +- .../use-yjs/filters/block-selection/index.js | 2 +- .../collaborative-editing/use-yjs/index.js | 29 +++------- .../collaborative-editing/use-yjs/yjs-doc.js | 57 ++++++++----------- src/store/collab/controls.js | 2 +- src/store/peers/selectors.js | 4 -- 7 files changed, 37 insertions(+), 62 deletions(-) diff --git a/src/components/block-editor-toolbar/header-toolbar/index.js b/src/components/block-editor-toolbar/header-toolbar/index.js index 72bd07aeb..a130dcf1f 100644 --- a/src/components/block-editor-toolbar/header-toolbar/index.js +++ b/src/components/block-editor-toolbar/header-toolbar/index.js @@ -54,8 +54,7 @@ function HeaderToolbar( props ) { }, [] ); const isLargeViewport = useViewportMatch( 'medium' ); const isWideViewport = useViewportMatch( 'wide' ); - const { inserter, toc, navigation, undo: undoSetting, selectorTool } = props.settings.iso.toolbar; - const undo = undoSetting; + const { inserter, toc, navigation, undo, selectorTool } = props.settings.iso.toolbar; const displayBlockToolbar = ! isLargeViewport || previewDeviceType !== 'Desktop' || hasFixedToolbar; const toolbarAriaLabel = displayBlockToolbar ? /* translators: accessibility text for the editor toolbar when Top Toolbar is on */ diff --git a/src/components/collaborative-editing/use-yjs/__tests__/undo.js b/src/components/collaborative-editing/use-yjs/__tests__/undo.js index 1870aff56..69bbf1787 100644 --- a/src/components/collaborative-editing/use-yjs/__tests__/undo.js +++ b/src/components/collaborative-editing/use-yjs/__tests__/undo.js @@ -120,8 +120,8 @@ describe( 'CollaborativeEditing: Undo/Redo', () => { userEvent.click( bobScreen.getByRole( 'button', { name: 'Undo' } ) ); expect( screen.queryByText( 'bob:' ) ).toBe( null ); - expect( screen.getByText( 'alice:hello' ) ).toBeInTheDocument(); + userEvent.click( aliceScreen.getByRole( 'button', { name: 'Undo' } ) ); expect( screen.queryByText( 'hello' ) ).toBe( null ); diff --git a/src/components/collaborative-editing/use-yjs/filters/block-selection/index.js b/src/components/collaborative-editing/use-yjs/filters/block-selection/index.js index f52c813aa..278182b11 100644 --- a/src/components/collaborative-editing/use-yjs/filters/block-selection/index.js +++ b/src/components/collaborative-editing/use-yjs/filters/block-selection/index.js @@ -45,5 +45,5 @@ const addSelectionBorders = ( OriginalComponent ) => { }; export const addFilterCollabBlockSelection = () => { - addFilter( 'editor.BlockListBlock', 'isolated-block-editor/collab', addSelectionBorders, 9 ); + addFilter( 'editor.BlockListBlock', 'isolated-block-editor', addSelectionBorders, 9 ); }; diff --git a/src/components/collaborative-editing/use-yjs/index.js b/src/components/collaborative-editing/use-yjs/index.js index e9bf49ac8..e7dd20ecf 100644 --- a/src/components/collaborative-editing/use-yjs/index.js +++ b/src/components/collaborative-editing/use-yjs/index.js @@ -2,7 +2,7 @@ * External dependencies */ import { v4 as uuidv4 } from 'uuid'; -import { noop, sample } from 'lodash'; +import { noop, once, sample } from 'lodash'; /** * Internal dependencies @@ -45,7 +45,7 @@ async function initYDoc( { settings, registry } ) { const doc = createDocument( { identity, - applyDataChanges: updatePostDoc, + applyChangesToYDoc: updatePostDoc, getData: postDocToObject, /** @param {Object} message */ sendMessage: ( message ) => { @@ -54,25 +54,15 @@ async function initYDoc( { settings, registry } ) { }, } ); - let isUndoManagerReady = false; - - doc.onStateChange( ( newState ) => { - // Set up undo manager only once per Yjs doc - if ( newState === 'on' && ! isUndoManagerReady ) { - setupUndoManager( doc.getPostMap(), identity, registry ); + doc.onConnectionReady( + once( () => { dispatch( 'isolated/editor' ).setYDoc( doc ); + setupUndoManager( doc.getPostMap(), identity, registry ); + } ) + ); - isUndoManagerReady = true; - } - } ); - - doc.onRemoteDataChange( ( changes ) => { - debug( 'remote change received by ydoc', changes ); - dispatch( 'isolated/editor' ).updateBlocksWithUndo( changes.blocks, { isTriggeredByYDoc: true } ); - } ); - - doc.onUndoRedo( ( changes ) => { - debug( 'ydoc updated by undo manager', changes ); + doc.onYDocTriggeredChange( ( changes ) => { + debug( 'changes triggered by ydoc, applying to editor state', changes ); dispatch( 'isolated/editor' ).updateBlocksWithUndo( changes.blocks, { isTriggeredByYDoc: true } ); } ); @@ -137,7 +127,6 @@ async function initYDoc( { settings, registry } ) { }, } ); }, - undoManager: doc.undoManager, disconnect: () => { window.removeEventListener( 'beforeunload', disconnect ); disconnect(); diff --git a/src/components/collaborative-editing/use-yjs/yjs-doc.js b/src/components/collaborative-editing/use-yjs/yjs-doc.js index edcd5e415..d517af053 100644 --- a/src/components/collaborative-editing/use-yjs/yjs-doc.js +++ b/src/components/collaborative-editing/use-yjs/yjs-doc.js @@ -13,33 +13,29 @@ const decodeArray = ( string ) => new Uint8Array( string.split( ',' ) ); * * @param {Object} opts * @param {string} opts.identity - Client identifier. - * @param {function(yjs.Doc, PostObject): void} opts.applyDataChanges - Function to apply changes to the Yjs doc. + * @param {function(yjs.Doc, PostObject): void} opts.applyChangesToYDoc - Function to apply changes to the Yjs doc. * @param {function(yjs.Doc): PostObject} opts.getData - Function to get post object data from the Yjs doc. * @param {function(any): void} opts.sendMessage */ -export function createDocument( { identity, applyDataChanges, getData, sendMessage } ) { +export function createDocument( { identity, applyChangesToYDoc, getData, sendMessage } ) { const doc = new yjs.Doc(); + /** @type {'off'|'connecting'|'on'} */ let state = 'off'; - let remoteChangeListeners = []; - let stateListeners = []; - let undoRedoListeners = []; + let yDocTriggeredChangeListeners = []; + let connectionReadyListeners = []; doc.on( 'update', ( update, origin ) => { - const isRemoteOrigin = origin !== identity && ! ( origin instanceof yjs.UndoManager ); - - if ( isRemoteOrigin ) { + // Change received from peer, or triggered by self undo/redo + if ( origin !== identity ) { const newData = getData( doc ); - remoteChangeListeners.forEach( ( listener ) => listener( newData ) ); - return; + yDocTriggeredChangeListeners.forEach( ( listener ) => listener( newData ) ); } - if ( origin instanceof yjs.UndoManager ) { - const newData = getData( doc ); - undoRedoListeners.forEach( ( listener ) => listener( newData ) ); - } + const isLocalOrigin = origin === identity || origin instanceof yjs.UndoManager; - if ( ! isRemoteOrigin && state === 'on' ) { + // Change should be broadcast to peers + if ( isLocalOrigin && state === 'on' ) { sendMessage( { protocol: 'yjs1', messageType: 'syncUpdate', @@ -50,7 +46,10 @@ export function createDocument( { identity, applyDataChanges, getData, sendMessa const setState = ( newState ) => { state = newState; - stateListeners.forEach( ( listener ) => listener( newState ) ); + + if ( newState === 'on' ) { + connectionReadyListeners.forEach( ( listener ) => listener() ); + } }; const sync = ( destination, isReply ) => { @@ -66,12 +65,12 @@ export function createDocument( { identity, applyDataChanges, getData, sendMessa }; return { - applyDataChanges( data ) { + applyChangesToYDoc( data ) { if ( state !== 'on' ) { throw 'wrong state'; } doc.transact( () => { - applyDataChanges( doc, data ); + applyChangesToYDoc( doc, data ); }, identity ); }, @@ -94,7 +93,7 @@ export function createDocument( { identity, applyDataChanges, getData, sendMessa } setState( 'on' ); - this.applyDataChanges( data ); + this.applyChangesToYDoc( data ); }, receiveMessage( { protocol, messageType, origin, ...content } ) { @@ -130,27 +129,19 @@ export function createDocument( { identity, applyDataChanges, getData, sendMessa } }, - onRemoteDataChange( listener ) { - remoteChangeListeners.push( listener ); - - return () => { - remoteChangeListeners = remoteChangeListeners.filter( ( l ) => l !== listener ); - }; - }, - - onStateChange( listener ) { - stateListeners.push( listener ); + onYDocTriggeredChange( listener ) { + yDocTriggeredChangeListeners.push( listener ); return () => { - stateListeners = stateListeners.filter( ( l ) => l !== listener ); + yDocTriggeredChangeListeners = yDocTriggeredChangeListeners.filter( ( l ) => l !== listener ); }; }, - onUndoRedo( listener ) { - undoRedoListeners.push( listener ); + onConnectionReady( listener ) { + connectionReadyListeners.push( listener ); return () => { - undoRedoListeners = undoRedoListeners.filter( ( l ) => l !== listener ); + connectionReadyListeners = connectionReadyListeners.filter( ( l ) => l !== listener ); }; }, diff --git a/src/store/collab/controls.js b/src/store/collab/controls.js index 45804e863..e159baba0 100644 --- a/src/store/collab/controls.js +++ b/src/store/collab/controls.js @@ -8,7 +8,7 @@ const applyChangesToYDoc = createRegistryControl( ( registry ) => ( action ) => const doc = registry.select( 'isolated/editor' ).getYDoc(); if ( doc && ! action.isTriggeredByYDoc ) { - doc.applyDataChanges( { blocks: action.blocks } ); + doc.applyChangesToYDoc( { blocks: action.blocks } ); } return action; diff --git a/src/store/peers/selectors.js b/src/store/peers/selectors.js index dd9274ae8..1de714372 100644 --- a/src/store/peers/selectors.js +++ b/src/store/peers/selectors.js @@ -1,7 +1,3 @@ export function getPeers( state ) { return state.peers; } - -export function hasPeers( state ) { - return Object.keys( state.peers ).length > 0; -}