Allow share both #1393
Allow share both #1393
Conversation
📝 WalkthroughWalkthroughIntroduces raw vs enhanced content awareness across share/export flows with a unified isViewingRaw flag, updates gating to hasShareableNote with dynamic shareTitle, adds an Obsidian icon and refined Obsidian export wiring, normalizes tag names by stripping leading "@", and prevents selection popover inside transcript areas. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant SB as Share Button/Chip UI
participant SL as useShareLogic
participant EH as exportHandlers
participant CL as Clipboard
participant PDF as PDF Generator
participant EM as Email Composer
participant OB as Obsidian Writer
U->>SB: Open Share
SB->>SL: compute hasShareableNote, isViewingRaw, shareTitle
alt hasShareableNote
SB-->>U: Show SharePopover (title based on isViewingRaw)
U->>SB: Choose action (Copy/PDF/Email/Obsidian)
SB->>EH: call handler(session, ... , isViewingRaw)
alt Copy
EH->>EH: pick content (raw vs enhanced, fallbacks)
EH->>CL: write text/html
else PDF
EH->>PDF: exportToPDF(session, theme, isViewingRaw)
else Email
EH->>EM: compose body (raw vs enhanced, fallbacks)
else Obsidian
EH->>OB: convert to MD (raw vs enhanced) and write
end
else No shareable content
SB-->>U: Show placeholder content
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
| const segments = htmlToStructuredText(session?.enhanced_memo_html || "No content available"); | ||
| let contentHtml = ""; | ||
| if (isViewingRaw && session?.raw_memo_html) { | ||
| contentHtml = session.raw_memo_html; |
There was a problem hiding this comment.
Assigning raw_memo_html to contentHtml introduces an XSS risk because the HTML is not sanitized before being inserted into the DOM
Prompt for AI agents
Address the following comment on apps/desktop/src/components/toolbar/utils/pdf-export.ts at line 443:
<comment>Assigning raw_memo_html to contentHtml introduces an XSS risk because the HTML is not sanitized before being inserted into the DOM</comment>
<file context>
@@ -437,7 +438,18 @@ export const exportToPDF = async (
pdf.line(margin, yPosition, pageWidth - margin, yPosition);
yPosition += lineHeight;
- const segments = htmlToStructuredText(session?.enhanced_memo_html || "No content available");
+ let contentHtml = "";
+ if (isViewingRaw && session?.raw_memo_html) {
+ contentHtml = session.raw_memo_html;
+ } else if (!isViewingRaw && session?.enhanced_memo_html) {
+ contentHtml = session.enhanced_memo_html;
</file context>
| if (session.enhanced_memo_html) { | ||
| if (isViewingRaw && session.raw_memo_html) { | ||
| textToCopy = html2md(session.raw_memo_html); | ||
| } else if (!isViewingRaw && session.enhanced_memo_html) { |
There was a problem hiding this comment.
Enhanced note is skipped when isViewingRaw is true but raw content is missing, leading to empty copy operation.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/editor-area/note-header/share-button-header.tsx at line 60:
<comment>Enhanced note is skipped when isViewingRaw is true but raw content is missing, leading to empty copy operation.</comment>
<file context>
@@ -51,11 +51,13 @@ interface ObsidianFolder {
}
const exportHandlers = {
- copy: async (session: Session): Promise<ExportResult> => {
+ copy: async (session: Session, isViewingRaw: boolean = false): Promise<ExportResult> => {
try {
let textToCopy = "";
- if (session.enhanced_memo_html) {
</file context>
| } else if (!isViewingRaw && session.enhanced_memo_html) { | |
| } else if (session.enhanced_memo_html) { |
![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 (9)
apps/desktop/src/components/toolbar/utils/pdf-export.ts (1)
441-453: Avoid duplicating content-picking logic across export pathsSame “raw vs enhanced with fallback” branching exists here and in ShareButton handlers (copy/email/obsidian). Centralize into a helper to keep behavior consistent.
Apply this minimal change here:
- let contentHtml = ""; - if (isViewingRaw && session?.raw_memo_html) { - contentHtml = session.raw_memo_html; - } else if (!isViewingRaw && session?.enhanced_memo_html) { - contentHtml = session.enhanced_memo_html; - } else if (session?.raw_memo_html) { - contentHtml = session.raw_memo_html; - } else { - contentHtml = "No content available"; - } + const contentHtml = selectSessionHtml(session, isViewingRaw);Add this helper in a shared utils module (recommended) or at top of this file:
export function selectSessionHtml( session: SessionData, isViewingRaw: boolean, ): string { if (isViewingRaw && session?.raw_memo_html) return session.raw_memo_html; if (!isViewingRaw && session?.enhanced_memo_html) return session.enhanced_memo_html; if (session?.raw_memo_html) return session.raw_memo_html; return "No content available"; }apps/desktop/src/components/toolbar/buttons/share-button.tsx (2)
28-63: Trim SVG inline comments to follow repo guideline (“comments: Why, not What”)The descriptive SVG comments document shapes (“Main base shape”, “Top right highlight”, etc.) which are “what”, not “why”. Remove or replace with a single short “why” comment (e.g., why a custom icon is embedded).
Apply this diff to remove noisy comments:
- {/* Main base shape */} <path d="M382.3 475.6c-3.1 23.4-26 41.6-48.7 35.3-32.4-8.9-69.9-22.8-103.6-25.4l-51.7-4a34 34 0 0 1-22-10.2l-89-91.7a34 34 0 0 1-6.7-37.7s55-121 57.1-127.3c2-6.3 9.6-61.2 14-90.6 1.2-7.9 5-15 11-20.3L248 8.9a34.1 34.1 0 0 1 49.6 4.3L386 125.6a37 37 0 0 1 7.6 22.4c0 21.3 1.8 65 13.6 93.2 11.5 27.3 32.5 57 43.5 71.5a17.3 17.3 0 0 1 1.3 19.2 1494 1494 0 0 1-44.8 70.6c-15 22.3-21.9 49.9-25 73.1z" fill="currentColor" /> - {/* Bottom left highlight - creates internal structure */} <path d="M165.9 478.3c41.4-84 40.2-144.2 22.6-187-16.2-39.6-46.3-64.5-70-80-.6 2.3-1.3 4.4-2.2 6.5L60.6 342a34 34 0 0 0 6.6 37.7l89.1 91.7a34 34 0 0 0 9.6 7z" fill="rgba(255,255,255,0.15)" /> - {/* Top right highlight */} <path d="M278.4 307.8c11.2 1.2 22.2 3.6 32.8 7.6 34 12.7 65 41.2 90.5 96.3 1.8-3.1 3.6-6.2 5.6-9.2a1536 1536 0 0 0 44.8-70.6 17 17 0 0 0-1.3-19.2c-11-14.6-32-44.2-43.5-71.5-11.8-28.2-13.5-72-13.6-93.2 0-8.1-2.6-16-7.6-22.4L297.6 13.2a34 34 0 0 0-1.5-1.7 96 96 0 0 1 2 54 198.3 198.3 0 0 1-17.6 41.3l-7.2 14.2a171 171 0 0 0-19.4 71c-1.2 29.4 4.8 66.4 24.5 115.8z" fill="rgba(255,255,255,0.2)" /> - {/* Top left highlight */} <path d="M278.4 307.8c-19.7-49.4-25.8-86.4-24.5-115.9a171 171 0 0 1 19.4-71c2.3-4.8 4.8-9.5 7.2-14.1 7.1-13.9 14-27 17.6-41.4a96 96 0 0 0-2-54A34.1 34.1 0 0 0 248 9l-105.4 94.8a34.1 34.1 0 0 0-10.9 20.3l-12.8 85-.5 2.3c23.8 15.5 54 40.4 70.1 80a147 147 0 0 1 7.8 24.8c28-6.8 55.7-11 82.1-8.3z" fill="rgba(255,255,255,0.25)" /> - {/* Bottom right highlight */} <path d="M333.6 511c22.7 6.2 45.6-12 48.7-35.4a187 187 0 0 1 19.4-63.9c-25.6-55-56.5-83.6-90.4-96.3-36-13.4-75.2-9-115 .7 8.9 40.4 3.6 93.3-30.4 162.2 4 1.8 8.1 3 12.5 3.3 0 0 24.4 2 53.6 4.1 29 2 72.4 17.1 101.6 25.2z" fill="rgba(255,255,255,0.1)" />Optionally add a single “why” comment above the SVG:
// Why: custom Obsidian brand icon (not in lucide)
491-501: Deduplicate “pick content” branching across handlersThe raw/enhanced selection with fallback is repeated here (copy/email/obsidian) and in
exportToPDF. Extract a single helper (e.g.,selectSessionHtml(session, isViewingRaw)) and reuse across all call sites to keep behavior uniform and reduce future drift.Example minimal replacements:
- if (isViewingRaw && session.raw_memo_html) { - textToCopy = html2md(session.raw_memo_html); - } else if (!isViewingRaw && session.enhanced_memo_html) { - textToCopy = html2md(session.enhanced_memo_html); - } else if (session.raw_memo_html) { - textToCopy = html2md(session.raw_memo_html); - } else { - textToCopy = session.title || "No content available"; - } + const html = selectSessionHtml(session, isViewingRaw); + textToCopy = html ? html2md(html) : (session.title || "No content available");- if (isViewingRaw && session.raw_memo_html) { - bodyContent += html2md(session.raw_memo_html); - } else if (!isViewingRaw && session.enhanced_memo_html) { - bodyContent += html2md(session.enhanced_memo_html); - } else if (session.raw_memo_html) { - bodyContent += html2md(session.raw_memo_html); - } else { - bodyContent += "No content available"; - } + const html = selectSessionHtml(session, isViewingRaw); + bodyContent += html ? html2md(html) : "No content available";- let convertedMarkdown = ""; - if (isViewingRaw && session.raw_memo_html) { - convertedMarkdown = html2md(session.raw_memo_html); - } else if (!isViewingRaw && session.enhanced_memo_html) { - convertedMarkdown = html2md(session.enhanced_memo_html); - } else if (session.raw_memo_html) { - convertedMarkdown = html2md(session.raw_memo_html); - } + const html = selectSessionHtml(session, isViewingRaw); + const convertedMarkdown = html ? html2md(html) : "";You can place the helper in
apps/desktop/src/components/toolbar/utils/share-content.tsand reuse it here and inpdf-export.ts.Also applies to: 513-519, 525-536, 571-601
apps/desktop/src/components/editor-area/note-header/share-button-header.tsx (6)
54-66: DRY up note-content selection across share flowsThe raw/enhanced selection logic appears multiple times (copy/email/obsidian). Centralizing it avoids divergence and eases future tweaks.
Apply this diff within the copy handler:
- let textToCopy = ""; - - if (isViewingRaw && session.raw_memo_html) { - textToCopy = html2md(session.raw_memo_html); - } else if (!isViewingRaw && session.enhanced_memo_html) { - textToCopy = html2md(session.enhanced_memo_html); - } else if (session.raw_memo_html) { - textToCopy = html2md(session.raw_memo_html); - } else { - textToCopy = session.title || "No content available"; - } + const html = selectShareHtml(session, isViewingRaw); + const textToCopy = html ? html2md(html) : (session.title || "No content available");Supporting helper (place near exportHandlers):
function selectShareHtml(session: Session, isViewingRaw: boolean): string | undefined { if (isViewingRaw && session.raw_memo_html) return session.raw_memo_html; if (!isViewingRaw && session.enhanced_memo_html) return session.enhanced_memo_html; if (session.raw_memo_html) return session.raw_memo_html; return undefined; }
89-99: Reuse selection helper and simplify email body compositionLeverage the same selection helper here to keep logic consistent with other flows.
Apply this diff:
- let bodyContent = "Here is the meeting summary: \n\n"; - - if (isViewingRaw && session.raw_memo_html) { - bodyContent += html2md(session.raw_memo_html); - } else if (!isViewingRaw && session.enhanced_memo_html) { - bodyContent += html2md(session.enhanced_memo_html); - } else if (session.raw_memo_html) { - bodyContent += html2md(session.raw_memo_html); - } else { - bodyContent += "No content available"; - } + const html = selectShareHtml(session, isViewingRaw); + let bodyContent = "Here is the meeting summary: \n\n" + (html ? html2md(html) : "No content available");
114-126: Prefer recipients in the mailto path over a “to” query paramSome clients ignore the "to" header param. Putting recipients in the path is more broadly compatible. Also add a subject fallback.
Apply this diff:
- const participantEmails = sessionParticipants - ?.filter(participant => participant.email && participant.email.trim()) - ?.map(participant => participant.email!) - ?.join(",") || ""; - - const subject = encodeURIComponent(session.title); - const body = encodeURIComponent(bodyContent); - - const to = participantEmails ? `&to=${encodeURIComponent(participantEmails)}` : ""; - - const url = `mailto:?subject=${subject}&body=${body}${to}`; + const toList = sessionParticipants + ?.map(p => p.email?.trim()) + ?.filter(Boolean) + ?.join(",") || ""; + + const subject = encodeURIComponent(session.title || "Meeting notes"); + const body = encodeURIComponent(bodyContent); + + const recipientPart = toList ? encodeURIComponent(toList) : ""; + const url = `mailto:${recipientPart}?subject=${subject}&body=${body}`;Would you like me to run a quick repo-wide check to see if other mailto builders use the "to" query param?
134-165: Obsidian export: reuse helper and ensure graceful empty-content handlingReuse the selection helper to mirror the other flows. If neither HTML is present, you currently write an empty file body—acceptable but consider a lightweight fallback (e.g., title header) if desired.
Apply this diff:
- let convertedMarkdown = ""; - if (isViewingRaw && session.raw_memo_html) { - convertedMarkdown = html2md(session.raw_memo_html); - } else if (!isViewingRaw && session.enhanced_memo_html) { - convertedMarkdown = html2md(session.enhanced_memo_html); - } else if (session.raw_memo_html) { - convertedMarkdown = html2md(session.raw_memo_html); - } + const html = selectShareHtml(session, isViewingRaw); + let convertedMarkdown = html ? html2md(html) : "";
187-192: Normalize tag names when patching Obsidian frontmatter (strip leading “@”)If tag normalization is a goal, strip any leading "@", so Obsidian tags don’t carry that prefix.
Apply this diff:
- value: sessionTags.map(tag => tag.name), + value: sessionTags.map(tag => tag.name.replace(/^@+/, "")),If normalization happens upstream already, feel free to skip this.
579-583: Avoid shadowing: clarify shareTitle override vs computed valueYou accept a shareTitle prop and also compute shareTitle via the hook. Renaming the destructured value reduces ambiguity and telegraphs the override behavior.
Apply this diff:
- const { - shareTitle, + const { + shareTitle: computedShareTitle, expandedId, selectedObsidianFolder, setSelectedObsidianFolder, selectedPdfTheme, setSelectedPdfTheme, includeTranscript, setIncludeTranscript, copySuccess, obsidianFolders, directActions, exportOptions, exportMutation, toggleExpanded, handleExport, } = useShareLogic();- <div className="text-sm font-medium text-neutral-700">{propShareTitle || shareTitle}</div> + <div className="text-sm font-medium text-neutral-700">{propShareTitle || computedShareTitle}</div>Also applies to: 600-600
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/desktop/src/components/editor-area/index.tsx(1 hunks)apps/desktop/src/components/editor-area/note-header/chips/share-chip.tsx(3 hunks)apps/desktop/src/components/editor-area/note-header/share-button-header.tsx(11 hunks)apps/desktop/src/components/editor-area/text-selection-popover.tsx(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(13 hunks)apps/desktop/src/components/toolbar/utils/pdf-export.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/editor-area/index.tsxapps/desktop/src/components/editor-area/text-selection-popover.tsxapps/desktop/src/components/toolbar/utils/pdf-export.tsapps/desktop/src/components/editor-area/note-header/chips/share-chip.tsxapps/desktop/src/components/editor-area/note-header/share-button-header.tsxapps/desktop/src/components/toolbar/buttons/share-button.tsx
🧬 Code graph analysis (3)
apps/desktop/src/components/editor-area/note-header/chips/share-chip.tsx (1)
apps/desktop/src/components/editor-area/note-header/share-button-header.tsx (2)
useShareLogic(340-576)SharePopoverContent(579-746)
apps/desktop/src/components/editor-area/note-header/share-button-header.tsx (5)
plugins/db/js/bindings.gen.ts (1)
Session(181-181)packages/tiptap/src/shared/utils.ts (1)
html2md(48-50)apps/desktop/src/components/toolbar/utils/pdf-export.ts (2)
ThemeName(8-8)exportToPDF(308-529)apps/desktop/src/components/toolbar/utils/pdf-themes.ts (1)
ThemeName(1-14)packages/utils/src/contexts/sessions.tsx (1)
useSession(49-74)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (4)
packages/utils/src/contexts/sessions.tsx (1)
useSession(49-74)apps/desktop/src/components/toolbar/utils/pdf-export.ts (2)
ThemeName(8-8)exportToPDF(308-529)plugins/db/js/bindings.gen.ts (1)
Session(181-181)packages/tiptap/src/shared/utils.ts (1)
html2md(48-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (12)
apps/desktop/src/components/toolbar/utils/pdf-export.ts (1)
311-312: AllexportToPDFcall sites updatedI’ve located calls to
exportToPDFin both places where it’s used:
- apps/desktop/src/components/toolbar/buttons/share-button.tsx
- apps/desktop/src/components/editor-area/note-header/share-button-header.tsx
Each invocation now passes the
isViewingRawargument explicitly. No further updates are needed.apps/desktop/src/components/editor-area/note-header/chips/share-chip.tsx (3)
12-19: Good: gate side effects behind hasShareableNoteDeferring
handleOpenStateChangeuntil there’s shareable content avoids unnecessary queries and analytics when showing the placeholder.
38-39: LGTM: conditional contentSwitching between
<SharePopoverContent>and a placeholder based onhasShareableNotematches the updated sharing model.
47-58: Copy updates are clear and consistentThe placeholder header and guidance now reflect the generalized “Share Note” flow.
apps/desktop/src/components/toolbar/buttons/share-button.tsx (3)
82-88: Clean content gating and UX
hasShareableNote,isViewingRaw, andshareTitlecalculations are correct and align with the new “share both” behavior.
205-243: Propagating isViewingRaw to all exports is correctPassing
isViewingRawinto copy/pdf/email/obsidian handlers ensures consistent content-source selection.
291-292: Disable Share when nothing to shareGood UX guard. Prevents opening an interactive popover with no actionable options.
apps/desktop/src/components/editor-area/note-header/share-button-header.tsx (5)
76-82: Good: PDF export now respects raw/enhanced selectionPassing isViewingRaw through to exportToPDF aligns the UI state and the generated artifact.
352-358: Derivation of isViewingRaw/shareTitle looks correctThis matches the intended behavior: default to raw when enhanced isn’t available; otherwise follow showRaw. Dynamic header text is a nice touch.
454-465: Consistent propagation of isViewingRaw across actionsPassing isViewingRaw into copy/pdf/email keeps behavior aligned with the UI toggle.
483-490: Obsidian export wiring is robustRefetching tags/participants on demand and forwarding isViewingRaw is sound. Good job handling stale caches.
555-558: Exposing hasShareableNote/shareTitle/isViewingRaw from the hook is usefulThis enables upstream gating and consistent header text. No issues spotted.
| const cleanedTagName = tagName.startsWith("@") ? tagName.slice(1) : tagName; | ||
| const existingTag = existingTagsMap.get(cleanedTagName.toLowerCase()); | ||
|
|
||
| const tag = await dbCommands.upsertTag({ | ||
| id: existingTag?.id || crypto.randomUUID(), | ||
| name: tagName, | ||
| name: cleanedTagName, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize suggested tag names more robustly and avoid empty tags
- Strip multiple leading “@” and trim whitespace.
- Skip creating a tag when the cleaned name becomes empty (e.g., tag
"@"). - De-dup against legacy “@tag” entries to prevent duplicates after normalization.
Apply this diff:
- const cleanedTagName = tagName.startsWith("@") ? tagName.slice(1) : tagName;
- const existingTag = existingTagsMap.get(cleanedTagName.toLowerCase());
+ const cleanedTagName = tagName.replace(/^@+/, "").trim();
+ if (!cleanedTagName) {
+ continue;
+ }
+ const existingTag =
+ existingTagsMap.get(cleanedTagName.toLowerCase())
+ // fallback: match legacy '@tag' entries to avoid duplicates
+ || existingTagsMap.get(("@"+cleanedTagName).toLowerCase());📝 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.
| const cleanedTagName = tagName.startsWith("@") ? tagName.slice(1) : tagName; | |
| const existingTag = existingTagsMap.get(cleanedTagName.toLowerCase()); | |
| const tag = await dbCommands.upsertTag({ | |
| id: existingTag?.id || crypto.randomUUID(), | |
| name: tagName, | |
| name: cleanedTagName, | |
| }); | |
| const cleanedTagName = tagName.replace(/^@+/, "").trim(); | |
| if (!cleanedTagName) { | |
| continue; | |
| } | |
| const existingTag = | |
| existingTagsMap.get(cleanedTagName.toLowerCase()) | |
| // fallback: match legacy '@tag' entries to avoid duplicates | |
| || existingTagsMap.get(("@"+cleanedTagName).toLowerCase()); | |
| const tag = await dbCommands.upsertTag({ | |
| id: existingTag?.id || crypto.randomUUID(), | |
| name: cleanedTagName, | |
| }); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/editor-area/index.tsx around lines 84–90,
normalize the incoming tagName by removing all leading '@' characters and
trimming whitespace (e.g., tagName.replace(/^@+/, '').trim()), then if the
cleaned name is empty skip creating/upserting the tag; for deduplication also
check existingTagsMap for both cleanedName.toLowerCase() and ('@' +
cleanedName).toLowerCase() to find legacy entries and reuse their id when
calling dbCommands.upsertTag (generate a new id only when no existing tag is
found).
| const commonAncestor = range.commonAncestorContainer; | ||
| const container = commonAncestor.nodeType === Node.TEXT_NODE | ||
| ? commonAncestor.parentElement | ||
| : commonAncestor as Element; | ||
|
|
||
| // block popover in transcript area | ||
| if (container?.closest(".tiptap-transcript")) { | ||
| setSelection(null); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Block popover when selection intersects transcript: check both ends and optionally clear DOM selection
Using only range.commonAncestorContainer can miss cases where the selection spans transcript and non‑transcript nodes (common ancestor outside transcript). Check both startContainer and endContainer. Also consider clearing the DOM selection to avoid immediate re-triggers. Update the inline comment to explain why we block (not what).
Apply this diff:
- const commonAncestor = range.commonAncestorContainer;
- const container = commonAncestor.nodeType === Node.TEXT_NODE
- ? commonAncestor.parentElement
- : commonAncestor as Element;
-
- // block popover in transcript area
- if (container?.closest(".tiptap-transcript")) {
- setSelection(null);
- return;
- }
+ const isInTranscript = (n: Node) => {
+ const el = n.nodeType === Node.TEXT_NODE ? (n as Text).parentElement : (n as Element | null);
+ return el?.closest(".tiptap-transcript");
+ };
+ // Why: avoid popover interfering with transcript interactions
+ if (isInTranscript(range.startContainer) || isInTranscript(range.endContainer)) {
+ sel.removeAllRanges();
+ setSelection(null);
+ return;
+ }📝 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.
| const commonAncestor = range.commonAncestorContainer; | |
| const container = commonAncestor.nodeType === Node.TEXT_NODE | |
| ? commonAncestor.parentElement | |
| : commonAncestor as Element; | |
| // block popover in transcript area | |
| if (container?.closest(".tiptap-transcript")) { | |
| setSelection(null); | |
| return; | |
| } | |
| const isInTranscript = (n: Node) => { | |
| const el = n.nodeType === Node.TEXT_NODE | |
| ? (n as Text).parentElement | |
| : (n as Element | null); | |
| return el?.closest(".tiptap-transcript"); | |
| }; | |
| // Why: avoid popover interfering with transcript interactions | |
| if (isInTranscript(range.startContainer) || isInTranscript(range.endContainer)) { | |
| sel.removeAllRanges(); | |
| setSelection(null); | |
| return; | |
| } |
Summary by cubic
Enable sharing for all notes. Users can share either the enhanced note or the raw note, and all export options use the content the user is viewing.
New Features
Bug Fixes