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

More control on editor.normalizeNode #5295

Merged
merged 7 commits into from
Feb 22, 2023

Conversation

zbeyens
Copy link
Contributor

@zbeyens zbeyens commented Feb 15, 2023

Description

not sure if the coding style fits as it looks to be the first time we have many parameters in an editor method, any suggestion is welcome

Non-breaking changes:

  • new editor method that can be overridden to control when the normalization should stop. Default behavior (unchanged) is to throw an error when it iterates over 42 times the dirty paths length.
shouldNormalize: ({
  iteration,
  dirtyPaths,
  operation,
}: {
  iteration: number
  dirtyPaths: Path[]
  operation?: Operation
}) => boolean
  • editor.onChange signature change: (options?: { operation?: Operation }) => void where operation is triggering the function.
  • editor.normalizeNode signature change: (entry: NodeEntry, options?: { operation?: Operation }) => void where operation is triggering the function.
  • EditorNormalizeOptions new option operation?: Operation where operation is triggering the function.

Issue
Fixes: #5222

Checks

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)
  • You've added a changeset if changing functionality. (Add one with yarn changeset add.)

@changeset-bot
Copy link

changeset-bot bot commented Feb 15, 2023

🦋 Changeset detected

Latest commit: 2dc096a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@zbeyens zbeyens marked this pull request as draft February 15, 2023 19:06
@zbeyens zbeyens changed the title feat More control on editor.normalizeNode Feb 15, 2023
@zbeyens zbeyens marked this pull request as ready for review February 16, 2023 00:48
Copy link
Collaborator

@dylans dylans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks useful to me. Leaving this open for a few days to give others time to review and/or give feedback.

@dylans dylans merged commit 84f811a into ianstormtaylor:main Feb 22, 2023
@delijah
Copy link
Contributor

delijah commented Jun 21, 2023

@dylans @zbeyens @ianstormtaylor Was this PR doing any good? I've just tried to use the operation from within normalizeNode and it looks like this is always undefined since most transform operations are running within withoutNormalizing...

It also looks a bit wrong to me, passing the operation to onChange, since the call to onChange is kind of asynchronous, which means more than 1 operation could have been executed when calling onChange. You anyways can access the operations through editor.operations isn't it?

Documentation seems a bit minimal..

Wouldn't the correct approach be, to use the following code inside normalizeNode, in order to find out what caused the current call to normalizeNode?

const operation = editor.operations.slice(-1)[0];

@zbeyens
Copy link
Contributor Author

zbeyens commented Jun 22, 2023

You're right. This option was experimental and can be removed since no use-case has been built upon it.

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

Successfully merging this pull request may close these issues.

Increase number of max normalization iterations
3 participants