feat: math block#2857
Open
matthewlipski wants to merge 7 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR moves Shiki syntax highlighting out of ChangesCode Block Refactor and SyntaxHighlighting Extension
New
Sequence Diagram(s)sequenceDiagram
participant User
participant BlockNoteEditor
participant SyntaxHighlightingExtension
participant lazyShikiPlugin
participant createPreviewWithSourcePopup
User->>BlockNoteEditor: click math/code preview
BlockNoteEditor->>createPreviewWithSourcePopup: mousedown on preview dom
createPreviewWithSourcePopup->>BlockNoteEditor: set cursor to block start, show popup
BlockNoteEditor->>SyntaxHighlightingExtension: selection change triggers highlight
SyntaxHighlightingExtension->>lazyShikiPlugin: extract language from block attrs via highlightBlock
lazyShikiPlugin-->>BlockNoteEditor: apply decorations
User->>BlockNoteEditor: Escape key
BlockNoteEditor->>createPreviewWithSourcePopup: NodeSelection on block-content node, hide popup
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
packages/math-block/src/helpers/toExternalHTML/createMathML.ts (1)
8-17: Consider explicitly pinning the Temml trust mode.At lines 8–13, while Temml's default
trustvalue isfalse(which is secure), explicitly settingtrust: falsemakes the security intent clear and protects against potential upstream default changes.Suggested change
const mathml = temml.renderToString(getMathSource(block), { displayMode: true, annotate: true, + trust: false, // Export gracefully renders invalid LaTeX rather than throwing. throwOnError: false, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/math-block/src/helpers/toExternalHTML/createMathML.ts` around lines 8 - 17, The temml.renderToString() call on line 8 does not explicitly set the trust property in its options object. Add trust: false to the options passed to temml.renderToString() alongside the existing displayMode, annotate, and throwOnError properties to explicitly document the security intent and protect against potential upstream default changes to Temml's trust setting.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/06-custom-schema/09-math-block/index.html`:
- Line 1: Add the HTML5 DOCTYPE declaration as the very first line of the file
before the <html> tag. Insert `<!doctype html>` on line 1, which will move the
existing `<html lang="en">` tag to line 2. This ensures the browser renders in
standards mode instead of triggering quirks mode, and makes the HTML document
compliant with proper HTML5 structure requirements.
In `@examples/06-custom-schema/09-math-block/main.tsx`:
- Line 4: The import statement for App specifies the wrong file extension (.jsx
instead of .tsx). Change the import path from ./src/App.jsx to ./src/App.tsx to
match the actual file name and allow TypeScript module resolution to succeed
with the allowJs: false configuration.
In `@examples/06-custom-schema/09-math-block/vite.config.ts`:
- Around line 16-27: The vite alias configuration in the conditional block is
using `../../` in the path.resolve calls for both `@blocknote/core` and
`@blocknote/react`, but it should use `../../../` to resolve the packages from the
correct directory level. Update each path string passed to path.resolve (for
"`@blocknote/core`" and "`@blocknote/react`") to include one additional `../` in the
relative path so the source-alias branch activates correctly when the packages
source directory exists.
In `@packages/core/src/blocks/Code/CodeBlockOptions.ts`:
- Around line 72-80: The getLanguageId function performs case-sensitive
comparisons when matching the languageName parameter against language aliases
and IDs, causing inputs like `TS` or `Js` to fail to map to their canonical
lowercase configured IDs. Fix this by normalizing all comparisons to be
case-insensitive: convert the incoming languageName parameter to lowercase, and
convert both the aliases from the supportedLanguages configuration and the id
values to lowercase before performing the comparison checks in the find
callback.
In `@packages/core/src/blocks/Code/helpers/parse/parsePreCode.ts`:
- Around line 19-22: The language-class detection in the code is too broad
because it uses includes("language-") which matches the substring anywhere in
the class name, potentially capturing false positives like classes containing
"language-" in the middle of the string. Replace the includes("language-") check
with startsWith("language-") to ensure only class names that actually begin with
"language-" are considered valid language identifiers, restricting the match to
the beginning of the string only.
In `@packages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.ts`:
- Around line 10-12: The language lookup on line 11 does not normalize language
aliases before accessing the supportedLanguages map. If the language value is an
alias such as "ts", the lookup will fail and createPreview will be skipped,
causing fallback to source-only rendering. Resolve the language string through a
shared language-ID resolver to normalize it to its canonical form before using
it as the lookup key in supportedLanguages on line 11.
In `@packages/core/src/blocks/Code/helpers/render/createSourceBlock.ts`:
- Around line 8-25: The language value being assigned to select.value is not
validated against the supported languages, which can result in no option being
selected if the stored language is an alias or outdated identifier. Before
assigning select.value in the language dropdown initialization, add logic to
resolve the language variable to a canonical key from
options.supportedLanguages. Check if the language exists as a key in the
supportedLanguages object; if not, fall back to the default language or the
first available option. Only after normalizing the language should you assign it
to select.value.
In `@packages/core/src/editor/Block.css`:
- Around line 482-517: Remove the empty lines that appear before CSS property
declarations in the `.bn-code-block-source-popup` and its related child
selectors (including `.bn-code-block-source-popup > div > select`,
`.bn-code-block-source-popup > div > select > option`, and
`.bn-code-block-source-popup > pre`). These blank lines before declarations are
triggering Stylelint violations. Go through each CSS rule block and delete the
extra blank lines while maintaining proper formatting between distinct rule
blocks.
In `@packages/core/src/extensions/SyntaxHighlighting/shiki.ts`:
- Around line 29-32: The code caches the Shiki highlighter and parser on
globalThis, causing all editor instances to share the same cache regardless of
their individual syntaxHighlighting.createHighlighter configuration. This
violates the editor-level configuration contract. Remove the global caching
mechanism by eliminating the globalThis symbol-keyed properties and instead
store the highlighter and parser at the instance level (as properties on an
editor-specific object or context). Specifically, refactor the
globalThisForShiki type definition and related caching logic at the three
affected sites in packages/core/src/extensions/SyntaxHighlighting/shiki.ts
(lines 29-32, 44-46, and 74-76) to use instance-specific storage instead of
globalThis, ensuring each editor instance maintains its own separate highlighter
and parser cache.
In `@packages/math-block/src/helpers/getMathSource.ts`:
- Around line 8-11: In the getMathSource function's array mapping logic, add a
defensive check before using the `in` operator on the node variable. Since
block.content is of unknown type, the array items could be primitives, null, or
non-objects that would throw a TypeError when used with the `in` operator.
Modify the map callback to first verify that node is an object (using typeof
node === "object" && node !== null or a similar guard) before attempting to
access the "text" property, ensuring the function gracefully handles unexpected
data types without crashing.
In `@packages/math-block/vite.config.ts`:
- Around line 52-56: The vite.config.ts build configuration is currently
including devDependencies in the externals check along with dependencies and
peerDependencies. Remove the spread of pkg.devDependencies from the
Object.keys() call in the external configuration block so that only actual
runtime dependencies (dependencies and peerDependencies) are marked as external.
This ensures that accidental imports of dev-only packages will fail during the
build rather than being shipped unresolved to library consumers.
---
Nitpick comments:
In `@packages/math-block/src/helpers/toExternalHTML/createMathML.ts`:
- Around line 8-17: The temml.renderToString() call on line 8 does not
explicitly set the trust property in its options object. Add trust: false to the
options passed to temml.renderToString() alongside the existing displayMode,
annotate, and throwOnError properties to explicitly document the security intent
and protect against potential upstream default changes to Temml's trust setting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9b64910-b7e1-4084-b088-4b77233bd8c5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (54)
docs/content/docs/features/blocks/code-blocks.mdxdocs/package.jsonexamples/04-theming/06-code-block/src/App.tsxexamples/04-theming/07-custom-code-block/src/App.tsxexamples/06-custom-schema/09-math-block/.bnexample.jsonexamples/06-custom-schema/09-math-block/README.mdexamples/06-custom-schema/09-math-block/index.htmlexamples/06-custom-schema/09-math-block/main.tsxexamples/06-custom-schema/09-math-block/package.jsonexamples/06-custom-schema/09-math-block/src/App.tsxexamples/06-custom-schema/09-math-block/tsconfig.jsonexamples/06-custom-schema/09-math-block/vite.config.tspackages/code-block/package.jsonpackages/code-block/src/index.test.tspackages/code-block/src/index.tspackages/core/package.jsonpackages/core/src/blocks/Code/CodeBlockOptions.tspackages/core/src/blocks/Code/block.test.tspackages/core/src/blocks/Code/block.tspackages/core/src/blocks/Code/helpers/extensions/createCodeKeyboardShortcutsExtension.tspackages/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.tspackages/core/src/blocks/Code/helpers/extensions/createPreviewSourceSelectionExtension.tspackages/core/src/blocks/Code/helpers/parse/parsePreCode.tspackages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.tspackages/core/src/blocks/Code/helpers/render/createPreviewWithSourcePopup.tspackages/core/src/blocks/Code/helpers/render/createSourceBlock.tspackages/core/src/blocks/Code/helpers/toExternalHTML/createPreCode.tspackages/core/src/blocks/Code/shiki.tspackages/core/src/blocks/index.tspackages/core/src/editor/Block.csspackages/core/src/editor/BlockNoteEditor.tspackages/core/src/editor/managers/ExtensionManager/extensions.tspackages/core/src/extensions/SyntaxHighlighting/SyntaxHighlighting.test.tspackages/core/src/extensions/SyntaxHighlighting/SyntaxHighlighting.tspackages/core/src/extensions/SyntaxHighlighting/shiki.tspackages/core/src/extensions/index.tspackages/core/src/index.tspackages/core/src/schema/blocks/createSpec.tspackages/core/src/schema/blocks/types.tspackages/math-block/.gitignorepackages/math-block/LICENSEpackages/math-block/package.jsonpackages/math-block/src/block.test.tspackages/math-block/src/block.tspackages/math-block/src/helpers/getMathSource.tspackages/math-block/src/helpers/parse/parseMathML.tspackages/math-block/src/helpers/render/createMathPreview.tspackages/math-block/src/helpers/toExternalHTML/createMathML.tspackages/math-block/src/index.tspackages/math-block/src/vite-env.d.tspackages/math-block/tsconfig.jsonpackages/math-block/vite.config.tspackages/math-block/vitestSetup.tsplayground/src/examples.gen.tsx
💤 Files with no reviewable changes (1)
- packages/core/src/blocks/Code/shiki.ts
Comment on lines
+29
to
+32
| const globalThisForShiki = globalThis as { | ||
| [shikiHighlighterPromiseSymbol]?: Promise<HighlighterGeneric<any, any>>; | ||
| [shikiParserSymbol]?: Parser; | ||
| }; |
Contributor
There was a problem hiding this comment.
Avoid global Shiki cache collisions across editor instances.
Line 44 and Line 74 cache the highlighter/parser on globalThis, so the first editor instance can silently control highlighting behavior for later editors with different syntaxHighlighting.createHighlighter options. This violates the editor-level configuration contract and can produce incorrect themes/language availability in multi-editor pages.
Suggested fix
- const globalThisForShiki = globalThis as {
- [shikiHighlighterPromiseSymbol]?: Promise<HighlighterGeneric<any, any>>;
- [shikiParserSymbol]?: Parser;
- };
+ let highlighterPromise: Promise<HighlighterGeneric<any, any>> | undefined;
let highlighter: HighlighterGeneric<any, any> | undefined;
let parser: Parser | undefined;
@@
if (!highlighter) {
- globalThisForShiki[shikiHighlighterPromiseSymbol] =
- globalThisForShiki[shikiHighlighterPromiseSymbol] ||
- options.createHighlighter();
+ highlighterPromise = highlighterPromise || options.createHighlighter();
- return globalThisForShiki[shikiHighlighterPromiseSymbol].then(
+ return highlighterPromise.then(
(createdHighlighter) => {
highlighter = createdHighlighter;
},
);
}
@@
if (!parser) {
- parser =
- globalThisForShiki[shikiParserSymbol] ||
- createParser(highlighter as any);
- globalThisForShiki[shikiParserSymbol] = parser;
+ parser = createParser(highlighter as any);
}Also applies to: 44-46, 74-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/extensions/SyntaxHighlighting/shiki.ts` around lines 29 -
32, The code caches the Shiki highlighter and parser on globalThis, causing all
editor instances to share the same cache regardless of their individual
syntaxHighlighting.createHighlighter configuration. This violates the
editor-level configuration contract. Remove the global caching mechanism by
eliminating the globalThis symbol-keyed properties and instead store the
highlighter and parser at the instance level (as properties on an
editor-specific object or context). Specifically, refactor the
globalThisForShiki type definition and related caching logic at the three
affected sites in packages/core/src/extensions/SyntaxHighlighting/shiki.ts
(lines 29-32, 44-46, and 74-76) to use instance-specific storage instead of
globalThis, ensuring each editor instance maintains its own separate highlighter
and parser cache.
Comment on lines
+8
to
+11
| if (Array.isArray(block.content)) { | ||
| return block.content | ||
| .map((node) => ("text" in node ? node.text : "")) | ||
| .join(""); |
Contributor
There was a problem hiding this comment.
Guard non-object inline nodes before using the in operator.
Line 10 can throw at runtime when an array item is not an object ("text" in node on primitives/null). Since block.content is unknown, this path should be defensive instead of crashing preview/export.
Proposed fix
if (Array.isArray(block.content)) {
return block.content
- .map((node) => ("text" in node ? node.text : ""))
+ .map((node) =>
+ node && typeof node === "object" && "text" in node
+ ? String((node as { text?: unknown }).text ?? "")
+ : "",
+ )
.join("");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (Array.isArray(block.content)) { | |
| return block.content | |
| .map((node) => ("text" in node ? node.text : "")) | |
| .join(""); | |
| if (Array.isArray(block.content)) { | |
| return block.content | |
| .map((node) => | |
| node && typeof node === "object" && "text" in node | |
| ? String((node as { text?: unknown }).text ?? "") | |
| : "", | |
| ) | |
| .join(""); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/math-block/src/helpers/getMathSource.ts` around lines 8 - 11, In the
getMathSource function's array mapping logic, add a defensive check before using
the `in` operator on the node variable. Since block.content is of unknown type,
the array items could be primitives, null, or non-objects that would throw a
TypeError when used with the `in` operator. Modify the map callback to first
verify that node is an object (using typeof node === "object" && node !== null
or a similar guard) before attempting to access the "text" property, ensuring
the function gracefully handles unexpected data types without crashing.
Comment on lines
+52
to
+56
| Object.keys({ | ||
| ...pkg.dependencies, | ||
| ...((pkg as any).peerDependencies || {}), | ||
| ...pkg.devDependencies, | ||
| }).some((dep) => source === dep || source.startsWith(dep + "/")) |
Contributor
There was a problem hiding this comment.
Do not externalize devDependencies in the published library build.
Including pkg.devDependencies in external can ship unresolved runtime imports to consumers if a dev-only package is imported by mistake. Keep externals to dependencies/peerDependencies so declaration mistakes fail fast during packaging.
Proposed fix
Object.keys({
...pkg.dependencies,
...((pkg as any).peerDependencies || {}),
- ...pkg.devDependencies,
}).some((dep) => source === dep || source.startsWith(dep + "/"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Object.keys({ | |
| ...pkg.dependencies, | |
| ...((pkg as any).peerDependencies || {}), | |
| ...pkg.devDependencies, | |
| }).some((dep) => source === dep || source.startsWith(dep + "/")) | |
| Object.keys({ | |
| ...pkg.dependencies, | |
| ...((pkg as any).peerDependencies || {}), | |
| }).some((dep) => source === dep || source.startsWith(dep + "/")) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/math-block/vite.config.ts` around lines 52 - 56, The vite.config.ts
build configuration is currently including devDependencies in the externals
check along with dependencies and peerDependencies. Remove the spread of
pkg.devDependencies from the Object.keys() call in the external configuration
block so that only actual runtime dependencies (dependencies and
peerDependencies) are marked as external. This ensures that accidental imports
of dev-only packages will fail during the build rather than being shipped
unresolved to library consumers.
YousefED
reviewed
Jun 17, 2026
YousefED
left a comment
YousefED
left a comment
Collaborator
There was a problem hiding this comment.
Exciting!
Looking at the code from a high level (no user testing or deep dive into the functions), I have the following high-level questions:
- Should we use React or Vanilla for the components? (especially thinking about customizability)
- Curious if the current code design (with the keyboard handlers) also scales to interfaces where the editor is not in a pop-up (e.g.: Notion / TypeCell style editors that can collapse)
| **Syntax Highlighting** | ||
|
|
||
| BlockNote also provides a generic set of options for syntax highlighting in the `@blocknote/code-block` package, which support a wide range of languages: | ||
| Syntax highlighting is handled by a separate editor extension, configured at the editor level via the `syntaxHighlighting` option (not on the code block itself), so it can highlight any block's content: |
Collaborator
There was a problem hiding this comment.
Curious if we can find a better way to do this. This will make it difficult to create easily installable block-plugins later I think
| const { block } = editor.getTextCursorPosition(); | ||
| if (block.type === blockType) { | ||
| // TODO should probably only tab when at a line start or already tabbed in | ||
| tr.insertText(" "); |
Collaborator
There was a problem hiding this comment.
spaces or \t? or configurable?
| @@ -0,0 +1,170 @@ | |||
| import { | |||
Collaborator
There was a problem hiding this comment.
do we prefer react / vanilla for these components? (cc @nperez0111 )
Collaborator
There was a problem hiding this comment.
first thought; vanilla makes it pretty complex to customize them in a block-view style manner right?
| editor: BlockNoteEditor<any>, | ||
| ) => { | ||
| dom: HTMLElement; | ||
| ignoreMutation?: (mutation: ViewMutationRecord) => boolean; |
Collaborator
There was a problem hiding this comment.
if a user wants to implement their own preview, would they need to implement ignoreMutation? It's a very prosemirror-style (magic 🤯) API. Can we figure an API design that abstracts this out? (maybe same for dom and destroy actually)
YousefED
reviewed
Jun 17, 2026
Collaborator
There was a problem hiding this comment.
Here's some feedback after a super early round of user-testing:
- pressing up after “s” doesn’t go to the first popup:
-
esc should close popup
-
formattingtoolbar appears when highlighting text.
-
I’m allowed to “bold” things. I think we should see if we can make sure at a lower level code blocks cannot allow formatted text. This probably also caused the issue with the exporters recently (remember?)
-
preview opens when dragging block
-
keyboard is buggy when two math blocks sit above/below combined with the “gapcursor”
-
this is a bit misaligned:
This might also be nice for testing:
- slashmenu support in demos
- collaboration example
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/math-block/src/block.test.ts (1)
762-762: ⚡ Quick winPrefer semantic MathML assertions over exact full-string equality.
This assertion is fragile to harmless serialization differences. Assert the exported node structure and
application/x-texannotation content instead of matching one exact serialized string.Suggested test hardening
- expect(serializedMathML).toBe( - `"<math xmlns="http://www.w3.org/1998/Math/MathML" ... </math>"`, - ); + const parsed = new DOMParser().parseFromString(serializedMathML, "text/html"); + const math = parsed.querySelector("math"); + expect(math).not.toBeNull(); + const tex = math + ?.querySelector('annotation[encoding="application/x-tex"]') + ?.textContent + ?.trim(); + expect(tex).toBe("a^2 + b^2 = c^2");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/math-block/src/block.test.ts` at line 762, The test assertion at line 762 performs brittle exact full-string matching of the entire serialized MathML output, which is fragile to harmless serialization differences. Instead of comparing the complete serialized string, extract and assert specific structural elements: validate the exported node structure and specifically check the application/x-tex annotation content value rather than relying on full-string equality to make the test more resilient to formatting or serialization changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.ts`:
- Around line 175-218: The Enter and Escape handlers are missing
scrollIntoView() calls after setting the selection, creating an inconsistency
with the arrow handlers (lines 127, 147) which do include these calls. Add
tr.scrollIntoView() after each tr.setSelection() call in both the Enter handler
and the Escape handler to ensure the viewport scrolls to keep the focused block
visible when the selection changes.
---
Nitpick comments:
In `@packages/math-block/src/block.test.ts`:
- Line 762: The test assertion at line 762 performs brittle exact full-string
matching of the entire serialized MathML output, which is fragile to harmless
serialization differences. Instead of comparing the complete serialized string,
extract and assert specific structural elements: validate the exported node
structure and specifically check the application/x-tex annotation content value
rather than relying on full-string equality to make the test more resilient to
formatting or serialization changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49fca915-9fa2-4f91-9469-351dd45a6d53
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/src/unit/core/schema/__snapshots__/blocks.jsonis excluded by!**/__snapshots__/**
📒 Files selected for processing (14)
packages/core/src/blocks/Code/CodeBlockOptions.tspackages/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.tspackages/core/src/blocks/Code/helpers/parse/parsePreCode.tspackages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.tspackages/core/src/blocks/Code/helpers/render/createPreviewWithSourcePopup.tspackages/core/src/blocks/Code/helpers/render/createSourceBlock.tspackages/core/src/blocks/index.tspackages/core/src/editor/Block.csspackages/math-block/package.jsonpackages/math-block/src/block.test.tspackages/math-block/src/block.tspackages/math-block/src/helpers/parse/parseMathML.tspackages/math-block/src/helpers/render/createMathPreview.tspackages/math-block/src/helpers/toExternalHTML/createMathML.ts
💤 Files with no reviewable changes (2)
- packages/core/src/blocks/index.ts
- packages/math-block/src/helpers/parse/parseMathML.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/core/src/blocks/Code/helpers/parse/parsePreCode.ts
- packages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.ts
- packages/core/src/blocks/Code/helpers/render/createSourceBlock.ts
- packages/math-block/src/block.ts
- packages/core/src/blocks/Code/CodeBlockOptions.ts
- packages/core/src/editor/Block.css
- packages/core/src/blocks/Code/helpers/render/createPreviewWithSourcePopup.ts
Comment on lines
+175
to
+218
| Enter: ({ editor }) => | ||
| editor.transact((tr) => { | ||
| const blockInfo = getBlockInfoFromTransaction(tr); | ||
| if ( | ||
| !blockInfo.isBlockContainer || | ||
| blockInfo.blockNoteType !== blockType | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| if (tr.selection instanceof NodeSelection) { | ||
| tr.setSelection( | ||
| TextSelection.create( | ||
| tr.doc, | ||
| blockInfo.blockContent.beforePos + 1, | ||
| ), | ||
| ); | ||
| } else { | ||
| tr.setSelection( | ||
| NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos), | ||
| ); | ||
| } | ||
|
|
||
| return true; | ||
| }), | ||
| // Closes the source code popup by setting the selection on the whole block content node. | ||
| Escape: ({ editor }) => | ||
| editor.transact((tr) => { | ||
| const blockInfo = getBlockInfoFromTransaction(tr); | ||
|
|
||
| if ( | ||
| !blockInfo.isBlockContainer || | ||
| blockInfo.blockNoteType !== blockType || | ||
| tr.selection instanceof NodeSelection | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| tr.setSelection( | ||
| NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos), | ||
| ); | ||
|
|
||
| return true; | ||
| }), |
Contributor
There was a problem hiding this comment.
Missing scrollIntoView() after selection changes in Enter/Escape handlers.
The arrow handlers (lines 127, 147) call .scrollIntoView() after setting selection, but Enter and Escape do not. This inconsistency could cause the selection to move without scrolling the viewport, potentially leaving the focused block off-screen.
Proposed fix
if (tr.selection instanceof NodeSelection) {
tr.setSelection(
TextSelection.create(
tr.doc,
blockInfo.blockContent.beforePos + 1,
),
- );
+ ).scrollIntoView();
} else {
tr.setSelection(
NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos),
- );
+ ).scrollIntoView();
}
return true;
}),
// Closes the source code popup by setting the selection on the whole block content node.
Escape: ({ editor }) =>
editor.transact((tr) => {
const blockInfo = getBlockInfoFromTransaction(tr);
if (
!blockInfo.isBlockContainer ||
blockInfo.blockNoteType !== blockType ||
tr.selection instanceof NodeSelection
) {
return false;
}
tr.setSelection(
NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos),
- );
+ ).scrollIntoView();
return true;
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Enter: ({ editor }) => | |
| editor.transact((tr) => { | |
| const blockInfo = getBlockInfoFromTransaction(tr); | |
| if ( | |
| !blockInfo.isBlockContainer || | |
| blockInfo.blockNoteType !== blockType | |
| ) { | |
| return false; | |
| } | |
| if (tr.selection instanceof NodeSelection) { | |
| tr.setSelection( | |
| TextSelection.create( | |
| tr.doc, | |
| blockInfo.blockContent.beforePos + 1, | |
| ), | |
| ); | |
| } else { | |
| tr.setSelection( | |
| NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos), | |
| ); | |
| } | |
| return true; | |
| }), | |
| // Closes the source code popup by setting the selection on the whole block content node. | |
| Escape: ({ editor }) => | |
| editor.transact((tr) => { | |
| const blockInfo = getBlockInfoFromTransaction(tr); | |
| if ( | |
| !blockInfo.isBlockContainer || | |
| blockInfo.blockNoteType !== blockType || | |
| tr.selection instanceof NodeSelection | |
| ) { | |
| return false; | |
| } | |
| tr.setSelection( | |
| NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos), | |
| ); | |
| return true; | |
| }), | |
| Enter: ({ editor }) => | |
| editor.transact((tr) => { | |
| const blockInfo = getBlockInfoFromTransaction(tr); | |
| if ( | |
| !blockInfo.isBlockContainer || | |
| blockInfo.blockNoteType !== blockType | |
| ) { | |
| return false; | |
| } | |
| if (tr.selection instanceof NodeSelection) { | |
| tr.setSelection( | |
| TextSelection.create( | |
| tr.doc, | |
| blockInfo.blockContent.beforePos + 1, | |
| ), | |
| ).scrollIntoView(); | |
| } else { | |
| tr.setSelection( | |
| NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos), | |
| ).scrollIntoView(); | |
| } | |
| return true; | |
| }), | |
| // Closes the source code popup by setting the selection on the whole block content node. | |
| Escape: ({ editor }) => | |
| editor.transact((tr) => { | |
| const blockInfo = getBlockInfoFromTransaction(tr); | |
| if ( | |
| !blockInfo.isBlockContainer || | |
| blockInfo.blockNoteType !== blockType || | |
| tr.selection instanceof NodeSelection | |
| ) { | |
| return false; | |
| } | |
| tr.setSelection( | |
| NodeSelection.create(tr.doc, blockInfo.blockContent.beforePos), | |
| ).scrollIntoView(); | |
| return true; | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.ts`
around lines 175 - 218, The Enter and Escape handlers are missing
scrollIntoView() calls after setting the selection, creating an inconsistency
with the arrow handlers (lines 127, 147) which do include these calls. Add
tr.scrollIntoView() after each tr.setSelection() call in both the Enter handler
and the Escape handler to ensure the viewport scrolls to keep the focused block
visible when the selection changes.
Collaborator
Author
|
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/math-block
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a LaTeX math block. To facilitate that, changes have been made to the code block and how syntax highlighting is handled.
Code Block & Syntax Highlighting Changes
Code Block
The core code block remains unchanged functionally. It's just been made more composable as helpers for
render,parse, extensions, etc, have been extracted to separate files.Additionally, helper functions have been added to render previews for source code.
Syntax Highlighting
Syntax highlighting configuration has been moved out of the code block entirely and into a new
syntaxHighlightingeditor option. This has the following fields:createHighlighter: The same field as used to be in the code block options. Moved here as syntax highlighting can now be applied to multiple block types rather than just the code block.highlightBlock: Function which tells the editor for which blocks to apply syntax highlighting. Returning a language string will apply highlighting on that block for that language, while returning undefined will do nothing.This is done in order to only have a single highlighter plugin from
prosemirror-highlight, rather than having an instance per block. TheSyntaxHighlightingExtensionis now loaded by the editor whenever thesyntaxHighlightingeditor option is passed.Math Block
A math block has been added, which renders an equation from LaTeX, stored in its inline content. It uses the following dependencies:
temml: LateX to MathML for rendering & external HTML export.mathml-to-latex: MathML to LaTeX for external HTML parsing.The math block has been added as a separate package.
Rationale
We have wanted to implement a math block for a while. While existing PRs exist for that, this one has a broader scope which includes the above code block/syntax highlighting changes.
Changes
See above.
Impact
N/A
Testing
Added unit tests for
SyntaxHighlightingextension & math block.Screenshots/Video
TODO
Checklist
Additional Notes
Summary by CodeRabbit