-
Notifications
You must be signed in to change notification settings - Fork 2
Add markdown support #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add markdown support #213
Conversation
…ng and rendering a markdown link and a markdown table
… review across performance, security, maintenance, etc
…mains applicable to only the local user, not across the collaboration session
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one suggestion on toast messages. Otherwise looks great!
| if (error instanceof TypeError && error.message.includes("Expected table token")) { | ||
| toast.error("Unable to paste table. The table format may be invalid."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Matching on actual error output strings is usually fairly brittle in my own experience. This one seems pretty specific as well since, I would assume, user's will be using more than just tables. I like the generic toast message below. That, to me, would handle all cases well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes good catch. I had Claude break this out into a general error function, but it didn't rework the overly specific error here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 6fb18fc
…general error function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Description
Enable full markdown interpretation in coaching notes editor, including markdown table paste support and enhanced link handling with keyboard shortcuts.
GitHub Issue: Closes #189
Changes
Markdown Table Support: Added markdown table parsing and rendering with
@tiptap/extension-tableintegrationmarkdown-table-extension.tsxwith custom parse/render handlers for marked.jsTableMarkdownPasteHandlerextension for clipboard paste detectionresizable: falsedue to known Yjs collaboration conflicttiptap-table.scsswith borders, padding, and header backgroundsTipTap Extension Configuration Fixes:
StarterKitwith individual extension imports to avoid History/Collaboration conflictUnderlineextension (was referenced in toolbar but not configured)Link Keyboard Shortcut (CMD+K / Ctrl+K):
LinkKeyboardShortcutextension for CMD+K/Ctrl+K supporttriggerLinkCreation()utility to eliminate duplication between button and keyboardLinkZombieCleanupextension for consistent behaviorCode Quality:
coaching-notes-markdown-extensions.test.tsx(3 passing tests)LinkBubbleMenucomponentScreenshots / Videos Showing UI Changes (if applicable)
Markdown tables and links converted to rich text in light mode:

Markdown tables and links converted to rich text in dark mode:

Testing Strategy
Manual Testing:
Some valid and invalid markdown tables and links for manual testing
Automated Testing:
npm test -- __tests__/components/ui/coaching-sessions/coaching-notes/coaching-notes-markdown-extensions.test.tsx npm run typecheckConcerns