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

Allow default behaviour in stopEvent/ignoreMutation overrides #5590

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

alexkuz
Copy link

@alexkuz alexkuz commented Sep 5, 2024

Changes Overview

In my case, I needed to override stopEvent, but only for a specific case. Otherwise I'd like for it to have a default behaviour. To do this, I'd have to recreate all the checks from default stopEvent in my override. This PR addresses this inconvenience.

Implementation Approach

stopEvent ignores override only if you explicitly return a null. This should minimize a breaking change for existing usages.

Testing Done

Verification Steps

Additional Notes

Checklist

  • I have created a changeset for this PR if necessary.
  • My changes do not break the library.
  • I have added tests where applicable.
  • I have followed the project guidelines.
  • I have fixed any lint issues.

Related Issues

#257

Copy link

changeset-bot bot commented Sep 5, 2024

🦋 Changeset detected

Latest commit: 1587216

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

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

Copy link

netlify bot commented Sep 5, 2024

Deploy Preview for tiptap-embed failed. Why did it fail? →

Name Link
🔨 Latest commit 1587216
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/66d963138020b200085ce28b

@@ -119,7 +119,10 @@ export class NodeView<
}

if (typeof this.options.stopEvent === 'function') {
return this.options.stopEvent({ event })
const shouldStopEvent = return this.options.stopEvent({ event })
Copy link
Contributor

Choose a reason for hiding this comment

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

This syntax looks wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Triage open
Development

Successfully merging this pull request may close these issues.

2 participants