-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
editor.setEditable should be allowed to omit update. #3289
Comments
I can agree that something like this is really needed, struggled yesterday about the same situation. The |
Would it make sense for |
@Rhys-T Breaking an existing contract for the added functionality probably isn't worth it. Consuming code won't be affected by the suggested change and it's lower touch. If there's a case for what you are suggesting I think it would warrant it's own issue and discussion. |
This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days |
Resolved with the merge of #3301 |
What problem are you facing?
After #3140 some behaviour broke in my app because I wasn't expecting the command to trigger
onUpdate()
.What’s the solution you would like to see?
I'd like an optional parameter
emitUpdate
similar to the API provided byeditor.commands.setContent
:tiptap/packages/core/src/commands/setContent.ts
Line 21 in c729810
The parameter would gate the event emission here:
tiptap/packages/core/src/Editor.ts
Lines 170 to 173 in c729810
What alternatives did you consider?
Maybe there's a way to look at the
editor
state ortransaction
state in theonUpdate({editor, transaction}) => ...
callback to know when theisEditable
property has been flipped but I couldn't think of anything.Anything to add? (optional)
I'm happy to implement this PR if you all don't see any issues with this. Adding an optional parameter is a non-breaking change and the default value should be
true
.The text was updated successfully, but these errors were encountered: