draft-js-plugins icon indicating copy to clipboard operation
draft-js-plugins copied to clipboard

Static toolbar is not initialised/updated soon enough when rendered above the Editor

Open ab-pm opened this issue 2 years ago • 7 comments

The toolbar store https://github.com/draft-js-plugins/draft-js-plugins/blob/b89a8146e6d697ebd15bf10f8970086862a56b47/packages/static-toolbar/src/index.tsx#L32 is initialised from the initialize hook only when the <Editor> is rendered/mounted, and the editor.props.editorState that is returned by getEditorState of the PluginFunctions https://github.com/draft-js-plugins/draft-js-plugins/blob/0f8f7f5e273ba3c7c164515a65c87fb89549a922/packages/editor/src/Editor/index.tsx#L185 is similarly only updated when the <Editor> is rendered by React.

This causes various problems when the <StaticToolbar> is rendered above (before) the <Editor> in an application. When the buttons in the toolbar try to access the editor state, they either

  • get no state at all, because getEditorState is still undefined. See e.g. inline style buttons explicitly working around this issue in https://github.com/draft-js-plugins/draft-js-plugins/blob/0f8f7f5e273ba3c7c164515a65c87fb89549a922/packages/buttons/src/utils/createInlineStyleButton.tsx#L28-L31
  • get stale state from the previous render, which e.g. contains the wrong selection

This is very annoying. As a workaround, I needed to do

<Toolbar>{(props) => {
   props.getEditorState = () => this.state.myEditorState;
   return <>
      … // buttons with {...props}
   </>
}}</Toolbar>

I think the <Toolbar>s (not just the static one, also the inline and side toolbars, although those are less likely to be affected) should

  • subscribe to the getEditorState part of the store
  • not render children when getEditorState is still missing
  • use onChange(newState) { store.updateItem('getEditorState', () => newState); } in addition to the initialize hook

What do you think?

Reproducible Demo

See https://codesandbox.io/s/jolly-bird-hzc9m6?file=/src/Editor.js - try placing the cursor on the text and move around

ab-pm avatar Oct 10 '23 22:10 ab-pm

The problem is that the toolbar is placed and rendered before the editor that updates the status. If you move the toolbar after the editor, everything works as expected. The best solution I have seen so far is to place the toolbar after the editor and change the order via CSS. Currently I have no idea for a simple solution without rewriting the plugin. If you want to take care of the problem I would be happy about a merge request

fxOne avatar Oct 11 '23 05:10 fxOne

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Feb 02 '24 02:02 github-actions[bot]

It's still a bug, even if it hasn't been fixed in 60 days.

ab-pm avatar Feb 02 '24 02:02 ab-pm

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Apr 05 '24 02:04 github-actions[bot]

It's still a bug, even if it hasn't been fixed in 120 days.

ab-pm avatar Apr 05 '24 11:04 ab-pm