Skip to content

Conversation

sawka
Copy link
Member

@sawka sawka commented Sep 15, 2025

the primary purpose of this PR is to fix a showstopper bug in the CodeEditor component by setting "path" to a stable UUID. the bug was that it started as empty string, so it created a shared model between all of the codeeditor components. now each will get their own monaco model.

also took this opportunity to do more more preview view refactoring, splitting up code, and more tailwind migrations.

Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

Walkthrough

Refactors preview UI and utilities, introduces new components, and updates props and styles. Directory preview extracts helpers to preview-directory-utils (size/time formatting, MIME cleaning, sort icons, rename/delete RPC handlers) and adds EntryManagerOverlay. ErrorOverlay moves to its own module. CodeEditor drops RPC-based path resolution, simplifies props to readonly, adds a JSON Monaco worker, and uses a stable per-instance editor path. Markdown adds a contentClassName prop and minor TOC adjustments. Multiple views switch to Tailwind classes; preview.scss and directory search SCSS are removed. Streaming preview layout and controls are repositioned. A console.log in markdown-util is commented out.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely highlights the primary change (fixing the CodeEditor/Monaco model bug) and notes the secondary preview refactoring; both points match the changeset and are understandable at a glance by teammates.
Description Check ✅ Passed The description accurately states the PR's main objective (assigning a stable UUID to the CodeEditor path so each instance gets its own Monaco model) and mentions the related preview refactoring and Tailwind migrations, which aligns with the file-level summaries and PR objectives.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/edit-bug

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e281753 and fb6bca9.

📒 Files selected for processing (2)
  • frontend/app/view/preview/entry-manager.tsx (1 hunks)
  • frontend/app/view/preview/preview-directory-utils.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/app/view/preview/preview-directory-utils.tsx
  • frontend/app/view/preview/entry-manager.tsx
⏰ 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: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build for TestDriver.ai

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
frontend/app/element/markdown-util.ts (2)

175-176: Fix return type: function returns null but is typed Promise.

resolveSrcSet returns null on Line 176, violating its Promise<string> signature and causing downstream type mismatches.

Apply this diff:

-export const resolveSrcSet = async (srcSet: string, resolveOpts: MarkdownResolveOpts): Promise<string> => {
-    if (!srcSet) return null;
+export const resolveSrcSet = async (
+    srcSet: string | undefined,
+    resolveOpts: MarkdownResolveOpts
+): Promise<string | undefined> => {
+    if (!srcSet) return undefined;

Follow-ups:

  • Update consumers to accept string | undefined (see comments in markdown.tsx for useState types).

193-201: Filter out unresolved candidates to avoid “null/undefined” literals in srcset.

If a URL can’t be resolved, this reconstructs a candidate as "null 2x" etc.

Apply this diff:

-    return resolvedCandidates
+    return resolvedCandidates
+        .filter((candidate) => !!candidate.url)
         .map((candidate) => {
             let part = candidate.url;
             if (candidate.w) part += ` ${candidate.w}w`;
             if (candidate.h) part += ` ${candidate.h}h`;
             if (candidate.d) part += ` ${candidate.d}x`;
             return part;
         })
         .join(", ");
frontend/app/view/preview/preview-directory.tsx (3)

651-656: Fix non-boolean returns in directoryKeyDownHandler (TypeScript type bug).

Branches return with no value in a function typed to return boolean. This can fail type-checking and cause subtle behavior inconsistencies.

Apply this diff:

@@
-            if (checkKeyPressed(waveEvent, "Escape")) {
-                setSearchText("");
-                return;
-            }
+            if (checkKeyPressed(waveEvent, "Escape")) {
+                setSearchText("");
+                return true;
+            }
@@
-            if (checkKeyPressed(waveEvent, "Enter")) {
-                if (filteredData.length == 0) {
-                    return;
-                }
+            if (checkKeyPressed(waveEvent, "Enter")) {
+                if (filteredData.length == 0) {
+                    return false;
+                }

Also applies to: 673-676


114-126: MIME-type icon fallback walks string one character at a time (likely wrong).

Slicing "text/plain" to "text/plai", "text/pla", … will never match config keys; most will fall back to the generic icon.

Apply this diff to degrade by segments (exact → type/* → type):

   const getIconFromMimeType = useCallback(
     (mimeType: string): string => {
-      while (mimeType.length > 0) {
-        const icon = fullConfig.mimetypes?.[mimeType]?.icon ?? null;
-        if (isIconValid(icon)) {
-          return `fa fa-solid fa-${icon} fa-fw`;
-        }
-        mimeType = mimeType.slice(0, -1);
-      }
+      const tryKeys = [mimeType];
+      if (mimeType.includes("/")) {
+        const [type] = mimeType.split("/");
+        tryKeys.push(`${type}/*`, type);
+      }
+      for (const key of tryKeys) {
+        const icon = fullConfig.mimetypes?.[key]?.icon ?? null;
+        if (isIconValid(icon)) return `fa fa-solid fa-${icon} fa-fw`;
+      }
       return "fa fa-solid fa-file fa-fw";
     },
     [fullConfig.mimetypes]
   );

810-830: Create/mkdir flows lack error handling and leak console logs.

If RPC fails (exists/permissions), the UI silently fails; debugging logs remain in prod.

Apply this diff:

   const newFile = useCallback(() => {
     setEntryManagerProps({
       entryManagerType: EntryManagerType.NewFile,
       onSave: (newName: string) => {
-        console.log(`newFile: ${newName}`);
-        fireAndForget(async () => {
-          await RpcApi.FileCreateCommand(
-            TabRpcClient,
-            {
-              info: {
-                path: await model.formatRemoteUri(`${dirPath}/${newName}`, globalStore.get),
-              },
-            },
-            null
-          );
-          model.refreshCallback();
-        });
+        fireAndForget(async () => {
+          try {
+            if (!newName?.trim()) throw new Error("File name cannot be empty");
+            await RpcApi.FileCreateCommand(
+              TabRpcClient,
+              { info: { path: await model.formatRemoteUri(`${dirPath}/${newName}`, globalStore.get) } },
+              null
+            );
+          } catch (e) {
+            getApi().setErrorMsg?.({ status: "Create Failed", text: `${e}`, level: "error" });
+          }
+          model.refreshCallback();
+        });
         setEntryManagerProps(undefined);
       },
     });
   }, [dirPath]);
   const newDirectory = useCallback(() => {
     setEntryManagerProps({
       entryManagerType: EntryManagerType.NewDirectory,
       onSave: (newName: string) => {
-        console.log(`newDirectory: ${newName}`);
-        fireAndForget(async () => {
-          await RpcApi.FileMkdirCommand(TabRpcClient, {
-            info: {
-              path: await model.formatRemoteUri(`${dirPath}/${newName}`, globalStore.get),
-            },
-          });
-          model.refreshCallback();
-        });
+        fireAndForget(async () => {
+          try {
+            if (!newName?.trim()) throw new Error("Folder name cannot be empty");
+            await RpcApi.FileMkdirCommand(TabRpcClient, {
+              info: { path: await model.formatRemoteUri(`${dirPath}/${newName}`, globalStore.get) },
+            });
+          } catch (e) {
+            getApi().setErrorMsg?.({ status: "Mkdir Failed", text: `${e}`, level: "error" });
+          }
+          model.refreshCallback();
+        });
         setEntryManagerProps(undefined);
       },
     });
   }, [dirPath]);

Also applies to: 832-848

🧹 Nitpick comments (23)
frontend/app/element/markdown-util.ts (2)

165-165: Remove commented-out debug line or gate it behind a debug flag.

Avoid leaving commented code. If needed, prefer a gated console.debug (or central logger).

Apply this diff:

-        // console.log("markdown resolve", resolveOpts, filepath, "=>", baseDirUri, remoteUri);
+        // if (import.meta?.env?.DEV) console.debug("markdown resolve", { resolveOpts, filepath, baseDirUri, remoteUri });

158-160: Treat empty local paths as “no path” for consistency.

Returning "" (empty string) propagates a falsy-but-string value; downstream typically expects “absent” as null/undefined.

Apply this diff:

-    if (!filepath || filepath.startsWith("http://") || filepath.startsWith("https://")) {
-        return filepath;
-    }
+    if (!filepath) return null;
+    if (filepath.startsWith("http://") || filepath.startsWith("https://")) return filepath;
frontend/app/element/markdown.tsx (2)

499-502: Give the TOC semantic navigation and labels.

Expose landmark/label for a11y.

Apply this diff:

-            {toc && (
-                <OverlayScrollbarsComponent className="toc mt-1" options={{ scrollbars: { autoHide: "leave" } }}>
+            {toc && (
+                <OverlayScrollbarsComponent
+                    className="toc mt-1"
+                    role="navigation"
+                    aria-label="Table of contents"
+                    options={{ scrollbars: { autoHide: "leave" } }}
+                >
                     <div className="toc-inner">
                         <h4 className="font-bold">Table of Contents</h4>
                         {toc}
                     </div>
                 </OverlayScrollbarsComponent>
             )}

194-206: Align types with updated resolveSrcSet return and optional DOM attrs.

useState<string>(props.srcSet) conflicts with possibly-undefined srcSet and the proposed Promise<string | undefined> return. Use string | undefined.

Apply this diff:

-    const [resolvedSrcSet, setResolvedSrcSet] = useState<string>(props.srcSet);
+    const [resolvedSrcSet, setResolvedSrcSet] = useState<string | undefined>(props.srcSet);
@@
-        const resolvePath = async () => {
-            const resolved = await resolveSrcSet(props.srcSet, resolveOpts);
+        const resolvePath = async () => {
+            const resolved = await resolveSrcSet(props.srcSet, resolveOpts);
             setResolvedSrcSet(resolved);
             setResolving(false);
         };
@@
-    return <source srcSet={resolvedSrcSet} media={props.media} />;
+    return <source srcSet={resolvedSrcSet} media={props.media} />;

Optional: apply the same string | undefined adjustment to MarkdownImg’s resolvedSrcSet state for consistency.

Also applies to: 211-212

frontend/app/view/codeeditor/codeeditor.tsx (1)

123-123: Make path a proper Monaco URI to avoid edge cases.

Some Monaco internals expect a URI with scheme and path. A bare UUID parses to a scheme-only URI; use an inmemory URI to be safe.

Apply:

-    const editorPath = useRef(crypto.randomUUID()).current;
+    const editorPath = useRef(`inmemory://wave/${crypto.randomUUID()}`).current;

Optional: derive a stable path per block to reuse model/view state across remounts (if desired):

-    const editorPath = useRef(`inmemory://wave/${crypto.randomUUID()}`).current;
+    const editorPath = useRef(`inmemory://wave/${blockId}`).current;

Also applies to: 165-165

frontend/app/view/preview/preview-edit.tsx (1)

64-71: Guard against null/undefined fileInfo when passing readonly.

Avoid a potential undefined→boolean mismatch during initial load.

Apply:

-            readonly={fileInfo.readonly}
+            readonly={!!fileInfo?.readonly}
frontend/app/view/preview/preview-streaming.tsx (4)

16-26: Add accessible labels to zoom buttons.

Icons alone aren’t announced by screen readers; add aria-labels.

-            <Button onClick={() => zoomIn()} title="Zoom In" className="py-1 px-[5px]">
+            <Button onClick={() => zoomIn()} title="Zoom In" aria-label="Zoom In" className="py-1 px-[5px]">
...
-            <Button onClick={() => zoomOut()} title="Zoom Out" className="py-1 px-[5px]">
+            <Button onClick={() => zoomOut()} title="Zoom Out" aria-label="Zoom Out" className="py-1 px-[5px]">
...
-            <Button onClick={() => resetTransform()} title="Reset Zoom" className="py-1 px-[5px]">
+            <Button onClick={() => resetTransform()} title="Reset Zoom" aria-label="Reset Zoom" className="py-1 px-[5px]">

37-39: Provide alt text and minor perf hints on image.

Improves a11y and UX while keeping overlay layering intact.

-                            <img src={url} className="z-[1]" />
+                            <img src={url} alt="Image preview" loading="lazy" decoding="async" draggable={false} className="z-[1]" />

60-62: Add a title to the PDF iframe for accessibility.

Screen readers need a descriptive title.

-                <iframe src={streamingUrl} width="100%" height="100%" name="pdfview" />
+                <iframe src={streamingUrl} width="100%" height="100%" name="pdfview" title="PDF preview" />

67-69: Reduce bandwidth on media by hinting preload.

Preloading metadata is usually sufficient for previews.

-                <video controls className="w-full h-full p-[10px] object-contain">
+                <video controls preload="metadata" className="w-full h-full p-[10px] object-contain">
...
-                <audio controls className="w-full h-full p-[10px] object-contain">
+                <audio controls preload="metadata" className="w-full h-full p-[10px] object-contain">

Also applies to: 76-78

frontend/app/view/preview/preview-error-overlay.tsx (3)

15-17: Use strict equality.

Avoid loose equality for predictable semantics.

-    if (errorMsg.level == "warning") {
+    if (errorMsg.level === "warning") {

19-22: Handle clipboard copy failures.

Clipboard API can throw (permissions/HTTP). Guard and no-op gracefully.

-    const handleCopyToClipboard = useCallback(async () => {
-        await navigator.clipboard.writeText(errorMsg.text);
-    }, [errorMsg.text]);
+    const handleCopyToClipboard = useCallback(async () => {
+        try {
+            await navigator.clipboard.writeText(errorMsg.text);
+        } catch (err) {
+            console.warn("Copy failed:", err);
+        }
+    }, [errorMsg.text]);

51-62: Use stable keys instead of randomUUID.

Random keys force remounts every render; use index or a stable identifier.

-                                {errorMsg.buttons?.map((buttonDef) => (
+                                {errorMsg.buttons?.map((buttonDef, idx) => (
                                     <Button
                                         className={buttonClassName}
                                         onClick={() => {
                                             buttonDef.onClick();
                                             resetOverlay();
                                         }}
-                                        key={crypto.randomUUID()}
+                                        key={buttonDef.key ?? buttonDef.text ?? String(idx)}
                                     >
frontend/app/view/preview/preview-directory-utils.tsx (2)

67-74: Tighten icon validation.

Prefer RegExp.test and normalize to lower-case to match your regex.

-export function isIconValid(icon: string): boolean {
-    if (isBlank(icon)) {
-        return false;
-    }
-    return icon.match(iconRegex) != null;
-}
+export function isIconValid(icon: string): boolean {
+    if (isBlank(icon)) return false;
+    return iconRegex.test(icon.toLowerCase());
+}

117-136: Add prompts for overwrite/merge errors (constants already exported).

Rename/delete currently only handle recursive confirmation. Consider handling overwrite/merge to guide users.

If these errors can be returned by FileMoveCommand/FileDeleteCommand, I can draft the prompt wiring. Confirm the exact error substrings emitted by the RPC so we can match reliably.

Also applies to: 158-176

frontend/app/view/preview/entry-manager.tsx (1)

34-43: Default empty input value and make Cancel safe.

Avoid passing undefined to Input and Button.

-        const [value, setValue] = useState(startingValue);
+        const [value, setValue] = useState(startingValue ?? "");
...
-                    <Button className="vertical-padding-4 red outlined" onClick={onCancel}>
+                    <Button className="vertical-padding-4 red outlined" onClick={() => onCancel?.()}>
frontend/app/view/preview/preview-directory.tsx (7)

636-647: Make search consistently case-insensitive.

When building searchText via key events, casing may differ; ensure comparison lowercases both sides.

Apply this diff:

-                return fileInfo.name.toLowerCase().includes(searchText);
+                return fileInfo.name.toLowerCase().includes(searchText.toLowerCase());

354-385: Scroll-into-view effect misses osRef in deps and osRef can be null.

  • Effect won’t rerun when the scrollbar ref becomes available.
  • Prop type should allow null; you’re passing osRef.current which can be null.

Apply this diff:

@@
 interface TableBodyProps {
@@
-    osRef: OverlayScrollbarsComponentRef;
+    osRef: OverlayScrollbarsComponentRef | null;
 }
@@
-    useEffect(() => {
+    useEffect(() => {
         if (focusIndex === null || !bodyRef.current || !osRef) {
             return;
         }
@@
-    }, [focusIndex]);
+    }, [focusIndex, osRef]);

No behavioral change needed in the caller, you’re already passing osRef={osRef.current}.

Also applies to: 335-336, 318-319


53-73: Header cell: remove redundant key and add a11y (keyboard + aria-sort).

  • The child sets key while the parent already keys it.
  • Add role, keyboard support, and aria-sort for screen readers.

Apply this diff:

 function DirectoryTableHeaderCell({ header }: DirectoryTableHeaderCellProps) {
   return (
-        <div
+        <div
           className="dir-table-head-cell"
-          key={header.id}
           style={{ width: `calc(var(--header-${header.id}-size) * 1px)` }}
         >
-          <div className="dir-table-head-cell-content" onClick={() => header.column.toggleSorting()}>
+          <div
+            className="dir-table-head-cell-content"
+            role="button"
+            tabIndex={0}
+            aria-sort={
+              header.column.getIsSorted() === "asc"
+                ? "ascending"
+                : header.column.getIsSorted() === "desc"
+                ? "descending"
+                : "none"
+            }
+            onClick={() => header.column.toggleSorting()}
+            onKeyDown={(e) => {
+              if (e.key === "Enter" || e.key === " ") header.column.toggleSorting();
+            }}
+          >
             {header.isPlaceholder ? null : flexRender(header.column.columnDef.header, header.getContext())}
             {getSortIcon(header.column.getIsSorted())}
           </div>

Also applies to: 301-303


199-202: Remove debug log from rename flow.

Avoid noisy logs; UI already surfaces errors via setErrorMsg.

Apply this diff:

-                        console.log(`replacing ${fileName} with ${newName}: ${path}`);

451-466: Hidden input for search banner appears unnecessary.

The hidden input won’t capture user input; onChangeCapture at the container won’t fire meaningfully here. Consider removing the input or wiring an actual search field.

Would you like me to propose a small inline search input that syncs with searchText?


280-285: Null-guard osRef usage in onScroll.

Small safety net to avoid rare NPEs on unmount.

Apply this diff:

   const onScroll = useCallback(
     debounce(2, () => {
-      setScrollHeight(osRef.current.osInstance().elements().viewport.scrollTop);
+      const viewport = osRef.current?.osInstance()?.elements()?.viewport;
+      if (viewport) setScrollHeight(viewport.scrollTop);
     }),
     []
   );

189-208: Guard empty filename in updateName.

If path unexpectedly ends with “/” or is malformed, fileName can be undefined.

Apply this diff:

-        (path: string, isDir: boolean) => {
-            const fileName = path.split("/").at(-1);
+        (path: string, isDir: boolean) => {
+            const fileName = path.split("/").filter(Boolean).at(-1) ?? "";
+            if (!fileName) return;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50cc08a and e281753.

📒 Files selected for processing (13)
  • frontend/app/element/markdown-util.ts (1 hunks)
  • frontend/app/element/markdown.tsx (7 hunks)
  • frontend/app/view/codeeditor/codeeditor.tsx (4 hunks)
  • frontend/app/view/preview/directorypreview.scss (0 hunks)
  • frontend/app/view/preview/entry-manager.tsx (1 hunks)
  • frontend/app/view/preview/preview-directory-utils.tsx (1 hunks)
  • frontend/app/view/preview/preview-directory.tsx (7 hunks)
  • frontend/app/view/preview/preview-edit.tsx (1 hunks)
  • frontend/app/view/preview/preview-error-overlay.tsx (1 hunks)
  • frontend/app/view/preview/preview-markdown.tsx (1 hunks)
  • frontend/app/view/preview/preview-streaming.tsx (3 hunks)
  • frontend/app/view/preview/preview.scss (0 hunks)
  • frontend/app/view/preview/preview.tsx (2 hunks)
💤 Files with no reviewable changes (2)
  • frontend/app/view/preview/preview.scss
  • frontend/app/view/preview/directorypreview.scss
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/app/view/preview/preview-directory-utils.tsx (2)
pkg/wshrpc/wshrpctypes.go (1)
  • FileInfo (412-427)
frontend/util/util.ts (2)
  • isBlank (415-415)
  • fireAndForget (411-411)
frontend/app/element/markdown.tsx (1)
frontend/util/util.ts (1)
  • cn (407-407)
frontend/app/view/preview/preview.tsx (1)
frontend/app/view/preview/preview-error-overlay.tsx (1)
  • ErrorOverlay (10-84)
frontend/app/view/codeeditor/codeeditor.tsx (2)
frontend/app/store/global.ts (1)
  • useOverrideConfigAtom (807-807)
frontend/util/util.ts (1)
  • boundNumber (406-406)
frontend/app/view/preview/preview-directory.tsx (2)
pkg/wshrpc/wshrpctypes.go (1)
  • FileInfo (412-427)
frontend/app/view/preview/preview-directory-utils.tsx (3)
  • getSortIcon (76-85)
  • handleRename (92-139)
  • handleFileDelete (141-180)
⏰ 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: Build for TestDriver.ai
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
frontend/app/view/preview/preview-markdown.tsx (2)

22-22: Verify no nested scrollbars/regressions after wrapper class change.

Parent overflow-auto + child OverlayScrollbars can double-scroll. Also confirm removing .view-preview{,-markdown} doesn’t break legacy CSS hooks.


29-29: LGTM: forwarding contentClassName enables inner padding without leaking layout concerns.

frontend/app/element/markdown.tsx (3)

13-13: LGTM: local cn import aligns with util helpers and Tailwind merging.


300-301: LGTM: new contentClassName prop and destructure look correct.

Prop is later applied in both scrollable and non-scrollable paths.

Also applies to: 315-316


460-462: LGTM: applying contentClassName via cn in both scrollable and non-scrollable variants.

Also applies to: 476-477

frontend/app/view/codeeditor/codeeditor.tsx (3)

11-11: LGTM on the utility import.

boundNumber usage below is consistent with its semantics.


147-155: Good: readOnly wired into memoized editor options.

This correctly reflects readonly changes without recreating the editor.


106-113: No stale props — CodeEditor call sites updated to readonly.
Verified CodeEditorProps (frontend/app/view/codeeditor/codeeditor.tsx) uses readonly and editor options set accordingly; preview-edit.tsx passes readonly; no occurrences found of filename/fileinfo/meta being passed to .

frontend/app/view/preview/preview.tsx (1)

148-151: LGTM: externalized ErrorOverlay and simplified layout.

Cleaner separation of concerns; wrapper/content classes look correct.

Comment on lines +388 to 413
if (showToc) {
if (tocRef.current.length > 0) {
return tocRef.current.map((item) => {
return (
<a
key={item.href}
className="toc-item"
style={{ "--indent-factor": item.depth } as React.CSSProperties}
onClick={() => setFocusedHeading(item.href)}
>
{item.value}
</a>
);
});
} else {
return (
<a
key={item.href}
className="toc-item"
style={{ "--indent-factor": item.depth } as React.CSSProperties}
onClick={() => setFocusedHeading(item.href)}
<div
className="toc-item toc-empty text-secondary"
style={{ "--indent-factor": 2 } as React.CSSProperties}
>
{item.value}
</a>
No sub-headings found
</div>
);
});
}
}
}, [showToc, tocRef]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add href to TOC anchors and re-compute TOC when content changes.

Anchors lack href (accessibility/keyboard issue). TOC memo depends only on showToc, so it can go stale when text changes.

Apply this diff:

-    const toc = useMemo(() => {
-        if (showToc) {
-            if (tocRef.current.length > 0) {
-                return tocRef.current.map((item) => {
-                    return (
-                        <a
-                            key={item.href}
-                            className="toc-item"
-                            style={{ "--indent-factor": item.depth } as React.CSSProperties}
-                            onClick={() => setFocusedHeading(item.href)}
-                        >
-                            {item.value}
-                        </a>
-                    );
-                });
-            } else {
+    const toc = useMemo(() => {
+        if (showToc) {
+            if (tocRef.current.length > 0) {
+                return tocRef.current.map((item) => (
+                    <a
+                        key={item.href}
+                        href={item.href}
+                        className="toc-item"
+                        style={{ "--indent-factor": item.depth } as React.CSSProperties}
+                        onClick={(e) => {
+                            e.preventDefault();
+                            setFocusedHeading(item.href);
+                        }}
+                    >
+                        {item.value}
+                    </a>
+                ));
+            } else {
                 return (
                     <div
                         className="toc-item toc-empty text-secondary"
                         style={{ "--indent-factor": 2 } as React.CSSProperties}
                     >
                         No sub-headings found
                     </div>
                 );
-            }
-        }
-    }, [showToc, tocRef]);
+            }
+        }
+    }, [showToc, transformedText]);
📝 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.

Suggested change
if (showToc) {
if (tocRef.current.length > 0) {
return tocRef.current.map((item) => {
return (
<a
key={item.href}
className="toc-item"
style={{ "--indent-factor": item.depth } as React.CSSProperties}
onClick={() => setFocusedHeading(item.href)}
>
{item.value}
</a>
);
});
} else {
return (
<a
key={item.href}
className="toc-item"
style={{ "--indent-factor": item.depth } as React.CSSProperties}
onClick={() => setFocusedHeading(item.href)}
<div
className="toc-item toc-empty text-secondary"
style={{ "--indent-factor": 2 } as React.CSSProperties}
>
{item.value}
</a>
No sub-headings found
</div>
);
});
}
}
}, [showToc, tocRef]);
const toc = useMemo(() => {
if (showToc) {
if (tocRef.current.length > 0) {
return tocRef.current.map((item) => (
<a
key={item.href}
href={item.href}
className="toc-item"
style={{ "--indent-factor": item.depth } as React.CSSProperties}
onClick={(e) => {
e.preventDefault();
setFocusedHeading(item.href);
}}
>
{item.value}
</a>
));
} else {
return (
<div
className="toc-item toc-empty text-secondary"
style={{ "--indent-factor": 2 } as React.CSSProperties}
>
No sub-headings found
</div>
);
}
}
}, [showToc, transformedText]);

@sawka sawka merged commit 77bbf74 into main Sep 15, 2025
7 of 8 checks passed
@sawka sawka deleted the sawka/edit-bug branch September 15, 2025 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant