-
Notifications
You must be signed in to change notification settings - Fork 110
fix: prevent loading the code block extension twice for plain text #7315
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7315 +/- ##
==========================================
+ Coverage 52.09% 58.98% +6.89%
==========================================
Files 483 482 -1
Lines 41999 37113 -4886
Branches 1047 1048 +1
==========================================
+ Hits 21878 21891 +13
+ Misses 20018 15120 -4898
+ Partials 103 102 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
76465d8 to
f931dcf
Compare
mejo-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Didn't test but looks sensible to me.
I have two smaller remarks, none of them blocking.
| extensions: [ | ||
| PlainText, | ||
| CodeBlockLowlight.configure({ | ||
| PlainText.configure({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| PlainText.configure({ | |
| PlainText.configure({ | |
| // Options to be passed through to `CodeBlockPlainText` |
How about a short code comment here to explain why we pass codeblock config options to the plaintext extension?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively the options could be set as a separate object on the code block lowlight key similar to how tiptap defines its options in the starter kit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment for now, we can move to separate options if we have some for other extensions being passed as well
max-nextcloud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this. Also saw it and had it on my todo list. Happy i can strike that off.
| extensions: [ | ||
| PlainText, | ||
| CodeBlockLowlight.configure({ | ||
| PlainText.configure({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively the options could be set as a separate object on the code block lowlight key similar to how tiptap defines its options in the starter kit.
Signed-off-by: Julius Knorr <[email protected]>
f931dcf to
0a0827e
Compare
This fixes a warning that occurs as for plain text editing we include the codeBlock extension twice. I noticed this when running tests on #7300
We can only add the extension in our PlainTextExtension as it wraps everything we need for that already. In this case the addition options are just needed to pass them trough to the code block extension.
Fixes tons of warnings like: