diff --git a/CHANGELOG.md b/CHANGELOG.md index 80aa11f2..f5174863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `@rwdocs/core` exposes `RwSite.listSections()`, which returns every documentation section in one call — flat, each with its canonical ref (`kind:namespace/name`), scope path, and full nearest-first ancestry (root last) — so a host no longer needs N+1 `getNavigation()` calls to walk nested sections (which deliberately hide sub-sections as childless leaves). +### Changed + +- Opening an inline-comment deep link (`#comment-`), or stepping to an inline comment with `n`/`p`, now lands the highlighted passage about a third of the way down the viewport instead of dead-center — so the passage sits where the eye rests on arrival, with room above for the comment thread. + ### Fixed - Relative `.md` links from a leaf page (e.g. `[sibling](./other.md)` in `docs/specs/notif.md`) now resolve to the sibling page (`/specs/other`) instead of a non-existent path nested under the current page (`/specs/notif/other`). Links now follow standard CommonMark semantics — resolved relative to the source file's directory — for both leaf pages and `index.md` directory pages. Links from README/`index.md` homepages (including the `docs/` source-dir prefix case) are unchanged. diff --git a/packages/viewer/e2e/comment-deeplink.spec.ts b/packages/viewer/e2e/comment-deeplink.spec.ts index bda3a448..7df46ed9 100644 --- a/packages/viewer/e2e/comment-deeplink.spec.ts +++ b/packages/viewer/e2e/comment-deeplink.spec.ts @@ -116,9 +116,22 @@ test("deep-link to an inline comment opens the sidebar and highlights it", async const active = page.locator('article rw-annotation[data-active="true"]'); await expect(active.first()).toBeVisible(); await expect(active.first()).toBeInViewport(); - // The passage is centered (not jammed to the top), so the pinned sidebar - // thread — whose header anchors to the highlight — is not clipped above the - // viewport. Its top must sit at or below 0. + + // The passage lands ~⅓ down the viewport (scroll-margin-top: 33vh + + // block:"start"), not centered and not jammed to the top. This leaves room + // above for the pinned sidebar thread and matches where the eye rests when + // arriving via a deeplink. The getting-started fixture is intentionally tall + // enough (its "Tips" section) that the anchor has real scroll room below it, + // so 33vh genuinely engages — without the scroll-margin, block:"start" would + // land the highlight at top≈0 and fail the lower bound below. + const { top, vh } = await active.first().evaluate((el) => ({ + top: el.getBoundingClientRect().top, + vh: window.innerHeight, + })); + expect(top).toBeGreaterThan(vh * 0.2); + expect(top).toBeLessThan(vh * 0.45); + + // The pinned sidebar thread header is still not clipped above the viewport. const cardTop = await sidebar .getByTestId("comment-thread") .first() diff --git a/packages/viewer/e2e/comment-navigation.spec.ts b/packages/viewer/e2e/comment-navigation.spec.ts index 94773c8d..67069106 100644 --- a/packages/viewer/e2e/comment-navigation.spec.ts +++ b/packages/viewer/e2e/comment-navigation.spec.ts @@ -200,8 +200,18 @@ test.describe("Comment keyboard navigation", () => { await expect(page.getByRole("complementary", { name: "Comments" })).toBeVisible(); expect(await activeHighlightId(page)).not.toBeNull(); await expect(liveRegion(page)).toContainText("Comment 1 of 1"); - // The active highlight is scrolled into view (centered) on the jump. - await expect(page.locator("article rw-annotation[data-active='true']")).toBeInViewport(); + // The active highlight is scrolled into the upper region on the jump + // (scroll-margin-top: 33vh + block:"start"), no longer centered. This page's + // anchor sits high enough that, scrolled to the top, it is already above the + // 33vh mark — scroll-margin-top can't push it down past the document top — so + // here we only assert it is clearly above center (the old block:"center" + // landed it at ~50vh). The exact ⅓-down landing is pinned by the deeplink + // spec, whose anchor has room to scroll. + const active = page.locator("article rw-annotation[data-active='true']").first(); + await expect(active).toBeInViewport(); + const top = await active.evaluate((el) => el.getBoundingClientRect().top); + const vh = await page.evaluate(() => window.innerHeight); + expect(top).toBeLessThan(vh * 0.45); }); test("n steps through inline then page comments and wraps", async ({ page }) => { diff --git a/packages/viewer/e2e/fixtures/docs/getting-started/index.md b/packages/viewer/e2e/fixtures/docs/getting-started/index.md index b913b95d..10cfbd09 100644 --- a/packages/viewer/e2e/fixtures/docs/getting-started/index.md +++ b/packages/viewer/e2e/fixtures/docs/getting-started/index.md @@ -14,3 +14,19 @@ Follow these steps to begin: - [Installation Guide](./installation.md) - How to install - [Configuration Guide](./configuration.md) - How to configure + +## Tips + +A few practices that tend to save time once your setup grows beyond the basics. + +Keep your configuration under version control so changes are reviewable. Small, +focused commits make it easier to trace when a setting changed and why. + +Prefer environment variables for secrets and per-machine values, and commit +only the defaults that are safe to share across every environment. + +When something behaves unexpectedly, re-run with verbose logging before +changing configuration — the logs usually point straight at the cause. + +Document any non-obvious setting next to where it is defined, so the next +reader does not have to reverse-engineer its purpose from behavior alone. diff --git a/packages/viewer/src/components/PageContent.svelte b/packages/viewer/src/components/PageContent.svelte index a92d0d7a..ca6cb8b9 100644 --- a/packages/viewer/src/components/PageContent.svelte +++ b/packages/viewer/src/components/PageContent.svelte @@ -423,6 +423,9 @@ if (!activeId || !articleRef) return null; return articleRef.querySelector(`rw-annotation[data-comment-id="${escapeId(activeId)}"]`); }, + // block:"start" so the highlight lands ~⅓ down via scroll-margin-top: 33vh + // (see content.css) — same position as the deeplink reveal. + "start", ); // Pending-selection overlay — paint the user's in-progress text selection @@ -549,9 +552,11 @@ if (kind === "inline") { const el = articleRef?.querySelector(`rw-annotation[data-comment-id="${escapeId(id)}"]`); if (!el) return false; - // Center (not start) — matches keyboard nav, and leaves room above the - // passage for the sidebar thread, which pins its header to the highlight. - el.scrollIntoView({ behavior: "auto", block: "center" }); + // block:"start" + `rw-annotation { scroll-margin-top: 33vh }` lands the + // passage ~⅓ down the viewport (where the eye rests on a deeplink), not + // centered — and leaves room above for the pinned sidebar thread. Matches + // keyboard nav. CSS offset (not scroll math) keeps it embedded-safe. + el.scrollIntoView({ behavior: "auto", block: "start" }); return true; } // page / resolved: the timeline thread wrapper carries id="comment-". diff --git a/packages/viewer/src/styles/content.css b/packages/viewer/src/styles/content.css index e241000c..d0e239c1 100644 --- a/packages/viewer/src/styles/content.css +++ b/packages/viewer/src/styles/content.css @@ -387,6 +387,14 @@ rw-annotation { background-size: 100% 2px; -webkit-box-decoration-break: clone; box-decoration-break: clone; + /* When a comment is revealed (deeplink reveal or n/p keyboard nav, both via + * scrollIntoView block:"start" in PageContent.svelte), land the highlight + * ~⅓ down the viewport instead of at the very top — that is where the eye + * rests on arrival. A CSS offset (not manual scroll math) is used so it works + * whether the window or an embedding host owns the scroll container. 33vh sits + * well below the ~49px sticky mobile header, so it clears it without reusing + * --scroll-anchor-offset (which carries header-clearance semantics). */ + scroll-margin-top: 33vh; } rw-annotation[data-strategy="fuzzy"] { /* Dashed equivalent: a horizontal repeating gradient at the box bottom. */