Feat react markdown #1
Conversation
- Added toolbar with formatting options (bold, italic, underline, strikethrough, links, code, blockquotes, and lists). - Introduced markdown to HTML conversion and vice versa for better content handling. - Enhanced contentEditable area for improved user interaction and experience. - Implemented link editing functionality with a popup for managing links. - Refactored content change handling to support rich text features and maintain state consistency.
- Updated overscroll behavior in CSS to contain for better scrolling experience. - Adjusted z-index values in various components for improved layering. - Implemented auto-scrolling during drag events in ArticleEditor for better usability. - Enhanced TextEditor with a configurable toolbar and floating toolbar for rich text editing features, including link management and formatting options.
WalkthroughThis PR addresses stacking order issues across multiple UI components and introduces significant editor enhancements. Five files receive z-index adjustments to ensure proper overlaying of dropdowns, panels, and modals. The team-blog-selector is refactored to use a portal-driven dropdown anchored to button coordinates rather than inline rendering. The content.tsx file receives extensive additions including a rich text editor with configurable toolbars, markdown/HTML conversion utilities, link editing popups, drag-and-drop section reordering, and keyboard shortcuts. Additionally, overscroll-behavior CSS values are changed from Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/team-blog/team-blog-selector.tsx (1)
54-66: Click-inside closes dropdown due to portal; include dropdownRef in outside check.The outside-click logic only checks containerRef. Since the dropdown is portaled to body, any click inside it is considered "outside" and closes immediately.
Apply:
- const buttonRef = useRef<HTMLButtonElement>(null) + const buttonRef = useRef<HTMLButtonElement>(null) + const dropdownRef = useRef<HTMLDivElement>(null)useEffect(() => { function handleClickOutside(event: MouseEvent) { - if (containerRef.current && !containerRef.current.contains(event.target as Node)) { - setIsOpen(false) - } + const target = event.target as Node + const withinToggle = !!containerRef.current?.contains(target) + const withinDropdown = !!dropdownRef.current?.contains(target) + if (!withinToggle && !withinDropdown) setIsOpen(false) } if (isOpen) { - document.addEventListener('mousedown', handleClickOutside) - return () => document.removeEventListener('mousedown', handleClickOutside) + document.addEventListener('mousedown', handleClickOutside) + return () => document.removeEventListener('mousedown', handleClickOutside) } }, [isOpen])Also attach the ref to the portal root:
- const dropdownContent = isOpen && createPortal( - <div className="fixed z-[100]" style={{ top: `${buttonPosition.top}px`, left: `${buttonPosition.left}px` }}> + const dropdownContent = isOpen && createPortal( + <div ref={dropdownRef} className="fixed z-[100]" style={{ top: `${buttonPosition.top}px`, left: `${buttonPosition.left}px` }}>Optional: use 'pointerdown' instead of 'mousedown' to close before focus changes, and add Escape-to-close via a keydown listener when open.
src/routes/_protected/content.tsx (2)
1443-1483: Fix duplicate article flow: wrong article updated/returned; revision created for wrong doc.The mutation creates
newArticlebut:
- calls
createInitialRevision(article, …)for the original,- updates
article.$id(original),- returns
article(original),- navigates using the original id.
Result: the “duplicate” menu copies but updates the source and sends the user back to the original. Fix by targeting
newArticle, returning it, and updating itsactiveRevisionId.const duplicateArticle = useMutation({ mutationFn: async () => { // Use shared state article data if (!article) { throw new Error('Article not found in shared state') } if (article.createdBy !== userId) throw new Error('Forbidden') // Create a copy of the article with a new ID const duplicateData: Omit<Articles, keyof Models.Document> = { trailer: article.trailer, title: `${article.title} (Copy)`, status: 'draft', // Always create as draft subtitle: article.subtitle, images: article.images, body: article.body, // Copy all sections authors: article.authors, live: false, // Always create as not live pinned: false, // Don't copy pin status redirect: article.redirect, categories: article.categories, createdBy: userId, slug: null, // Will be generated from title blogId: article.blogId, activeRevisionId: null, // Will be set after revision creation } - const newArticle = await db.articles.create(duplicateData, currentTeam?.$id) + const newArticle = await db.articles.create(duplicateData, currentTeam?.$id) // Create initial revision with user info const userInfo = { userId: user.$id, userName: user.name || '', userEmail: user.email || '' } - const revision = await createInitialRevision(article, currentTeam?.$id, userInfo) + const revision = await createInitialRevision(newArticle, currentTeam?.$id, userInfo) // Update article with revision ID - await db.articles.update(article.$id, { activeRevisionId: revision.$id }) + await db.articles.update(newArticle.$id, { activeRevisionId: revision.$id }) - return article + return newArticle }, - onSuccess: (newArticle) => { + onSuccess: (newArticle) => { qc.invalidateQueries({ queryKey: ['articles', userId, currentBlog?.$id] }) toast({ title: 'Article duplicated successfully', description: `Created "${newArticle.title}"` }) // Navigate to the new article navigate({ to: '/content', search: { articleId: newArticle.$id } }) },Also applies to: 1484-1492
5127-5133: Allow (0, 0) map coordinates; current truthy check hides valid locations.
nlat && nlngtreats 0 as falsy. Use finite checks.- {nlat && nlng ? ( + {Number.isFinite(nlat) && Number.isFinite(nlng) ? ( <div className="w-full aspect-video rounded-lg border overflow-hidden"> <iframe className="w-full h-full" src={iframe} title="Map preview" /> </div> ) : (
🧹 Nitpick comments (9)
src/components/team-blog/team-blog-selector.tsx (2)
161-174: A11y and semantics: add button type and ARIA state.Add type="button" to prevent accidental form submission and expose expanded state/relationship.
Apply:
- <Button + <Button ref={buttonRef} variant="ghost" size="sm" onClick={handleButtonClick} - className="flex items-center gap-2 text-sm font-medium hover:bg-accent cursor-pointer" + className="flex items-center gap-2 text-sm font-medium hover:bg-accent cursor-pointer" + type="button" + aria-haspopup="menu" + aria-expanded={isOpen} + aria-controls="team-blog-dropdown" >And set an id on the dropdown container:
- <div ref={dropdownRef} className="fixed z-[100]" style={{ top: `${buttonPosition.top}px`, left: `${buttonPosition.left}px` }}> + <div id="team-blog-dropdown" ref={dropdownRef} className="fixed z-[100]" style={{ top: `${buttonPosition.top}px`, left: `${buttonPosition.left}px` }}>
134-157: Portal block looks good; consider consistent layering token.Rendering via createPortal with fixed positioning is correct. Consider replacing hard-coded z-[100] with your standardized overlay token to keep layering consistent across the app.
src/routes/_protected/content.tsx (7)
1566-1624: Drag auto‑scroll: prefer rAF, throttle dragover, and mark listener passive to reduce jank.
dragoverfires at high frequency; you start a 16ms interval per edge. Use a single rAF loop while dragging, compute direction based on last pointer Y, and add the listener with{ passive: true }(since you don’t call preventDefault here).- document.addEventListener('dragover', handleGlobalDrag) + document.addEventListener('dragover', handleGlobalDrag, { passive: true })Conceptual rAF approach (snippet):
let rafId: number | null = null let lastY = 0 const onDragOver = (e: DragEvent) => { lastY = e.clientY } const tick = () => { const threshold = 100 const speed = 8 const vh = window.innerHeight const container = document.querySelector('.h-dvh.overflow-y-auto') as HTMLElement | null if (lastY < threshold) (container ?? window).scrollBy({ top: -speed }) else if (lastY > vh - threshold) (container ?? window).scrollBy({ top: speed }) rafId = requestAnimationFrame(tick) } // start tick on drag start, cancel on drag end: rafId && cancelAnimationFrame(rafId)
1201-1210: Avoid stale toggle in preferences mutation onSuccess.
setHideComments(!hideComments)may flip to the wrong state if multiple rapid toggles occur. Set directly from the intended value.- onSuccess: () => { - qc.invalidateQueries({ queryKey: ['auth', 'preferences'] }) - setHideComments(!hideComments) - }, + onSuccess: (_data, variables) => { + qc.invalidateQueries({ queryKey: ['auth', 'preferences'] }) + setHideComments(variables) // variables is the 'hide' argument passed to mutate + },
1883-1891: Prefer stable unique ids for sections (Date.now can collide).Rapid adds in the same ms can duplicate ids. Use
crypto.randomUUID()when available.- id: Date.now().toString(), + id: (crypto?.randomUUID?.() ?? `${Date.now()}-${Math.random().toString(36).slice(2)}`),
3513-3517: Clean up rowRefs on unmount to avoid stale entries.When a row unmounts,
elisnullbut the ref doesn’t delete the key. This can bloatrowRefs.current.ref={(el) => { - if (el) { - rowRefs.current[s.id] = el - } + if (el) rowRefs.current[s.id] = el + else delete rowRefs.current[s.id] }}
153-175: Consistency: useposition(notorder) in JSON export.Sections elsewhere use
position; JSON helper outputsorder. Align to one key to avoid downstream mismatch.- order: section.order || 0 + position: section.position ?? section.order ?? 0
4499-4503: Heads‑up:document.execCommandis deprecated.It still works but is on a deprecation path. Consider migrating to Range/Selection APIs or a small editor lib for formatting. Not blocking this PR.
2529-2551: LGTM: editor chrome layering.Sticky toolbars/banners at
z-[50]and menus at higherzlook consistent. Consider centralizing z-index tokens (e.g., via CSS variables) to avoid drift.Also applies to: 2644-2881, 2926-2933
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/agent/agent-chat.tsx(2 hunks)src/components/notification/notification-bell.tsx(1 hunks)src/components/team-blog/team-blog-selector.tsx(4 hunks)src/components/ui/loader.tsx(1 hunks)src/components/user-avatar.tsx(1 hunks)src/index.css(1 hunks)src/routes/_protected/content.tsx(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/routes/_protected/content.tsx (4)
functions/agent/index.js (1)
i(1266-1266)src/components/ui/input.tsx (1)
Input(39-39)src/components/ui/button.tsx (1)
Button(56-56)src/components/ui/label.tsx (1)
Label(24-24)
src/components/team-blog/team-blog-selector.tsx (2)
src/contexts/team-blog-context.tsx (1)
useTeamBlogContext(234-240)src/components/team-blog/team-blog-dropdown.tsx (1)
TeamBlogDropdown(36-344)
🪛 Biome (2.1.2)
src/routes/_protected/content.tsx
[error] 4109-4118: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4121-4130: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4149-4150: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4575-4576: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (9)
src/components/team-blog/team-blog-selector.tsx (2)
188-194: Verify CreateBlogModal teamId fallback.teamId falls back to '' when no team is selected. Confirm CreateBlogModal supports an empty teamId (UX and validation).
If needed, we can adjust to gate opening until a team exists or prompt for team selection.
2-2: LGTM on these localized changes.Imports, dropdownContent usage, and team creation wiring look consistent.
Also applies to: 6-6, 13-13, 176-176, 178-186
src/components/user-avatar.tsx (1)
181-182: LGTM! Z-index adjustment aligns with PR-wide stacking standardization.The z-index increase from 50 to 70 for both the dropdown arrow and panel is consistent with the same update in notification-bell.tsx and ensures proper layering above other UI elements.
src/components/ui/loader.tsx (1)
79-79: LGTM! Appropriate z-index for fullscreen overlay.The z-index increase to 9999 ensures the fullscreen loader appears above all other UI elements, which is the correct behavior for a global loading state.
src/components/notification/notification-bell.tsx (1)
63-64: LGTM! Consistent z-index adjustment for dropdown elements.The z-index increase to 70 matches the identical change in user-avatar.tsx, ensuring consistent stacking behavior across all dropdown components.
src/components/agent/agent-chat.tsx (1)
1989-1999: LGTM! Proper z-index hierarchy for chat panel and resize handle.The z-index adjustments maintain the correct stacking order:
- Resize handle (70) is positioned above the aside container (60) for proper interaction
- Both values fit appropriately within the overall hierarchy: loader (9999) > dropdowns/resize handle (70) > chat aside (60)
src/index.css (1)
124-135: Verify mobile scroll behavior after changing fromnonetocontain.The change from
overscroll-behavior: nonetooverscroll-behavior: containis confirmed and intentional:
- Previous (
none): Completely disabled scroll chaining and elastic/rubber-band effects- Current (
contain): Prevents scroll chaining to parent containers but re-enables elastic bounce effects on iOS devicesThis restores native elastic scrolling behavior across html, html-specific, and body selectors. The change is consistently applied throughout the codebase (code-mirror.css, block-edit.css, content.tsx). Test on actual iOS and Android devices to confirm this aligns with your desired mobile UX and works properly with the z-index stacking and portal-based dropdowns.
src/routes/_protected/content.tsx (2)
280-283: LGTM: container overscroll containment aligns with global change.Switch to
overscroll-containon the main scroll surface looks good.
292-336: LGTM: header layering.
stickyheader withz-[60]and backdrop-blur should resolve stacking collisions.
| // Update button position when dropdown opens or on scroll | ||
| useEffect(() => { | ||
| const updatePosition = () => { | ||
| if (buttonRef.current) { | ||
| const rect = buttonRef.current.getBoundingClientRect() | ||
| setButtonPosition({ | ||
| top: rect.bottom + window.scrollY, | ||
| left: rect.left + window.scrollX | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| if (isOpen) { | ||
| updatePosition() | ||
| const handleScroll = () => updatePosition() | ||
| window.addEventListener('scroll', handleScroll, true) | ||
| return () => window.removeEventListener('scroll', handleScroll, true) | ||
| } | ||
| }, [isOpen]) |
There was a problem hiding this comment.
Fix coordinate space: remove scroll offsets and handle resize.
You’re using a fixed-position portal but adding window.scrollX/Y to rect, which double-compensates on scroll. For position: fixed, use viewport coords (rect.* only). Also update on resize.
Apply:
// Update button position when dropdown opens or on scroll
useEffect(() => {
const updatePosition = () => {
if (buttonRef.current) {
const rect = buttonRef.current.getBoundingClientRect()
setButtonPosition({
- top: rect.bottom + window.scrollY,
- left: rect.left + window.scrollX
+ top: rect.bottom,
+ left: rect.left
})
}
}
if (isOpen) {
updatePosition()
- const handleScroll = () => updatePosition()
- window.addEventListener('scroll', handleScroll, true)
- return () => window.removeEventListener('scroll', handleScroll, true)
+ const handleScroll = () => updatePosition()
+ const handleResize = () => updatePosition()
+ window.addEventListener('scroll', handleScroll, true)
+ window.addEventListener('resize', handleResize)
+ return () => {
+ window.removeEventListener('scroll', handleScroll, true)
+ window.removeEventListener('resize', handleResize)
+ }
}
}, [isOpen])Optional: throttle updatePosition with requestAnimationFrame for smoother scrolling.
📝 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.
| // Update button position when dropdown opens or on scroll | |
| useEffect(() => { | |
| const updatePosition = () => { | |
| if (buttonRef.current) { | |
| const rect = buttonRef.current.getBoundingClientRect() | |
| setButtonPosition({ | |
| top: rect.bottom + window.scrollY, | |
| left: rect.left + window.scrollX | |
| }) | |
| } | |
| } | |
| if (isOpen) { | |
| updatePosition() | |
| const handleScroll = () => updatePosition() | |
| window.addEventListener('scroll', handleScroll, true) | |
| return () => window.removeEventListener('scroll', handleScroll, true) | |
| } | |
| }, [isOpen]) | |
| // Update button position when dropdown opens or on scroll | |
| useEffect(() => { | |
| const updatePosition = () => { | |
| if (buttonRef.current) { | |
| const rect = buttonRef.current.getBoundingClientRect() | |
| setButtonPosition({ | |
| top: rect.bottom, | |
| left: rect.left | |
| }) | |
| } | |
| } | |
| if (isOpen) { | |
| updatePosition() | |
| const handleScroll = () => updatePosition() | |
| const handleResize = () => updatePosition() | |
| window.addEventListener('scroll', handleScroll, true) | |
| window.addEventListener('resize', handleResize) | |
| return () => { | |
| window.removeEventListener('scroll', handleScroll, true) | |
| window.removeEventListener('resize', handleResize) | |
| } | |
| } | |
| }, [isOpen]) |
🤖 Prompt for AI Agents
In src/components/team-blog/team-blog-selector.tsx around lines 34 to 52, the
updatePosition function incorrectly adds window.scrollY/scrollX
(double-compensating for a fixed-position portal) and doesn't respond to
viewport resizes; change it to use the DOMRect viewport coordinates directly
(rect.bottom and rect.left) without adding scroll offsets, add a window 'resize'
listener alongside the 'scroll' listener and ensure both are cleaned up, and
optionally wrap updates in requestAnimationFrame (or a rAF-based throttle) to
avoid excessive reflows during continuous scroll/resize.
| switch (tagName) { | ||
| case 'ul': | ||
| const ulItems = Array.from(element.childNodes) | ||
| .filter(n => (n as HTMLElement).tagName === 'LI') | ||
| .map(n => { | ||
| const text = Array.from((n as HTMLElement).childNodes) | ||
| .map(c => extractText(c, false, '')) | ||
| .join('') | ||
| return '- ' + text.trim() | ||
| }) | ||
| .filter(text => text.trim()) | ||
| return ulItems.join('\n') + '\n\n' | ||
|
|
||
| case 'ol': | ||
| const olItems = Array.from(element.childNodes) | ||
| .filter(n => (n as HTMLElement).tagName === 'LI') | ||
| .map((n, idx) => { | ||
| const text = Array.from((n as HTMLElement).childNodes) | ||
| .map(c => extractText(c, false, '')) | ||
| .join('') | ||
| return `${idx + 1}. ${text.trim()}` | ||
| }) | ||
| .filter(text => text.trim()) | ||
| return olItems.join('\n') + '\n\n' | ||
|
|
||
| case 'li': | ||
| return inList ? listPrefix + childrenStr.trim() + '\n' : '- ' + childrenStr.trim() | ||
|
|
||
| case 'strong': | ||
| case 'b': | ||
| return `**${childrenStr}**` | ||
| case 'em': | ||
| case 'i': | ||
| return `*${childrenStr}*` | ||
| case 'u': | ||
| return `<u>${childrenStr}</u>` | ||
| case 's': | ||
| case 'del': | ||
| return `~~${childrenStr}~~` | ||
| case 'code': | ||
| return `\`${childrenStr}\`` | ||
| case 'a': | ||
| const href = element.getAttribute('href') || '' | ||
| return `[${childrenStr}](${href})` | ||
| case 'blockquote': | ||
| return `> ${childrenStr.replace(/\n/g, '\n> ')}` | ||
| case 'br': | ||
| return '\n' | ||
| case 'p': | ||
| case 'div': | ||
| return inList ? childrenStr : childrenStr + '\n\n' | ||
| default: | ||
| return childrenStr | ||
| } | ||
| } | ||
|
|
||
| const text = extractText(tempDiv) | ||
| return text.trim() |
There was a problem hiding this comment.
Resolve linter errors: declarations inside switch cases (wrap in blocks).
htmlToMarkdown declares const ulItems/olItems/href inside case labels. Wrap each case in { … } to satisfy noSwitchDeclarations and avoid cross-case leakage.
switch (tagName) {
- case 'ul':
- const ulItems = Array.from(element.childNodes)
+ case 'ul': {
+ const ulItems = Array.from(element.childNodes)
.filter(n => (n as HTMLElement).tagName === 'LI')
.map(n => {
const text = Array.from((n as HTMLElement).childNodes)
.map(c => extractText(c, false, ''))
.join('')
return '- ' + text.trim()
})
.filter(text => text.trim())
- return ulItems.join('\n') + '\n\n'
+ return ulItems.join('\n') + '\n\n'
+ }
- case 'ol':
- const olItems = Array.from(element.childNodes)
+ case 'ol': {
+ const olItems = Array.from(element.childNodes)
.filter(n => (n as HTMLElement).tagName === 'LI')
.map((n, idx) => {
const text = Array.from((n as HTMLElement).childNodes)
.map(c => extractText(c, false, ''))
.join('')
return `${idx + 1}. ${text.trim()}`
})
.filter(text => text.trim())
- return olItems.join('\n') + '\n\n'
+ return olItems.join('\n') + '\n\n'
+ }
…
- case 'a':
- const href = element.getAttribute('href') || ''
- return `[${childrenStr}](${href})`
+ case 'a': {
+ const href = element.getAttribute('href') || ''
+ return `[${childrenStr}](${href})`
+ }📝 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.
| switch (tagName) { | |
| case 'ul': | |
| const ulItems = Array.from(element.childNodes) | |
| .filter(n => (n as HTMLElement).tagName === 'LI') | |
| .map(n => { | |
| const text = Array.from((n as HTMLElement).childNodes) | |
| .map(c => extractText(c, false, '')) | |
| .join('') | |
| return '- ' + text.trim() | |
| }) | |
| .filter(text => text.trim()) | |
| return ulItems.join('\n') + '\n\n' | |
| case 'ol': | |
| const olItems = Array.from(element.childNodes) | |
| .filter(n => (n as HTMLElement).tagName === 'LI') | |
| .map((n, idx) => { | |
| const text = Array.from((n as HTMLElement).childNodes) | |
| .map(c => extractText(c, false, '')) | |
| .join('') | |
| return `${idx + 1}. ${text.trim()}` | |
| }) | |
| .filter(text => text.trim()) | |
| return olItems.join('\n') + '\n\n' | |
| case 'li': | |
| return inList ? listPrefix + childrenStr.trim() + '\n' : '- ' + childrenStr.trim() | |
| case 'strong': | |
| case 'b': | |
| return `**${childrenStr}**` | |
| case 'em': | |
| case 'i': | |
| return `*${childrenStr}*` | |
| case 'u': | |
| return `<u>${childrenStr}</u>` | |
| case 's': | |
| case 'del': | |
| return `~~${childrenStr}~~` | |
| case 'code': | |
| return `\`${childrenStr}\`` | |
| case 'a': | |
| const href = element.getAttribute('href') || '' | |
| return `[${childrenStr}](${href})` | |
| case 'blockquote': | |
| return `> ${childrenStr.replace(/\n/g, '\n> ')}` | |
| case 'br': | |
| return '\n' | |
| case 'p': | |
| case 'div': | |
| return inList ? childrenStr : childrenStr + '\n\n' | |
| default: | |
| return childrenStr | |
| } | |
| } | |
| const text = extractText(tempDiv) | |
| return text.trim() | |
| switch (tagName) { | |
| case 'ul': { | |
| const ulItems = Array.from(element.childNodes) | |
| .filter(n => (n as HTMLElement).tagName === 'LI') | |
| .map(n => { | |
| const text = Array.from((n as HTMLElement).childNodes) | |
| .map(c => extractText(c, false, '')) | |
| .join('') | |
| return '- ' + text.trim() | |
| }) | |
| .filter(text => text.trim()) | |
| return ulItems.join('\n') + '\n\n' | |
| } | |
| case 'ol': { | |
| const olItems = Array.from(element.childNodes) | |
| .filter(n => (n as HTMLElement).tagName === 'LI') | |
| .map((n, idx) => { | |
| const text = Array.from((n as HTMLElement).childNodes) | |
| .map(c => extractText(c, false, '')) | |
| .join('') | |
| return `${idx + 1}. ${text.trim()}` | |
| }) | |
| .filter(text => text.trim()) | |
| return olItems.join('\n') + '\n\n' | |
| } | |
| case 'li': | |
| return inList ? listPrefix + childrenStr.trim() + '\n' : '- ' + childrenStr.trim() | |
| case 'strong': | |
| case 'b': | |
| return `**${childrenStr}**` | |
| case 'em': | |
| case 'i': | |
| return `*${childrenStr}*` | |
| case 'u': | |
| return `<u>${childrenStr}</u>` | |
| case 's': | |
| case 'del': | |
| return `~~${childrenStr}~~` | |
| case 'code': | |
| return `\`${childrenStr}\`` | |
| case 'a': { | |
| const href = element.getAttribute('href') || '' | |
| return `[${childrenStr}](${href})` | |
| } | |
| case 'blockquote': | |
| return `> ${childrenStr.replace(/\n/g, '\n> ')}` | |
| case 'br': | |
| return '\n' | |
| case 'p': | |
| case 'div': | |
| return inList ? childrenStr : childrenStr + '\n\n' | |
| default: | |
| return childrenStr | |
| } | |
| } | |
| const text = extractText(tempDiv) | |
| return text.trim() |
🧰 Tools
🪛 Biome (2.1.2)
[error] 4109-4118: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4121-4130: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4149-4150: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/routes/_protected/content.tsx around lines 4106 to 4163, multiple const
declarations (ulItems, olItems, href) are declared directly inside switch case
blocks which violates the noSwitchDeclarations lint rule; wrap each affected
case in its own block (e.g., case 'ul': { ... } break;) so the const/let
declarations are scoped to that block, add explicit breaks where needed, and
ensure returned values still flow correctly (return inside the block or set a
local variable then break and return after the switch).
| if (e.ctrlKey || e.metaKey) { | ||
| switch (e.key) { | ||
| case 'b': | ||
| e.preventDefault() | ||
| execCommand('bold') | ||
| break | ||
| case 'u': | ||
| e.preventDefault() | ||
| execCommand('underline') | ||
| break | ||
| case 'i': | ||
| e.preventDefault() | ||
| execCommand('italic') | ||
| break | ||
| case 'k': | ||
| e.preventDefault() | ||
| const url = prompt('Enter URL:') | ||
| if (url) { | ||
| execCommand('createLink', url) | ||
| } | ||
| break | ||
| } |
There was a problem hiding this comment.
Resolve linter error in keyboard shortcut switch; isolate declarations per case.
const url = prompt(...) is declared directly under a case. Wrap each case body with { }.
switch (e.key) {
- case 'b':
+ case 'b': {
e.preventDefault()
execCommand('bold')
break
- case 'u':
+ }
+ case 'u': {
e.preventDefault()
execCommand('underline')
break
- case 'i':
+ }
+ case 'i': {
e.preventDefault()
execCommand('italic')
break
- case 'k':
+ }
+ case 'k': {
e.preventDefault()
const url = prompt('Enter URL:')
if (url) {
execCommand('createLink', url)
}
break
+ }
}📝 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.
| if (e.ctrlKey || e.metaKey) { | |
| switch (e.key) { | |
| case 'b': | |
| e.preventDefault() | |
| execCommand('bold') | |
| break | |
| case 'u': | |
| e.preventDefault() | |
| execCommand('underline') | |
| break | |
| case 'i': | |
| e.preventDefault() | |
| execCommand('italic') | |
| break | |
| case 'k': | |
| e.preventDefault() | |
| const url = prompt('Enter URL:') | |
| if (url) { | |
| execCommand('createLink', url) | |
| } | |
| break | |
| } | |
| if (e.ctrlKey || e.metaKey) { | |
| switch (e.key) { | |
| case 'b': { | |
| e.preventDefault() | |
| execCommand('bold') | |
| break | |
| } | |
| case 'u': { | |
| e.preventDefault() | |
| execCommand('underline') | |
| break | |
| } | |
| case 'i': { | |
| e.preventDefault() | |
| execCommand('italic') | |
| break | |
| } | |
| case 'k': { | |
| e.preventDefault() | |
| const url = prompt('Enter URL:') | |
| if (url) { | |
| execCommand('createLink', url) | |
| } | |
| break | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 4575-4576: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/routes/_protected/content.tsx around lines 4558 to 4579, the switch cases
for keyboard shortcuts declare a block-scoped variable (const url = prompt(...))
directly under a case which causes a linter error; wrap the body of any case
that declares variables in curly braces so the const is scoped properly (e.g.,
change case 'k': to case 'k': { const url = prompt(...); if (url)
execCommand('createLink', url); break; } ), and apply the same { } pattern to
any other case that will declare local variables.
| <Label htmlFor={`text-${section.id}`}>Text</Label> | ||
| <Textarea | ||
| id={`text-${section.id}`} | ||
| ref={ref} | ||
| value={value} | ||
| onChange={handleChange} | ||
| placeholder="Write text…" | ||
| rows={1} | ||
| className="min-h-[40px] text-sm w-full min-w-0" | ||
| style={{ overflow: 'hidden', resize: 'none', width: '100%', maxWidth: '100%' }} | ||
| {/* Toolbar */} | ||
| <div className="border border-input rounded-md"> | ||
| <div className="flex items-center gap-0.5 p-1 border-b border-input bg-muted/20 rounded-t-md"> |
There was a problem hiding this comment.
Fix labeling and ARIA for contentEditable (accessibility blocker).
<Label htmlFor={text-${section.id}}> points to no element; the hidden input lacks an id, and the editable div lacks role/aria. Screen readers won’t associate the label.
-<Label htmlFor={`text-${section.id}`}>Text</Label>
+<Label htmlFor={`text-${section.id}`}>Text</Label>
…
-<div
+<div
ref={editorRef}
contentEditable={!disabled}
+ role="textbox"
+ aria-multiline="true"
+ aria-labelledby={`text-${section.id}-label`}
onInput={handleContentChange}
onKeyDown={handleKeyDown}
onPaste={handlePaste}
onClick={handleClick}
className="min-h-[120px] …
data-placeholder="Write text…"
/>
…
-<input
+<input
type="text"
value={value}
onChange={() => {}}
readOnly
- className="sr-only"
+ className="sr-only"
+ id={`text-${section.id}`}
tabIndex={-1}
aria-hidden="true"
/>Optionally set the label id explicitly:
-<Label htmlFor={`text-${section.id}`}>Text</Label>
+<Label id={`text-${section.id}-label`} htmlFor={`text-${section.id}`}>Text</Label>Also applies to: 4814-4822
🤖 Prompt for AI Agents
In src/routes/_protected/content.tsx around lines 4611-4614 (and similarly for
4814-4822), the Label's htmlFor points to no element and the contentEditable div
has no ARIA/role, so screen readers can't associate the label with the editable
region; fix by giving the hidden/paired input an id that matches the Label's
htmlFor (e.g. id={`text-${section.id}`}) or add an explicit id on the Label and
reference it from the editable region, and add accessible attributes to the
editable div such as role="textbox", aria-multiline="true", aria-labelledby
pointing to the label id (or aria-label/aria-describedby as appropriate), and
ensure the editable div is keyboard-focusable (tabIndex=0) so assistive tech can
reach it.
| const url = prompt('Enter URL:') | ||
| if (url) execCommand('createLink', url) | ||
| }} | ||
| disabled={disabled} |
There was a problem hiding this comment.
🧩 Analysis chain
Validate/sanitize link URLs (avoid javascript: and other unsafe schemes).
Toolbar, floating toolbar, and link popup accept arbitrary strings. Enforce allowed schemes (http, https, mailto, tel) before creating/updating links to prevent unsafe links leaking into published content.
-const url = prompt('Enter URL:')
-if (url) execCommand('createLink', url)
+const url = prompt('Enter URL:')
+if (url && isSafeUrl(url)) execCommand('createLink', url)-const url = prompt('Add link:')
-if (url) execCommand('createLink', url)
+const url = prompt('Add link:')
+if (url && isSafeUrl(url)) execCommand('createLink', url)-const handleSubmit = (e: React.FormEvent) => {
+const handleSubmit = (e: React.FormEvent) => {
e.preventDefault()
- if (url && url !== linkElement.href) {
+ if (url && url !== linkElement.href && isSafeUrl(url)) {
onUpdate(url)
}
onClose()
}Add helper (outside changed hunk; safe utility):
function isSafeUrl(raw: string) {
try {
const u = new URL(raw, window.location.origin)
const allowed = ['http:', 'https:', 'mailto:', 'tel:']
return allowed.includes(u.protocol)
} catch {
return false
}
}Please verify your publish/render path also rejects unsafe hrefs or sanitizes HTML.
Also applies to: 4921-4934, 4291-4305
I'll verify this security review by examining the codebase for URL validation patterns, the specific locations mentioned, and how the publish/render path handles content.
🏁 Script executed:
#!/bin/bash
# Verify all three locations mentioned in the review
echo "=== Checking line ranges in content.tsx ==="
fd -t f content.tsx | xargs -I {} sh -c "echo 'File: {}'; wc -l {}"
# Show the three line ranges mentioned
echo -e "\n=== Lines 4673-4676 ==="
fd -t f content.tsx | xargs -I {} sh -c "sed -n '4673,4676p' {}"
echo -e "\n=== Lines 4921-4934 ==="
fd -t f content.tsx | xargs -I {} sh -c "sed -n '4921,4934p' {}"
echo -e "\n=== Lines 4291-4305 ==="
fd -t f content.tsx | xargs -I {} sh -c "sed -n '4291,4305p' {}"Length of output: 1490
🏁 Script executed:
#!/bin/bash
# Search for existing URL validation/sanitization utilities
echo "=== Searching for URL validation functions ==="
rg -nP --type=ts --type=tsx '(isSafeUrl|sanitizeUrl|validateUrl|isValidUrl|safeUrl)' -C 2
echo -e "\n=== Searching for URL validation patterns ==="
rg -nP --type=ts --type=tsx '(http:|https:|mailto:|tel:|protocol|scheme)' -C 2 | head -50
echo -e "\n=== Searching for existing URL checks ==="
rg -nP --type=ts --type=tsx 'new URL' -C 2Length of output: 220
🏁 Script executed:
#!/bin/bash
# Search for publish/render path and HTML sanitization
echo "=== Searching for sanitize/html functions ==="
rg -nP --type=ts --type=tsx '(sanitize|xss|dompurify|marked|html)' -C 2 | head -60
echo -e "\n=== Searching for publish/render paths ==="
rg -nP --type=ts --type=tsx '(publish|render|output|export).*href' -C 2 | head -40
echo -e "\n=== Checking for DOMPurify or similar ==="
rg -nP 'dompurify|sanitize' -C 2 | head -30Length of output: 2105
🏁 Script executed:
#!/bin/bash
# Search for execCommand usage and createLink specifics
echo "=== Searching for execCommand usage ==="
rg -nP --type=ts --type=tsx "execCommand\(" -C 3 | head -60
echo -e "\n=== Search for createLink specifics ==="
rg -nP --type=ts --type=tsx "createLink" -C 3Length of output: 136
🏁 Script executed:
#!/bin/bash
# Search without type specification - rg will auto-detect
echo "=== Searching for execCommand usage ==="
rg -nP "execCommand" -C 3 src/routes/_protected/content.tsx | head -100
echo -e "\n=== Search for createLink usage ==="
rg -nP "createLink" -C 3 src/routes/_protected/content.tsx
echo -e "\n=== Searching for DOMPurify import and usage ==="
rg -nP "dompurify|DOMPurify" -C 2 src/
echo -e "\n=== Searching for URL validation patterns ==="
rg -nP "(new URL|protocol|href|javascript:)" -C 2 src/routes/_protected/content.tsx | head -80Length of output: 6917
🏁 Script executed:
#!/bin/bash
# Search for render/publish/output paths and sanitization
echo "=== Searching for publish or render logic ==="
rg -nP "(publish|render|export|output)" -C 3 src/routes/_protected/content.tsx | head -60
echo -e "\n=== Searching for href assignments ==="
rg -nP "\.href\s*=" -C 2 src/routes/_protected/content.tsx | head -40
echo -e "\n=== Checking package.json for security-related deps ==="
fd -t f "package.json" | xargs cat | grep -A 100 '"dependencies"' | head -30Length of output: 3374
🏁 Script executed:
#!/bin/bash
# Look for content serialization and how articles are displayed
echo "=== Searching for dangerouslySetInnerHTML or similar ==="
rg -nP "(dangerouslySetInnerHTML|innerHTML|insertAdjacentHTML)" -C 2 src/routes/_protected/content.tsx
echo -e "\n=== Searching for article rendering/display ==="
rg -nP "(article.*render|display.*content|show.*article)" -i -C 2 src/routes/_protected/content.tsx | head -50
echo -e "\n=== Searching for edit content and handlers ==="
rg -nP "onUpdate.*url|handleSubmit.*url" -C 3 src/routes/_protected/content.tsx | head -50Length of output: 2363
🏁 Script executed:
#!/bin/bash
# Find how articles are published and displayed to users
echo "=== Searching for article publishing logic ==="
rg -nP "publish|status.*publish" -C 3 src/routes/_protected/content.tsx | head -80
echo -e "\n=== Searching for content rendering in display routes ==="
fd -t f -e tsx -e ts src/routes | xargs grep -l "article\|content" | grep -v protected | head -10
echo -e "\n=== Check for any public display routes ==="
find src/routes -type d | grep -v _protected | grep -v __Length of output: 4007
🏁 Script executed:
#!/bin/bash
# Search for how articles are actually displayed to end users
echo "=== Searching for article display/render components ==="
fd -t f -e tsx src/ | xargs rg -nP "(article.*content|render.*article)" -C 2 -i | head -60
echo -e "\n=== Looking for environment/deployment publish logic ==="
rg -nP "(deploy|publish.*revision|active.*revision)" -C 2 src/routes/_protected/content.tsx | head -40
echo -e "\n=== Check database operations to see if content is stored ==="
rg -nP "db\..*update|db\..*create.*article" -C 2 src/routes/_protected/content.tsx | head -40Length of output: 8502
🏁 Script executed:
#!/bin/bash
# Check the actual publish flow and how content reaches end users
echo "=== Find public blog/article display ==="
find src/routes -name "*.tsx" -o -name "*.ts" | xargs grep -l "blog\|article.*display\|article.*render" 2>/dev/null | grep -v protected
echo -e "\n=== Verify if innerHTML is used for displaying article content ==="
rg -nP "innerHTML|dangerouslySetInnerHTML" -B5 -A5 src/ | grep -A5 -B5 "article\|content" | head -50Length of output: 3549
🏁 Script executed:
#!/bin/bash
# Find article display component - check for blog-related public routes
echo "=== Search for blog/article display components ==="
fd . 'src/routes' --type f --extension tsx --extension ts | xargs grep -l "article\|blog" 2>/dev/null | head -20
echo -e "\n=== Look for actual rendering of article content ==="
rg -nP "article\.(content|body)|sections.*render|display.*article" -C 2 src/routes --type ts --type tsx 2>/dev/null | head -60Length of output: 503
🏁 Script executed:
#!/bin/bash
# Verify if content is publicly accessible
echo "=== Check for public API endpoints that serve articles ==="
rg -nP "(query|api|fetch).*article.*body|article.*content.*render" -C 2 src/ | head -40
echo -e "\n=== Check routes structure for public blog access ==="
ls -la src/routes/ | grep -v "_"
find src/routes -maxdepth 1 -type dLength of output: 781
Add URL scheme validation to prevent stored XSS via unsafe link protocols (javascript:, data:, etc.).
The vulnerability is confirmed across four locations where user input directly controls link creation without validation:
- Line 4576: Keyboard shortcut handler (
execCommand('createLink', url)) - Line 4674: Toolbar button (
execCommand('createLink', url)) - Line 4923: Floating toolbar (
execCommand('createLink', url)) - Line 4831–4832: Link popup update (direct
hrefandsetAttributeassignment)
Additionally, the markdown parser at line 4249 creates unvalidated href attributes. Arbitrary URLs are stored directly in article content, creating a stored XSS risk when articles are displayed.
Implement the suggested isSafeUrl() helper to validate against safe schemes (http:, https:, mailto:, tel:) before any link creation or update. Also verify your publish/render path sanitizes HTML (or migrate to text-safe output) to block unsafe protocols on display.
This pull request focuses on improving UI layering and dropdown behavior across several components, as well as refining scroll behavior for a smoother user experience. The most significant changes include refactoring the team/blog selector dropdown to use a portal for correct positioning, standardizing and increasing z-index values for UI elements to prevent stacking issues, and updating CSS scroll containment.
UI Layering and Z-Index Improvements:
z-indexvalues for key UI elements (e.g., sidebars, dropdowns, loaders, notification panels, and user avatars) to ensure they appear above other content and avoid stacking conflicts. (src/components/agent/agent-chat.tsx[1] [2];src/components/notification/notification-bell.tsx[3];src/components/user-avatar.tsx[4];src/components/ui/loader.tsx[5]Dropdown and Modal Behavior Enhancements:
TeamBlogSelectordropdown to use a React portal, ensuring the dropdown is positioned correctly relative to the button even when scrolling, and updated arrow placement for visual consistency. (src/components/team-blog/team-blog-selector.tsx[1] [2] [3] [4]Scroll Behavior Adjustments:
overscroll-behaviorfromnonetocontainonbodyandhtmlinsrc/index.cssto better manage scroll chaining and elastic scroll, improving the overall scrolling experience. (src/index.csssrc/index.cssL124-R135)Summary by CodeRabbit
Release Notes
New Features
Style