feat(editor): migrate rich text editing to Tiptap#5148
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR migrates the management rich-text editor from Slate to Tiptap, adds Markdown GFM and syntax highlighting, standardizes empty-content handling, updates styling and localization, expands Playwright coverage, and documents editor and workflow contracts. ChangesTiptap editor migration
Test interaction stabilization
Workflow and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Author
participant ContentInput
participant Tiptap
participant StoredMarkdown
participant MarkdownPreview
Author->>ContentInput: edit rich content
ContentInput->>Tiptap: execute editor commands
Tiptap->>ContentInput: emit Markdown update
ContentInput->>StoredMarkdown: save Markdown
StoredMarkdown->>MarkdownPreview: render persisted content
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| apps/frontend-manage/src/components/common/ContentInput.tsx | Replaces the Slate editor with Tiptap and keeps editor input, prop sync, and saved output on the Markdown path. |
| packages/markdown/src/Markdown.tsx | Adds GFM and code highlighting support while keeping raw HTML disabled and sanitized rendering in place. |
| playwright/tests/ZA-editor-rich-features.spec.ts | Adds coverage for formatting, tables, code blocks, preview rendering, reopening saved content, and legacy empty content. |
Reviews (17): Last reviewed commit: "fix(editor): react to Tiptap selection s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
dosubot
Bot
added
size:XXL
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 1509424 | Triggered | Generic Password | a259f06 | .devcontainer/docker-compose.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![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 (4)
playwright/tests/K-elements-selection.spec.ts (1)
22-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate helper across two spec files — extract to shared test utils.
openAnswerCollectionOptionsis defined identically in bothK-elements-selection.spec.tsandL-elements-case-study.spec.ts. Extract it to a shared helper module (e.g.playwright/tests/utils/answerCollection.ts) and import it in both specs to avoid drift when the retry logic or selector needs updating later.♻️ Proposed extraction
+// playwright/tests/utils/answerCollection.ts +import { Page } from '`@playwright/test`' + +export async function openAnswerCollectionOptions(page: Page) { + await page.locator('.ProseMirror').first().waitFor({ state: 'visible' }) + for (let attempt = 0; attempt < 3; attempt++) { + await page.getByTestId('open-answer-collection-options').click() + if ( + await page + .getByTestId('search-answer-options') + .isVisible({ timeout: 1_000 }) + .catch(() => false) + ) { + return + } + } + throw new Error('Failed to open answer collection options') +}Then in both spec files:
-async function openAnswerCollectionOptions(page: Page) { - await page.locator('.ProseMirror').first().waitFor({ state: 'visible' }) - for (let attempt = 0; attempt < 3; attempt++) { - await page.getByTestId('open-answer-collection-options').click() - if ( - await page - .getByTestId('search-answer-options') - .isVisible({ timeout: 1_000 }) - .catch(() => false) - ) { - return - } - } - throw new Error('Failed to open answer collection options') -} +import { openAnswerCollectionOptions } from './utils/answerCollection'🤖 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 `@playwright/tests/K-elements-selection.spec.ts` around lines 22 - 37, The helper openAnswerCollectionOptions is duplicated in multiple Playwright specs, so extract the shared retry/click logic into a common test utility module and import it from both K-elements-selection.spec and L-elements-case-study. Keep the behavior identical by moving the page/locator logic and the failure throw into one reusable function so selector or retry changes only need to be updated in one place.apps/frontend-manage/src/components/common/ContentInput.tsx (2)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured path alias for this local import.
./MediaLibraryviolates the repository import convention for TS/TSX files; switch it to the equivalent@/~alias path. As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Use@and~path aliases for imports".🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx` at line 28, The local import in ContentInput.tsx should use the repository’s configured path alias instead of a relative path. Update the MediaLibrary import to the equivalent @ or ~ alias form so it matches the TS/TSX import convention used across the codebase.Source: Coding guidelines
159-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove placeholder styling out of the component.
The inline
<style>injects raw CSS perContentInputinstance; move this rule to the shared/global stylesheet or express it through the project’s Tailwind styling layer. As per coding guidelines, "**/*.{ts,tsx}: Use TailwindCSS utilities only; usetwMergefor conditional classes".🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx` around lines 159 - 167, The placeholder CSS is being injected inline inside ContentInput, which violates the shared styling approach and duplicates styles per instance. Move the .ProseMirror p.is-editor-empty:first-child::before rule out of the component into the global/shared stylesheet or Tailwind layer, and keep ContentInput limited to Tailwind utility classes with twMerge for any conditional styling.Source: Coding guidelines
playwright/tests/ZA-editor-rich-features.spec.ts (1)
79-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocators rely on fragile, incidental UI details instead of
data-testid.The table button (
svg[data-icon="table"]), row-toolbar (hasText: '+R'), and preview container (.w-256) are selected via icon attributes, visible shorthand text, and a Tailwind utility class, respectively. This is inconsistent with thedata-testidconvention used elsewhere in this same test (create-question,insert-question-title,insert-question-text). Any icon swap, label change, or Tailwind refactor unrelated to editor behavior would silently break this test.Also applies to: 133-141
🤖 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 `@playwright/tests/ZA-editor-rich-features.spec.ts` around lines 79 - 102, The rich-features Playwright test is using fragile UI selectors instead of the existing data-testid pattern. Update the table button, row toolbar action, and preview/container lookups in the editor test to use stable test ids or other durable hooks already used elsewhere in this spec, so the assertions in the table interaction flow continue to target the same elements even if icons, labels, or Tailwind classes change.
🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Around line 523-545: The ToolbarButton component currently uses a clickable
span, which is not keyboard-accessible and can trigger lint warnings; update
ToolbarButton to render a native button element instead, set type="button" to
avoid unintended form submission, and keep the same styling/active behavior.
Also refactor the component declaration to use a function-based component
definition to match the component guideline, and preserve the existing
forwardRef, onClick, active, className, and children handling when making the
change.
- Around line 74-124: ContentInput is treating Markdown as generic content,
which can lose formatting on load and sync. Update the useEditor setup and the
external sync effect in ContentInput so both the initial content and
editor.commands.setContent use markdown parsing explicitly via contentType:
'markdown'. Keep the fix localized around useEditor and the content-sync
useEffect so tables, lists, and other Markdown formatting are preserved.
---
Nitpick comments:
In `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Line 28: The local import in ContentInput.tsx should use the repository’s
configured path alias instead of a relative path. Update the MediaLibrary import
to the equivalent @ or ~ alias form so it matches the TS/TSX import convention
used across the codebase.
- Around line 159-167: The placeholder CSS is being injected inline inside
ContentInput, which violates the shared styling approach and duplicates styles
per instance. Move the .ProseMirror p.is-editor-empty:first-child::before rule
out of the component into the global/shared stylesheet or Tailwind layer, and
keep ContentInput limited to Tailwind utility classes with twMerge for any
conditional styling.
In `@playwright/tests/K-elements-selection.spec.ts`:
- Around line 22-37: The helper openAnswerCollectionOptions is duplicated in
multiple Playwright specs, so extract the shared retry/click logic into a common
test utility module and import it from both K-elements-selection.spec and
L-elements-case-study. Keep the behavior identical by moving the page/locator
logic and the failure throw into one reusable function so selector or retry
changes only need to be updated in one place.
In `@playwright/tests/ZA-editor-rich-features.spec.ts`:
- Around line 79-102: The rich-features Playwright test is using fragile UI
selectors instead of the existing data-testid pattern. Update the table button,
row toolbar action, and preview/container lookups in the editor test to use
stable test ids or other durable hooks already used elsewhere in this spec, so
the assertions in the table interaction flow continue to target the same
elements even if icons, labels, or Tailwind classes change.
🪄 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: 171c2f80-56dd-4c11-b3b3-6541c374b48d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.github/scripts/get-shard-files.jsapps/frontend-manage/package.jsonapps/frontend-manage/src/components/common/ContentInput.tsxapps/frontend-manage/src/globals.cssapps/frontend-pwa/src/globals.csspackages/i18n/messages/de.tspackages/i18n/messages/en.tspackages/markdown/src/Markdown.tsxpackages/shared-components/package.jsonpackages/shared-components/src/@types/slate.d.tspackages/shared-components/src/utils/slateMdConversion.tsplaywright/tests/K-elements-selection.spec.tsplaywright/tests/L-elements-case-study.spec.tsplaywright/tests/ZA-editor-rich-features.spec.ts
💤 Files with no reviewable changes (2)
- packages/shared-components/src/@types/slate.d.ts
- packages/shared-components/src/utils/slateMdConversion.ts
…ssibility findings
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend-manage/src/components/common/ContentInput.tsx (1)
78-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep
onChangefresh inuseEditor'sonUpdatecallback.useEditorcan keep the initial callback, so a newonChangeprop may be missed unless the editor is recreated. Use a ref or stable callback wrapper here.🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx` around lines 78 - 104, The onUpdate handler in ContentInput’s useEditor setup can capture a stale onChange prop because the editor instance may keep the initial callback. Update the ContentInput component to keep onChange fresh by using a ref or a stable callback wrapper, and have onUpdate call that current function instead of closing over the prop directly.
🧹 Nitpick comments (1)
apps/frontend-manage/src/components/common/ContentInput.tsx (1)
558-586: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
{...props}spread can override the computeddisabledattribute.
disabledis not destructured from the component props, so it remains in the restpropsobject. Since{...props}is spread afterdisabled={isDisabled}, a caller passingdisabled={false}would override the context-derivedisDisabled, making the button clickable even when the editor is disabled. No current caller passesdisabled, but this is a latent bug.🔧 Destructure `disabled` to prevent override
const ToolbarButton = React.forwardRef<HTMLButtonElement, ToolbarButtonProps>( function ToolbarButton( - { className, active, children, onClick, ...props }, + { className, active, children, onClick, disabled: propDisabled, ...props }, ref ) { - const { disabled } = React.useContext(ToolbarContext) - const isDisabled = disabled || props.disabled + const { disabled: contextDisabled } = React.useContext(ToolbarContext) + const isDisabled = contextDisabled || propDisabled return ( <button ref={ref} type="button" onClick={onClick} aria-pressed={active} disabled={isDisabled} className={twMerge( 'outline-hidden my-auto flex h-7 w-7 items-center justify-center rounded border-0 bg-transparent p-0', isDisabled ? 'cursor-not-allowed opacity-50' : 'hover:bg-uzh-grey-20 cursor-pointer', active && 'bg-uzh-grey-40', className )} {...props} > {children} </button> ) } )🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx` around lines 558 - 586, The ToolbarButton component in ContentInput should not let the spread props override the computed disabled state. In ToolbarButton, destructure disabled out of the incoming props and compute isDisabled from ToolbarContext plus that prop, then ensure the rendered button’s disabled attribute is controlled only by that computed value before spreading the remaining props. This keeps caller-supplied props from making the button clickable when the editor is disabled.
🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Line 30: The ContentInput editor is pulling in the full lowlight language
bundle via createLowlight(all), which unnecessarily increases bundle size.
Update the lowlight setup in ContentInput to use createLowlight(common) or
explicitly register only the languages the editor actually needs, keeping the
same editor integration while reducing the imported syntax-highlighting
footprint.
---
Outside diff comments:
In `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Around line 78-104: The onUpdate handler in ContentInput’s useEditor setup can
capture a stale onChange prop because the editor instance may keep the initial
callback. Update the ContentInput component to keep onChange fresh by using a
ref or a stable callback wrapper, and have onUpdate call that current function
instead of closing over the prop directly.
---
Nitpick comments:
In `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Around line 558-586: The ToolbarButton component in ContentInput should not
let the spread props override the computed disabled state. In ToolbarButton,
destructure disabled out of the incoming props and compute isDisabled from
ToolbarContext plus that prop, then ensure the rendered button’s disabled
attribute is controlled only by that computed value before spreading the
remaining props. This keeps caller-supplied props from making the button
clickable when the editor is disabled.
🪄 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: d1d7ffee-d09d-4382-9750-a2029b29a9de
📒 Files selected for processing (14)
.agents/skills/klicker-playwright-e2e/SKILL.mdAGENTS.mdapps/frontend-manage/src/components/common/ContentInput.tsxapps/frontend-manage/src/globals.cssapps/frontend-manage/src/pages/questions/[id].tsxapps/frontend-pwa/src/globals.cssdocs/log.mdpackages/i18n/messages/de.tspackages/i18n/messages/en.tspackages/markdown/src/Markdown.tsxplaywright/tests/K-elements-selection.spec.tsplaywright/tests/L-elements-case-study.spec.tsplaywright/tests/ZA-editor-rich-features.spec.tsplaywright/util/actions.ts
✅ Files skipped from review due to trivial changes (2)
- packages/i18n/messages/de.ts
- apps/frontend-manage/src/pages/questions/[id].tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/i18n/messages/en.ts
- apps/frontend-pwa/src/globals.css
- packages/markdown/src/Markdown.tsx
- playwright/tests/ZA-editor-rich-features.spec.ts
rschlaefli
changed the title
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
project/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the dependency non-goal.
This PR explicitly introduces Tiptap/Lowlight dependencies, so “No new dependencies” is ambiguous. Reword it to mean no additional dependencies beyond the migration already in this PR.
🤖 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 `@project/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md` at line 37, Reword the dependency non-goal in the migration plan so it excludes only dependencies beyond the Tiptap/Lowlight additions already introduced by this PR, while retaining the intent to avoid major upgrades.apps/frontend-manage/src/components/common/ContentInput.tsx (1)
175-175: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize the
ToolbarContextprovider value.
value={{ disabled }}creates a new object every render, so everyToolbarButtonconsumer re-renders on each keystroke even whendisabledhasn't changed. SonarCloud flags this too.♻️ Suggested fix
+ const toolbarContextValue = React.useMemo(() => ({ disabled }), [disabled]) + ... - <ToolbarContext.Provider value={{ disabled }}> + <ToolbarContext.Provider value={toolbarContextValue}>🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx` at line 175, Memoize the value passed to ToolbarContext.Provider in ContentInput using the existing React memoization pattern, with disabled as its dependency. Keep the provider behavior unchanged while ensuring ToolbarButton consumers do not re-render when disabled is unchanged.Source: Linters/SAST tools
apps/frontend-manage/src/globals.css (1)
314-415: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTable/code-highlighting CSS is duplicated verbatim across two apps.
This block (borders, padding,
pretheme,hljs-*/token.*color mappings) is nearly identical to the block added inapps/frontend-pwa/src/globals.css(Lines 313-395). Since the PR objective calls out "shared table and code styles," consider extracting this into a shared CSS partial (e.g. underpackages/shared-componentsorpackages/markdown) imported by both apps, so future theme/color tweaks don't need to be kept in sync manually across two files.🤖 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 `@apps/frontend-manage/src/globals.css` around lines 314 - 415, Extract the duplicated table and code-highlighting styles, including the table, pre, hljs, and token selectors, into a shared CSS partial in the appropriate shared package. Import that partial from both apps’ globals.css files and remove the duplicated definitions, preserving the existing selectors and styling.
🤖 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 `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Around line 88-96: Update the editor configuration in ContentInput so the
Placeholder extension reads the current placeholder after locale changes instead
of retaining the initial string. Use a ref-backed placeholder function, or
recreate the editor when placeholder changes, while preserving the existing
emptyEditorClass behavior.
In `@project/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md`:
- Around line 441-446: Update the “Next Steps” and rollout guidance to prohibit
boot-time backfills and require a controlled migration job for any writes. Keep
the read-only production audit first, then specify that remediation must be
idempotent, batched, ownership/lock protected, and observable to prevent
concurrent application instances from applying it.
---
Nitpick comments:
In `@apps/frontend-manage/src/components/common/ContentInput.tsx`:
- Line 175: Memoize the value passed to ToolbarContext.Provider in ContentInput
using the existing React memoization pattern, with disabled as its dependency.
Keep the provider behavior unchanged while ensuring ToolbarButton consumers do
not re-render when disabled is unchanged.
In `@apps/frontend-manage/src/globals.css`:
- Around line 314-415: Extract the duplicated table and code-highlighting
styles, including the table, pre, hljs, and token selectors, into a shared CSS
partial in the appropriate shared package. Import that partial from both apps’
globals.css files and remove the duplicated definitions, preserving the existing
selectors and styling.
In `@project/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md`:
- Line 37: Reword the dependency non-goal in the migration plan so it excludes
only dependencies beyond the Tiptap/Lowlight additions already introduced by
this PR, while retaining the intent to avoid major upgrades.
🪄 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: db66074c-dab4-49df-97c0-43bf5a6bcc1c
📒 Files selected for processing (20)
.agents/skills/klicker-frontend-ui/SKILL.md.agents/skills/klicker-playwright-e2e/SKILL.md.agents/skills/klicker-wiki-maintenance/SKILL.mdAGENTS.mdapps/frontend-manage/src/components/common/ContentInput.tsxapps/frontend-manage/src/components/courses/groupActivity/GroupActivityGradingStack.tsxapps/frontend-manage/src/components/elements/manipulation/ElementContentInput.tsxapps/frontend-manage/src/components/elements/manipulation/ElementExplanationField.tsxapps/frontend-manage/src/components/elements/manipulation/options/CaseStudyCasesFields.tsxapps/frontend-manage/src/components/elements/manipulation/options/ChoicesOptions.tsxapps/frontend-manage/src/globals.cssapps/frontend-pwa/src/globals.cssdocs/frontend-conventions.mddocs/log.mdpackages/i18n/messages/de.tspackages/i18n/messages/en.tsplaywright/tests/S-group-activity.spec.tsplaywright/tests/ZA-editor-rich-features.spec.tsplaywright/util/actions.tsproject/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md
💤 Files with no reviewable changes (2)
- packages/i18n/messages/de.ts
- packages/i18n/messages/en.ts
✅ Files skipped from review due to trivial changes (2)
- docs/frontend-conventions.md
- docs/log.md
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
Zganz404
left a comment
Zganz404
left a comment
There was a problem hiding this comment.
Logic looks correct. One suggestion: the early return on line prevents the cleanup function from running — worth adding a finally block.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/markdown/src/Markdown.tsx (1)
111-111: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPass
ignoreMissing: truetorehypePrismUnsupported fenced languages currently throw, so one bad code block can drop the whole markdown render into the"Failed to parse content."fallback.🤖 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/markdown/src/Markdown.tsx` at line 111, Update the rehypePrism plugin configuration in the Markdown rendering pipeline to pass the ignoreMissing option as true, ensuring unsupported fenced languages do not throw and trigger the failed-content fallback.
🤖 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.
Outside diff comments:
In `@packages/markdown/src/Markdown.tsx`:
- Line 111: Update the rehypePrism plugin configuration in the Markdown
rendering pipeline to pass the ignoreMissing option as true, ensuring
unsupported fenced languages do not throw and trigger the failed-content
fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f2afca9-a709-4b03-8b03-1ebc5e0940e0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/frontend-manage/package.jsonapps/frontend-manage/src/components/common/ContentInput.tsxdocs/log.mdpackages/markdown/src/Markdown.tsxplaywright/tests/ZA-editor-rich-features.spec.tsproject/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md
💤 Files with no reviewable changes (2)
- docs/log.md
- apps/frontend-manage/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- playwright/tests/ZA-editor-rich-features.spec.ts
- apps/frontend-manage/src/components/common/ContentInput.tsx
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@skills-lock.json`:
- Around line 34-38: Update the tiptap entry in skills-lock.json to reference
the existing local .agents/skills/tiptap/SKILL.md file, using the matching local
sourceType, source, and skillPath values, and recompute computedHash from that
local file.
🪄 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: 744141c3-ed89-4987-a279-97633ac1b45e
📒 Files selected for processing (8)
.agents/skills/klicker-frontend-ui/SKILL.md.agents/skills/tiptap/SKILL.md.gitignoreapps/frontend-manage/src/components/common/ContentInput.tsxdocs/frontend-conventions.mdplaywright/tests/ZA-editor-rich-features.spec.tsproject/2026-07-10-pr-5148-tiptap-editor-finalization-plan.mdskills-lock.json
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/frontend-conventions.md
- playwright/tests/ZA-editor-rich-features.spec.ts
- project/2026-07-10-pr-5148-tiptap-editor-finalization-plan.md
- apps/frontend-manage/src/components/common/ContentInput.tsx
| "tiptap": { | ||
| "source": "ueberdosis/tiptap", | ||
| "sourceType": "github", | ||
| "skillPath": "skills/tiptap/SKILL.md", | ||
| "computedHash": "d3abb7a7e4cb1eb4064e8dcb0928771d58052c7d92e2ec3725a3a9a3c6cd338b" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check if the local skill file exists and compute its SHA-256 hash
echo "=== Local file hash ==="
sha256sum .agents/skills/tiptap/SKILL.md
echo ""
echo "=== Expected hash from skills-lock.json ==="
echo "d3abb7a7e4cb1eb4064e8dcb0928771d58052c7d92e2ec3725a3a9a3c6cd338b"
echo ""
echo "=== Other local skills for comparison ==="
grep -A 4 'klicker-playwright-e2e' skills-lock.jsonRepository: uzh-bf/klicker-uzh
Length of output: 621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== skills-lock.json tiptap entry ==='
grep -A 6 -n '"tiptap"' skills-lock.json || true
echo
echo '=== local skill files ==='
find .agents/skills -maxdepth 2 -name SKILL.md | sort || true
echo
echo '=== tiptap file info ==='
python3 - <<'PY'
from pathlib import Path
import hashlib
p = Path('.agents/skills/tiptap/SKILL.md')
print('exists:', p.exists())
if p.exists():
data = p.read_bytes()
print('bytes:', len(data))
print('sha256:', hashlib.sha256(data).hexdigest())
print('first line:', data.splitlines()[0].decode('utf-8', 'replace') if data.splitlines() else '')
PYRepository: uzh-bf/klicker-uzh
Length of output: 1377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== skills-lock.json tiptap entry ===\n'
python3 - <<'PY'
import json
from pathlib import Path
data = json.loads(Path('skills-lock.json').read_text())
entry = data.get('tiptap')
print(json.dumps(entry, indent=2, sort_keys=True))
PY
printf '\n=== local path checks ===\n'
python3 - <<'PY'
from pathlib import Path
import hashlib
for path in [
Path('.agents/skills/tiptap/SKILL.md'),
Path('skills/tiptap/SKILL.md'),
]:
print(path, 'exists=', path.exists())
if path.exists():
data = path.read_bytes()
print(' sha256=', hashlib.sha256(data).hexdigest())
print(' bytes=', len(data))
PYRepository: uzh-bf/klicker-uzh
Length of output: 396
Fix the tiptap lock entry to point at the local skill. .agents/skills/tiptap/SKILL.md exists, skills/tiptap/SKILL.md does not, and the stored hash does not match the local file. Use the local sourceType/source/skillPath pair here.
🤖 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 `@skills-lock.json` around lines 34 - 38, Update the tiptap entry in
skills-lock.json to reference the existing local .agents/skills/tiptap/SKILL.md
file, using the matching local sourceType, source, and skillPath values, and
recompute computedHash from that local file.
Summary
This PR replaces the Slate-based lecturer editor with Tiptap while keeping Markdown as the persisted format.
<br>empty sentinel and treats break-only legacy rows as empty when reopened.Implementation notes
ContentInputparses and serializes Markdown through Tiptap 3.27.1.emitUpdate: false, preserves Markdown-significant leading whitespace, and applies focused parent resets instead of dropping them.''orundefined. A single editor-boundary normalizer maps sentinel-only<br>,<br/>, and<br />variants to an empty document without changing mixed Markdown.remark-gfmandrehype-prism-plus. Raw HTML remains disabled and the sanitizer only admitslanguage-*code classes.ContentInputusesuseEditorStatefor selection-dependent controls and setsimmediatelyRender: falsefor Next.js SSR..markdown-contenttable and code selectors..reference/.No schema migration or database write is included. Tiptap's programmatic synchronization suppresses updates, so the removed fallback did not write new sentinels. Historic Slate rows may still exist in scalar Markdown fields and JSON snapshots. Existing rows now reopen correctly. Any database cleanup should start with read-only production counts. If confirmed optional-field counts justify remediation, use an explicit idempotent batched job with single-owner locking, dry-run output, and progress/failure observability. Required content and
ElementInstancesnapshots need targeted handling, not a blind replacement.Branch coverage
v3at currentorigin/v39b18a0444cReview focus
packages/markdown.ZA-editor-rich-featuresassertions for table dimensions, JavaScript, TypeScript, and R tokens, saved-editor reopen behavior, plus new and legacy empty-editor placeholders.Verification
Current head:
pnpm run check:all: passed.ZA-editor-rich-features: passed.finally.9b18a0444c(Playwright run 29186014786). Playwright shard 4 initially hit a random seededParticipantGroup(courseId, code)collision before tests started; rerunning the failed job passed seeding and the complete shard.Earlier branch verification:
59453d5eee.c2849f526d.9e0438039e.opengrep scan --config autoreported 0 findings on the editor files before the final simplification.colspan, which is why merge and split controls were removed.Warnings and gaps:
T-resourcesanswer-collection action. Its full failed-job rerun passed without a branch change, so this is classified as a test flake rather than an editor regression.pr5148-finaldevrouter preview. Workspace authentication then posted to the base auth host, which belongs to another active worktree, so no authenticated browser assertion or screenshot is claimed for this commit.Security / privacy
rehype-sanitizeremains active before syntax highlighting.v3; an authorized maintainer must classify or clear that incident.Blocking before merge
9b18a0444c.Follow-up after merge