fix: skip hidden resolved comments during navigation (#322)

Author: tomasz-tomczyk

assets/js/document-renderer.js

@@ -4382,35 +4382,37 @@ function getInlineCommentCards(ctx) {
 }
 
 function navigateToComment(ctx, direction) {
-  const cards = getInlineCommentCards(ctx)
+  const allCards = getInlineCommentCards(ctx)
+  const isEligible = card => !isHideResolved(ctx) || !card.classList.contains('resolved-card')
+  const cards = allCards.filter(isEligible)
   if (cards.length === 0) return
 
   const header = document.querySelector('.crit-header')
   const headerHeight = header ? header.offsetHeight : 52
 
-  // Find current position by stored comment ID (immune to smooth-scroll race conditions)
+  // Keep the current position in the full order even when that card becomes
+  // hidden, then scan in the requested direction for the next eligible card.
   let idx = ctx._navCommentId
-    ? cards.findIndex(c => c.dataset.commentId === ctx._navCommentId)
+    ? allCards.findIndex(c => c.dataset.commentId === ctx._navCommentId)
     : -1
 
-  let targetIdx
-  if (direction === 1) {
-    if (idx < 0) {
-      // First use: pick first card below the header area by viewport position
-      const firstBelow = cards.findIndex(c => c.getBoundingClientRect().top > headerHeight + 8)
-      targetIdx = firstBelow >= 0 ? firstBelow : 0
-    } else {
-      targetIdx = idx >= cards.length - 1 ? 0 : idx + 1
+  let target
+  if (idx >= 0) {
+    for (let step = 1; step <= allCards.length; step++) {
+      const candidateIdx = (idx + direction * step + allCards.length) % allCards.length
+      if (isEligible(allCards[candidateIdx])) {
+        target = allCards[candidateIdx]
+        break
+      }
     }
+  } else if (direction === 1) {
+    // First use: pick first card below the header area by viewport position
+    const firstBelow = cards.findIndex(c => c.getBoundingClientRect().top > headerHeight + 8)
+    target = cards[firstBelow >= 0 ? firstBelow : 0]
   } else {
-    if (idx < 0) {
-      targetIdx = cards.length - 1
-    } else {
-      targetIdx = idx <= 0 ? cards.length - 1 : idx - 1
-    }
+    target = cards[cards.length - 1]
   }
 
-  const target = cards[targetIdx]
   ctx._navCommentId = target.dataset.commentId
 
   const rect = target.getBoundingClientRect()
@@ -4894,6 +4896,7 @@ export const DocumentRenderer = {
         const newEl = createCommentElement(comment, ctx)
         block.replaceWith(newEl)
       }
+      applyHideResolved(ctx)
       updateCommentCount(ctx)
       updateTreeBadge(ctx, comment.file_path)
       rerenderPanel(ctx)

e2e/hide-resolved.spec.ts

@@ -4,6 +4,7 @@ import {
   deleteReview,
   loadReview,
   addCommentViaUI,
+  seedComment,
 } from "./helpers";
 
 /**
@@ -129,6 +130,73 @@ test.describe("Hide Resolved", () => {
     await expect(resolvedBlock).toBeVisible();
   });
 
+  test("comment arrows skip hidden resolved comments in both directions", async ({
+    page,
+    request,
+  }) => {
+    await seedComment(request, token, {
+      body: "Open A",
+      startLine: 1,
+    });
+    await seedComment(request, token, {
+      body: "Open C",
+      startLine: 6,
+    });
+    await loadReview(page, token);
+    await addAndResolveComment(page, "Resolved B", {
+      lineIndex: 1,
+    });
+
+    const openA = page
+      .locator(".comment-card:not(.resolved-card)")
+      .filter({ hasText: "Open A" });
+    const resolvedCard = page
+      .locator(".comment-card.resolved-card")
+      .filter({ hasText: "Resolved B" });
+    const openC = page
+      .locator(".comment-card:not(.resolved-card)")
+      .filter({ hasText: "Open C" });
+
+    await page.locator("#comment-nav-next").click();
+    await expect(openA).toHaveClass(/comment-nav-highlight/);
+    await page.locator("#comment-nav-next").click();
+    await expect(resolvedCard).toHaveClass(/comment-nav-highlight/);
+
+    await page.keyboard.press("h");
+    await expect(resolvedCard).not.toBeVisible();
+    await page.locator("#comment-nav-prev").click();
+    await expect(openA).toHaveClass(/comment-nav-highlight/);
+    await expect(openC).not.toHaveClass(/comment-nav-highlight/);
+    await expect(resolvedCard).not.toHaveClass(/comment-nav-highlight/);
+
+    await page.keyboard.press("h");
+    await page.locator("#comment-nav-next").click();
+    await expect(resolvedCard).toHaveClass(/comment-nav-highlight/);
+
+    await page.keyboard.press("h");
+    await page.locator("#comment-nav-next").click();
+    await expect(openC).toHaveClass(/comment-nav-highlight/);
+    await expect(resolvedCard).not.toHaveClass(/comment-nav-highlight/);
+  });
+
+  test("resolving a comment while hide resolved is enabled hides it", async ({
+    page,
+  }) => {
+    await loadReview(page, token);
+    await addCommentViaUI(page, "Resolve while hidden");
+    await page.keyboard.press("h");
+
+    const card = page
+      .locator(".comment-card")
+      .filter({ hasText: "Resolve while hidden" });
+    await card.locator(".resolve-btn").click();
+
+    const resolvedBlock = page
+      .locator(".comment-block:not(.panel-comment-block)")
+      .filter({ has: page.locator(".resolved-card") });
+    await expect(resolvedBlock).not.toBeVisible();
+  });
+
   test("persists via localStorage across reload", async ({ page }) => {
     await loadReview(page, token);