Repository navigation
feat: improve diagram creation and quick views - #111
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:
📝 WalkthroughWalkthroughThis PR adds a shared diagram catalog, treats blank diagrams as an empty state, adds starring for diagrams and folders, and updates dashboard browsing and open behavior to use quick views and new-tab editor links. ChangesDiagram Workflows and Dashboard Behavior
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/hooks/useEditorState.ts (1)
43-50: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBlank short-circuit doesn't reset theme/font state.
Clearing
parseError/svgContent/renderIdRefis correct, butcurrentTheme/currentFontretain their last-rendered values when transitioning to blank. If the user then starts a brand-new diagram, the header may briefly show a stale theme/font from the previous content until the next successful render recalculates them. Low-impact but worth a defensive reset for consistency.As per coding guidelines, "Before implementing Mermaid logic, read `reference/standards/mermaid.md` and do not guess Mermaid syntax" — please confirm this blank short-circuit was validated against that standard.♻️ Optional fix
if (mermaidCode.trim().length === 0) { setParseError(null); setSvgContent(""); + setCurrentTheme("default"); + setCurrentFont("Default"); renderIdRef.current = null; if (onResetSelection) onResetSelection(); return; }🤖 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 `@src/hooks/useEditorState.ts` around lines 43 - 50, The blank-state early return in useEditorState should also defensively reset currentTheme and currentFont, since they currently keep stale values after the diagram is cleared. Update the empty-input branch in the editor state logic to clear those theme/font fields alongside parseError, svgContent, and renderIdRef, and keep the onResetSelection behavior unchanged. Confirm this change only affects the blank short-circuit path in useEditorState and is consistent with the Mermaid standards referenced for this logic.Source: Coding guidelines
src/lib/diagrams/utils.ts (1)
82-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOnly
C4Contexthas a friendly label; other C4 variants fall through to generic formatting.
C4Container,C4Component,C4Dynamic, andC4Deploymentare now detected by the regex at Line 82-83 but aren't present indiagramTypeLabel'sKNOWNmap (onlyC4Contextis, Line 121), so they'll render as auto-spaced labels (e.g. "C4 Dynamic") instead of a consistent "C4 Diagram" style label.♻️ Proposed fix
- C4Context: "C4 Diagram", + C4Context: "C4 Diagram", + C4Container: "C4 Diagram", + C4Component: "C4 Diagram", + C4Dynamic: "C4 Diagram", + C4Deployment: "C4 Diagram",Also applies to: 121-121
🤖 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 `@src/lib/diagrams/utils.ts` around lines 82 - 83, The C4 variant detection in diagramTypeLabel currently recognizes C4Container, C4Component, C4Dynamic, and C4Deployment, but only C4Context has a friendly label in the KNOWN map, so the other variants fall back to generic auto-spacing. Update diagramTypeLabel’s KNOWN mapping to include all matched C4 variants (or normalize them to the same friendly label) so they render consistently instead of as separate spaced names.src/lib/api/storageTypes.ts (1)
259-260: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
starred/starredAtinvariant not enforced in normalization.
normalizeDiagramDocument/normalizeFolderindependently coercestarredandstarredAtwithout correlating them, unlike the PUT API handlers which always null outstarredAtwhen unstarred. If raw/persisted data ever hasstarred: falsewith a stalestarredAtstring (e.g., direct storage edits or legacy data), normalization will surface an inconsistent state (unstarred item carrying a starred timestamp).🛡️ Proposed fix to enforce the invariant at normalization time
- starred: Boolean(raw.starred), - starredAt: typeof raw.starredAt === "string" ? raw.starredAt : null, + starred: Boolean(raw.starred), + starredAt: Boolean(raw.starred) && typeof raw.starredAt === "string" ? raw.starredAt : null,(apply the same pattern to
normalizeFolder)Also applies to: 275-276
🤖 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 `@src/lib/api/storageTypes.ts` around lines 259 - 260, normalizeDiagramDocument and normalizeFolder currently coerce starred and starredAt independently, which can expose stale starredAt values for unstarred items. Update the normalization logic in both functions so starredAt is only retained when the normalized starred flag is true, and otherwise is forced to null, matching the invariant already enforced by the PUT handlers. Use the existing starred/starredAt handling in storageTypes.ts as the place to apply the correlated normalization.
🤖 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 `@src/lib/diagrams/utils.ts`:
- Around line 80-88: determineDiagramType currently falls back to "blank" for
non-empty input that does not match any known diagram declaration, which causes
malformed or comment-only content to be treated the same as truly empty code.
Update the fallback in determineDiagramType in utils.ts to return a distinct
value such as "unknown", and make sure useEditorState and any header/card label
logic handle that new value separately from "blank" so only empty diagrams show
Blank Diagram.
---
Nitpick comments:
In `@src/hooks/useEditorState.ts`:
- Around line 43-50: The blank-state early return in useEditorState should also
defensively reset currentTheme and currentFont, since they currently keep stale
values after the diagram is cleared. Update the empty-input branch in the editor
state logic to clear those theme/font fields alongside parseError, svgContent,
and renderIdRef, and keep the onResetSelection behavior unchanged. Confirm this
change only affects the blank short-circuit path in useEditorState and is
consistent with the Mermaid standards referenced for this logic.
In `@src/lib/api/storageTypes.ts`:
- Around line 259-260: normalizeDiagramDocument and normalizeFolder currently
coerce starred and starredAt independently, which can expose stale starredAt
values for unstarred items. Update the normalization logic in both functions so
starredAt is only retained when the normalized starred flag is true, and
otherwise is forced to null, matching the invariant already enforced by the PUT
handlers. Use the existing starred/starredAt handling in storageTypes.ts as the
place to apply the correlated normalization.
In `@src/lib/diagrams/utils.ts`:
- Around line 82-83: The C4 variant detection in diagramTypeLabel currently
recognizes C4Container, C4Component, C4Dynamic, and C4Deployment, but only
C4Context has a friendly label in the KNOWN map, so the other variants fall back
to generic auto-spacing. Update diagramTypeLabel’s KNOWN mapping to include all
matched C4 variants (or normalize them to the same friendly label) so they
render consistently instead of as separate spaced names.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ef1b14d9-d16f-4e12-ac68-6dff1b658afa
📒 Files selected for processing (15)
src/app/api/diagrams/[id]/route.tssrc/app/api/diagrams/route.tssrc/app/api/folders/[id]/route.tssrc/app/api/folders/route.tssrc/components/Dashboard.tsxsrc/components/DiagramCard.tsxsrc/components/FolderCard.tsxsrc/components/editor/EditorCanvas.tsxsrc/components/editor/EditorHeader.tsxsrc/components/editor/LiveMaidEditor.tsxsrc/hooks/useEditorState.tssrc/lib/api/storageTypes.tssrc/lib/diagrams/catalog.tssrc/lib/diagrams/utils.tssrc/test/diagram-catalog.test.ts
|
🚅 Deployed to the livemaid-pr-111 environment in livemaid
|
|
/coderabbitai review |
This branch was successfully deployed
2 participants
Summary
Verification
npm run prepushnpx playwright test --config=/tmp/livemaid-playwright-existing-server.config.cjsagainst the existing3434dev server1600px,1280px,760px, and390pxNotes
npm run lintreports existing warnings but no errors during prepush.3435while Next detected the existing workspace dev server on3434, so the targeted dashboard spec was rerun against3434with a temporary config.Closes #106
Closes #107
Closes #108
Closes #109
Closes #110
Summary by CodeRabbit
starred/starredAtvalidation and normalization across diagram and folder flows.