Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds KaTeX-based block math support: new math dependencies and CSS import, a BlockMath node/extension with $$ input rule, a BlockMathEditor popup UI and icon, FormatBar/slash-command wiring to insert/edit block math, and KaTeX rendering styles. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor
participant SlashCommand
participant BlockMathEditor
participant MathExt
participant Document
User->>Editor: Type "$$" or click Block Math button
alt via slash menu
User->>SlashCommand: select "Block Math"
SlashCommand->>Editor: dispatch "slash-command-block-math"
end
Editor->>BlockMathEditor: open (initialLaTeX)
BlockMathEditor->>User: autofocus textarea
User->>BlockMathEditor: edit and submit (Cmd/Ctrl+Enter)
BlockMathEditor->>Editor: onSubmit(latex)
Editor->>MathExt: normalizeBlockMath / insert or update block-math node
MathExt->>Document: create/update math node (KaTeX content)
Editor->>BlockMathEditor: close and refocus editor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
Author
|
@erictli Editor.tsx is getting quite large (nearly 2,200 lines), which makes it harder to maintain. Could we split some logic into smaller components/hooks? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/components/editor/BlockMathEditor.tsx (1)
42-51: Consider usingcn()utility for className merging.As per coding guidelines, React components in
src/components/**/*.tsxshould use thecn()utility function for className merging. While the current static classNames work, usingcn()provides consistency and enables future extensibility.♻️ Optional refactor
+import { cn } from "../ui"; import { CheckIcon, XIcon } from "../icons"; import { IconButton } from "../ui"; // In the component: - <div className="flex flex-col gap-1.5 bg-bg border border-border rounded-lg shadow-md p-1.5 w-84"> + <div className={cn("flex flex-col gap-1.5 bg-bg border border-border rounded-lg shadow-md p-1.5 w-84")}> <textarea ref={textareaRef} value={latex} onChange={(e) => setLatex(e.target.value)} onKeyDown={handleKeyDown} placeholder="Enter KaTeX expression..." - className="w-full h-30 resize-y rounded-md border border-border bg-bg px-2.5 py-2 text-sm text-text focus:outline-none focus:ring-2 focus:ring-accent/40" + className={cn("w-full h-30 resize-y rounded-md border border-border bg-bg px-2.5 py-2 text-sm text-text focus:outline-none focus:ring-2 focus:ring-accent/40")} />As per coding guidelines: "Use cn() utility function for className merging in all React components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/editor/BlockMathEditor.tsx` around lines 42 - 51, The textarea's className in the BlockMathEditor component should use the cn() utility for class merging to follow project conventions; update the JSX in BlockMathEditor (the textarea element that uses textareaRef, value={latex}, onChange and onKeyDown={handleKeyDown}) to call cn(...) with the same class string (and any conditional classes if needed) instead of a raw string so future merges/conditionals follow the shared pattern.src/App.css (1)
393-401: Consider using a CSS variable for error color.The error color
#dc2626is hardcoded for both.inline-math-errorand.block-math-error. For consistency with the theme system, consider defining a--color-errorCSS variable or reusing an existing semantic color.♻️ Optional refactor
.prose .inline-math-error { - color: `#dc2626`; + color: var(--color-error, `#dc2626`); font-family: var(--font-mono); } .prose .block-math-error { - color: `#dc2626`; + color: var(--color-error, `#dc2626`); font-family: var(--font-mono); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.css` around lines 393 - 401, Replace the hardcoded color with a CSS variable and reuse it in both selectors: define a semantic variable like --color-error (or reuse an existing error color variable) in a root or theme rule, then change .prose .inline-math-error and .prose .block-math-error to use color: var(--color-error); (and keep font-family as-is) so both classes share the theme-aware error color.README.md (1)
18-18: Documentation is incomplete for block math support.The feature line only mentions inline math (
$...$) but the PR also implements block math ($$...$$). Consider updating to reflect the full feature set.📝 Suggested documentation update
-- **KaTeX math** - Render inline `$...$` equations +- **KaTeX math** - Render inline `$...$` and block `$$...$$` equations🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 18, Update the README entry that currently reads "- **KaTeX math** - Render inline `$...$` equations" to accurately document both inline and block math support by mentioning `$...$` for inline and `$$...$$` for block math (or "inline and block KaTeX math using $...$ and $$...$$ delimiters") and optionally note block rendering behavior; edit the line describing "KaTeX math" so it references both delimiters and clarifies that block equations are supported.src/components/editor/Editor.tsx (2)
967-988: Optional: Extract shared KaTeX macros to reduce duplication.The
macrosconfiguration is duplicated betweenScratchInlineMathandScratchBlockMath. Consider extracting to a shared constant:♻️ Suggested refactor
+const KATEX_MACROS = { + "\\R": "\\mathbb{R}", + "\\N": "\\mathbb{N}", +}; + ScratchInlineMath.configure({ katexOptions: { throwOnError: false, - macros: { - "\\R": "\\mathbb{R}", - "\\N": "\\mathbb{N}", - }, + macros: KATEX_MACROS, }, }), ScratchBlockMath.configure({ katexOptions: { throwOnError: false, displayMode: true, - macros: { - "\\R": "\\mathbb{R}", - "\\N": "\\mathbb{N}", - }, + macros: KATEX_MACROS, }, // ... }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/editor/Editor.tsx` around lines 967 - 988, Extract the shared KaTeX macros into a single constant (e.g., KATEX_MACROS) and use that constant in both ScratchInlineMath.configure and ScratchBlockMath.configure to remove duplication; update the ScratchInlineMath and ScratchBlockMath configuration objects to reference the new constant for their macros, leaving other options (throwOnError, displayMode, onClick/handleEditBlockMath) unchanged so behavior is preserved.
674-924: Consider extracting block math popup logic into a custom hook.The block math handling logic (
closeBlockMathPopup,handleEditBlockMath,handleAddBlockMath, and the associated ref) is self-contained and could be extracted into a custom hook likeuseBlockMathPopup(). This would:
- Reduce the Editor component's complexity
- Mirror the pattern that could also apply to
linkPopupRefhandling- Address the maintainability concern raised in the PR comments about the file's size (~2,200 lines)
This is a non-blocking suggestion that can be addressed in a follow-up PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/editor/Editor.tsx` around lines 674 - 924, The block-math popup logic (closeBlockMathPopup, handleEditBlockMath, handleAddBlockMath and blockMathPopupRef plus related virtualElement/component/tippy setup and usage of BlockMathEditor and normalizeBlockMath) is self-contained and should be extracted into a custom hook (e.g. useBlockMathPopup) that encapsulates the ref, open/close handlers, and tippy lifecycle; implement the hook to accept editorRef and any callbacks (or return onSubmit/onCancel handlers), move all virtualElement creation, ReactRenderer instantiation, and tippy options into the hook, expose functions like openEditAt(pos) / openAddAt(range) / closePopup and useBlockMathPopup in the Editor component, and follow the same pattern for linkPopupRef to mirror behavior and reduce Editor component size and complexity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 24: The package.json currently includes "@tiptap/extension-mathematics":
"^3.20.0" which pulls in `@tiptap/core`@3.20.0 and thus requires
`@tiptap/pm`@^3.20.0; update the pinned `@tiptap/pm` entry in package.json to at
least "3.20.0" (or "^3.20.0") to satisfy the peer dependency, then run your
package manager (npm/yarn/pnpm) to update the lockfile and verify no peer
dependency warnings remain; ensure the change references the existing dependency
key "@tiptap/pm" and keep the version in sync with
`@tiptap/core/`@tiptap/extension-mathematics.
In `@src/components/editor/MathExtensions.ts`:
- Around line 165-179: The current toggleBlockMath command uses
normalizeBlockMath(selectedText) and returns false when latex is falsy, which
blocks inserting a new empty block math when the selection is empty; update the
logic in toggleBlockMath so that if the selection is empty you allow insertion
with an empty latex string (i.e., only bail out when the selection was non-empty
and normalizeBlockMath returned a falsy/invalid value), then proceed to call
commands.insertContentAt({ from, to }, { type: this.name, attrs: { latex } }) as
before; reference toggleBlockMath, normalizeBlockMath, commands.insertContentAt,
and this.name to locate and change the conditional.
---
Nitpick comments:
In `@README.md`:
- Line 18: Update the README entry that currently reads "- **KaTeX math** -
Render inline `$...$` equations" to accurately document both inline and block
math support by mentioning `$...$` for inline and `$$...$$` for block math (or
"inline and block KaTeX math using $...$ and $$...$$ delimiters") and optionally
note block rendering behavior; edit the line describing "KaTeX math" so it
references both delimiters and clarifies that block equations are supported.
In `@src/App.css`:
- Around line 393-401: Replace the hardcoded color with a CSS variable and reuse
it in both selectors: define a semantic variable like --color-error (or reuse an
existing error color variable) in a root or theme rule, then change .prose
.inline-math-error and .prose .block-math-error to use color:
var(--color-error); (and keep font-family as-is) so both classes share the
theme-aware error color.
In `@src/components/editor/BlockMathEditor.tsx`:
- Around line 42-51: The textarea's className in the BlockMathEditor component
should use the cn() utility for class merging to follow project conventions;
update the JSX in BlockMathEditor (the textarea element that uses textareaRef,
value={latex}, onChange and onKeyDown={handleKeyDown}) to call cn(...) with the
same class string (and any conditional classes if needed) instead of a raw
string so future merges/conditionals follow the shared pattern.
In `@src/components/editor/Editor.tsx`:
- Around line 967-988: Extract the shared KaTeX macros into a single constant
(e.g., KATEX_MACROS) and use that constant in both ScratchInlineMath.configure
and ScratchBlockMath.configure to remove duplication; update the
ScratchInlineMath and ScratchBlockMath configuration objects to reference the
new constant for their macros, leaving other options (throwOnError, displayMode,
onClick/handleEditBlockMath) unchanged so behavior is preserved.
- Around line 674-924: The block-math popup logic (closeBlockMathPopup,
handleEditBlockMath, handleAddBlockMath and blockMathPopupRef plus related
virtualElement/component/tippy setup and usage of BlockMathEditor and
normalizeBlockMath) is self-contained and should be extracted into a custom hook
(e.g. useBlockMathPopup) that encapsulates the ref, open/close handlers, and
tippy lifecycle; implement the hook to accept editorRef and any callbacks (or
return onSubmit/onCancel handlers), move all virtualElement creation,
ReactRenderer instantiation, and tippy options into the hook, expose functions
like openEditAt(pos) / openAddAt(range) / closePopup and useBlockMathPopup in
the Editor component, and follow the same pattern for linkPopupRef to mirror
behavior and reduce Editor component size and complexity.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
README.mdpackage.jsonsrc/App.csssrc/components/editor/BlockMathEditor.tsxsrc/components/editor/Editor.tsxsrc/components/editor/MathExtensions.tssrc/components/editor/SlashCommand.tsxsrc/components/icons/index.tsxsrc/main.tsx
Owner
|
@eeee0717 - good call on the editor, I'll see if I can refactor it in the next release. |
Drop inline math ($...$) support entirely — the single-$ delimiter is inherently ambiguous with currency (e.g. "$18/mo ... Team$24/user/mo") and there is no reliable heuristic that survives the markdown roundtrip. Any guard on the tokenizer that skips currency-like patterns also causes intentionally-created inline math to silently disappear on save/reload, since the .md file is the only source of truth. Block math ($$...$$) is unambiguous and covers the primary use case of rendering equations and formulas. Also in this commit: - Regenerate package-lock.json (removes spurious "peer": true entries) - Expand KaTeX macros to standard number-field set (\R \N \Z \Q \C) - Update README to reflect block-math-only support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
|
Thanks for this! Pushed a commit on top of your branch with a few changes: Removed inline math ( The single- Block math ( Other changes in the commit:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/App.css (1)
385-388: Use a theme token for block-math error color.The hard-coded red bypasses your tokenized theme system and may look inconsistent across themes. Prefer a semantic CSS variable.
🎨 Proposed patch
+/* add near other color tokens */ +:root { + /* ...existing vars... */ + --color-error: `#dc2626`; +} + +.dark { + /* ...existing vars... */ + --color-error: `#f87171`; +} + .prose .block-math-error { - color: `#dc2626`; + color: var(--color-error); font-family: var(--font-mono); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.css` around lines 385 - 388, Replace the hard-coded hex color in the .prose .block-math-error rule with a semantic theme CSS variable (e.g., change color: `#dc2626`; to color: var(--color-error) or the project's equivalent token like var(--color-danger) / var(--text-error)); keep font-family as var(--font-mono) and ensure the chosen variable name matches the existing theme tokens used elsewhere so the error color adapts across themes.src/components/icons/index.tsx (1)
421-425: Usecn()for className composition in the new icon.The new
BlockMathIcondirectly assignsclassName, which bypasses the repository’s className merge pattern.♻️ Suggested update
+import { cn } from "../../lib/utils"; + -export function BlockMathIcon({ className = "w-4.5 h-4.5" }: IconProps) { +export function BlockMathIcon({ className }: IconProps) { return ( <svg - className={className} + className={cn("w-4.5 h-4.5", className)} fill="none" stroke="currentColor" viewBox="0 0 24 24"As per coding guidelines:
src/components/**/*.tsx: Use cn() utility function for className merging in all React components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/icons/index.tsx` around lines 421 - 425, The BlockMathIcon component assigns className directly and bypasses the project's class merging utility; update BlockMathIcon to import and use the cn() helper to merge the default class string ("w-4.5 h-4.5") with the incoming className prop (e.g., className={cn("w-4.5 h-4.5", className)}) so it follows the repository pattern for className composition and preserves any passed-in classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.css`:
- Around line 370-375: The block display math selector .prose
.tiptap-mathematics-render[data-type="block-math"] lacks horizontal overflow
handling, so very long equations can spill out; update that selector to contain
overflow by adding overflow-x: auto (and optionally max-width: 100% and
word-wrap/white-space rules as needed) so long display equations scroll
horizontally inside the block instead of breaking layout.
In `@src/components/editor/Editor.tsx`:
- Around line 763-767: The block-math popup is closed when opening the link
popup, but the link flow doesn't close block-math, allowing overlapping
popovers; update the link-opening and link-destroy flows to also call
closeBlockMathPopup() so popups are mutually exclusive. Specifically, add a call
to closeBlockMathPopup() in the code paths that create or show the link popup
(where linkPopupRef.current is set/opened) and also before/after
linkPopupRef.current.destroy() to ensure any active block-math popup is closed;
reference the existing closeBlockMathPopup function and
linkPopupRef.current.destroy()/linkPopupRef to locate the spots to change.
- Around line 725-731: The edit popup's onSubmit currently allows saving an
empty formula; update onSubmit in the Editor component to mirror the add-path
validation by rejecting when latex.trim() === "" (e.g., do an early return or
show validation) before calling currentEditor.chain().focus().updateBlockMath({
pos, latex: latex.trim() }).run() and closeBlockMathPopup(); ensure you
reference the onSubmit handler and updateBlockMath call so empty trimmed LaTeX
is not saved.
---
Nitpick comments:
In `@src/App.css`:
- Around line 385-388: Replace the hard-coded hex color in the .prose
.block-math-error rule with a semantic theme CSS variable (e.g., change color:
`#dc2626`; to color: var(--color-error) or the project's equivalent token like
var(--color-danger) / var(--text-error)); keep font-family as var(--font-mono)
and ensure the chosen variable name matches the existing theme tokens used
elsewhere so the error color adapts across themes.
In `@src/components/icons/index.tsx`:
- Around line 421-425: The BlockMathIcon component assigns className directly
and bypasses the project's class merging utility; update BlockMathIcon to import
and use the cn() helper to merge the default class string ("w-4.5 h-4.5") with
the incoming className prop (e.g., className={cn("w-4.5 h-4.5", className)}) so
it follows the repository pattern for className composition and preserves any
passed-in classes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
README.mdsrc/App.csssrc/components/editor/Editor.tsxsrc/components/editor/MathExtensions.tssrc/components/icons/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/editor/MathExtensions.ts
- README.md
Comment on lines
+370
to
+375
| .prose .tiptap-mathematics-render[data-type="block-math"] { | ||
| display: block; | ||
| margin: 0.75rem 0; | ||
| padding: 0.5rem 0.75rem; | ||
| border-radius: 0.5rem; | ||
| } |
There was a problem hiding this comment.
Add horizontal overflow handling for long display equations.
Long display-math content can exceed container width and cause horizontal layout spill. Add overflow containment to the block-math wrapper.
💡 Proposed patch
.prose .tiptap-mathematics-render[data-type="block-math"] {
display: block;
margin: 0.75rem 0;
padding: 0.5rem 0.75rem;
border-radius: 0.5rem;
+ overflow-x: auto;
+ overflow-y: hidden;
}📝 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
| .prose .tiptap-mathematics-render[data-type="block-math"] { | |
| display: block; | |
| margin: 0.75rem 0; | |
| padding: 0.5rem 0.75rem; | |
| border-radius: 0.5rem; | |
| } | |
| .prose .tiptap-mathematics-render[data-type="block-math"] { | |
| display: block; | |
| margin: 0.75rem 0; | |
| padding: 0.5rem 0.75rem; | |
| border-radius: 0.5rem; | |
| overflow-x: auto; | |
| overflow-y: hidden; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.css` around lines 370 - 375, The block display math selector .prose
.tiptap-mathematics-render[data-type="block-math"] lacks horizontal overflow
handling, so very long equations can spill out; update that selector to contain
overflow by adding overflow-x: auto (and optionally max-width: 100% and
word-wrap/white-space rules as needed) so long display equations scroll
horizontally inside the block instead of breaking layout.
- Add overflow-x: auto to block math container so long equations scroll instead of breaking layout - Validate empty formula on edit path (was only checked on add path) - Close block math popup when opening link popup for mutual exclusivity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
erictli
approved these changes
Mar 4, 2026
Shayaan-Azeem
pushed a commit
to Shayaan-Azeem/scratch
that referenced
this pull request
Mar 10, 2026
* feat: support katex * chore * fix: build error * fix: simplify toggle inline math * feat: support block math * fix: review * Remove inline math, keep only block math; clean up lockfile and macros Drop inline math ($...$) support entirely — the single-$ delimiter is inherently ambiguous with currency (e.g. "$18/mo ... Team$24/user/mo") and there is no reliable heuristic that survives the markdown roundtrip. Any guard on the tokenizer that skips currency-like patterns also causes intentionally-created inline math to silently disappear on save/reload, since the .md file is the only source of truth. Block math ($$...$$) is unambiguous and covers the primary use case of rendering equations and formulas. Also in this commit: - Regenerate package-lock.json (removes spurious "peer": true entries) - Expand KaTeX macros to standard number-field set (\R \N \Z \Q \C) - Update README to reflect block-math-only support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix block math overflow, empty formula validation, and popup exclusivity - Add overflow-x: auto to block math container so long equations scroll instead of breaking layout - Validate empty formula on edit path (was only checked on add path) - Close block math popup when opening link popup for mutual exclusivity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: erictli <hi@ericli.io> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.
#64
Summary by CodeRabbit
New Features
Style
Documentation