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

setBlock merge data option #3125

Closed
timothyarmes opened this issue Nov 21, 2019 · 4 comments
Closed

setBlock merge data option #3125

timothyarmes opened this issue Nov 21, 2019 · 4 comments

Comments

@timothyarmes
Copy link

feature

Editor.setBlocks({ data: { ... } -}) replaces the existing data with the new map.

This makes is awkward for plugins to add/remove their own data without affecting other plugins. For example, I have in image plugin that sets the src, and an alignment plugin that sets the block's alignment.

When the alignment plugin tries to add its data using:

Editor.setBlocks({ data: { alignment: 'left' } })

the image src is lost.

@ihd-dev
Copy link

ihd-dev commented Nov 21, 2019

You could use existing data when setting the new, so as: Editor.setBlocks({ data: { ...existingData, alignment: 'left' } })

@timothyarmes
Copy link
Author

timothyarmes commented Nov 21, 2019

No I can't, that assumes that all the selected blocks have the same data.

Currently I'm doing this:

    editor.withoutNormalizing(() => {
      editor.value.blocks.forEach((block) => {
        editor.setNodeByKey(block.key, { data: block.data.merge(alignmentData).toJS() });
      });
    });

However, it's worth noting that setBlocks ends update doing something more advanced, and doesn't necessarily alter all the blocks:

  // Check if we have a "hanging" selection case where the even though the
  // selection extends into the start of the end node, we actually want to
  // ignore that for UX reasons.
  const isHanging =
    isCollapsed === false &&
    start.offset === 0 &&
    end.offset === 0 &&
    isStartVoid === false &&
    start.key === startBlock.getFirstText().key &&
    end.key === endBlock.getFirstText().key

  // If it's a hanging selection, ignore the last block.
  const sets = isHanging ? blocks.slice(0, -1) : blocks

  editor.withoutNormalizing(() => {
    sets.forEach(block => {
      editor.setNodeByKey(block.key, properties)
    })
  })

It would be nice to have a mergeBlocks function provided.

@ghost
Copy link

ghost commented Nov 26, 2019

Hi @timothyarmes,
Slate is in flight on deprecating it's immutable js dependency in favor of immer, so long term this won't be an issue.

However, the first snippet you provided is the way consumers have been advised to handle data; you need to use the editor.setNodeByKey(block.key, { data: block.data.merge(alignmentData) }) form to preserve the existing data.

@ianstormtaylor
Copy link
Owner

I believe that this may be fixed by #3093, which has changed a lot of the logic in Slate and slate-react especially. I'm going to close this out, but as always, feel free to open a new issue if it persists for you. Thanks for understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants