Skip to content

Commit 191aae4

Browse files
docs: address review on the shared Plane theme
- PlaneHeader: remember and restore the scroll offset around the mobile menu's body scroll lock. `position: fixed` with `top: 0` dropped the document scroll, so opening the menu part-way down a page and closing it sent the reader back to the top. - PlaneHeader: type the nav off `DefaultTheme.Config` and narrow the `NavItem` union properly (string-link guard, dropdown predicate). This fixes 11 real type errors the previous check:types never looked at. - CookieConsent: both sites boot PostHog with `persistence: "memory"`, but granting consent only called `opt_in_capturing()`, so a consenting visitor still got a fresh distinct_id on every page load. Lift persistence on Accept, force it back on Decline, and revoke GA consent explicitly on Decline. - theme/index.ts: remove the leaked `hashchange` listener on unmount and drop the duplicate synthetic click in the tab-hash handler. - check-theme-sync: distinguish "sibling has not adopted plane/ yet" from "sibling unreachable". The master fallback 404s until both PRs land, which would have turned CI red on unrelated PRs in between. - check:types: run vue-tsc through a shared wrapper. Plain `tsc` cannot parse `.vue`, so the new gate silently skipped every component; it now checks them and ignores only the vendored VoidZero sources. Claude-Session: https://claude.ai/code/session_01JGiwdDajm1vYYNBfQMr44f
1 parent dea6a4f commit 191aae4

9 files changed

Lines changed: 244 additions & 51 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pnpm check:types # Type-check the VitePress config and theme
2121
pnpm check:theme-sync # Verify docs/.vitepress/theme/plane/ is identical to makeplane/docs (THEME_SIBLING_PATH=../docs for a local checkout)
2222
```
2323

24-
**CI checks on PRs** (to `master`): Prettier formatting + VitePress build must pass.
24+
**CI checks on PRs** (to `master`): Prettier formatting, type-check, VitePress build, and the shared-theme sync check must pass.
2525

2626
## Architecture
2727

docs/.vitepress/theme/plane/components/CookieConsent.vue

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ declare global {
77
posthog?: {
88
opt_in_capturing?: () => void;
99
opt_out_capturing?: () => void;
10+
set_config?: (config: Record<string, unknown>) => void;
1011
};
1112
}
1213
}
@@ -32,21 +33,28 @@ function grantConsent() {
3233
});
3334
}
3435
35-
// PostHog (may not be loaded if VITE_POSTHOG_KEY is unset)
36-
if (window.posthog?.opt_in_capturing) {
37-
window.posthog.opt_in_capturing();
38-
}
36+
// PostHog (may not be loaded if VITE_POSTHOG_KEY is unset).
37+
// Both sites boot PostHog with `persistence: "memory"` so nothing is stored
38+
// pre-consent; without lifting that here a consenting visitor still gets a new
39+
// distinct_id on every page load, so returning users never stitch together.
40+
window.posthog?.set_config?.({ persistence: "localStorage+cookie" });
41+
window.posthog?.opt_in_capturing?.();
3942
}
4043
4144
function denyConsent() {
4245
if (typeof window === "undefined") return;
4346
44-
// Google Analytics — consent stays denied by default, no update needed
47+
// Google Analytics — consent stays denied by default, but be explicit so a
48+
// later "change cookie settings" entry point can revoke a previous grant.
49+
if (typeof window.gtag === "function") {
50+
window.gtag("consent", "update", {
51+
analytics_storage: "denied",
52+
});
53+
}
4554
4655
// PostHog (may not be loaded if VITE_POSTHOG_KEY is unset)
47-
if (window.posthog?.opt_out_capturing) {
48-
window.posthog.opt_out_capturing();
49-
}
56+
window.posthog?.set_config?.({ persistence: "memory" });
57+
window.posthog?.opt_out_capturing?.();
5058
}
5159
5260
function accept() {

docs/.vitepress/theme/plane/components/PlaneHeader.vue

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -18,35 +18,36 @@ import { useLangs } from "@vp-composables/langs";
1818
const SIGN_IN_RE = /sign-in/i;
1919
const EXTERNAL_URL_RE = /^(?:[a-z]+:|\/\/)/i;
2020
21-
const { theme, frontmatter, isDark } = useData();
22-
const nav = computed(() => theme.value.nav ?? []);
21+
const { theme, frontmatter, isDark } = useData<DefaultTheme.Config>();
22+
const nav = computed<DefaultTheme.NavItem[]>(() => theme.value.nav ?? []);
23+
24+
/** VitePress allows `link` to be a function; the header only handles string links. */
25+
type LinkNavItem = DefaultTheme.NavItemWithLink & { link: string };
26+
/** Any level of the nav tree, including the groups nested inside a dropdown. */
27+
type AnyNavItem = DefaultTheme.NavItem | DefaultTheme.NavItemChildren;
28+
29+
const isLinkItem = (item: AnyNavItem): item is LinkNavItem =>
30+
"link" in item && typeof item.link === "string";
2331
2432
/**
2533
* Nav items flagged with `planeButton: "primary" | "secondary"` render as header
2634
* buttons (Sign in / cross-site link) instead of nav links. A `/sign-in` link is
2735
* treated as the primary button even without the flag.
2836
*/
29-
const isLinkItem = (item: DefaultTheme.NavItem): item is DefaultTheme.NavItemWithLink =>
30-
"link" in item && typeof item.link === "string";
31-
32-
const isPrimaryButtonItem = (item: DefaultTheme.NavItem) =>
37+
const isPrimaryButtonItem = (item: DefaultTheme.NavItem): item is LinkNavItem =>
3338
isLinkItem(item) && (item.planeButton === "primary" || SIGN_IN_RE.test(item.link));
3439
35-
const isSecondaryButtonItem = (item: DefaultTheme.NavItem) =>
40+
const isSecondaryButtonItem = (item: DefaultTheme.NavItem): item is LinkNavItem =>
3641
isLinkItem(item) && item.planeButton === "secondary";
3742
3843
const isNavButtonItem = (item: DefaultTheme.NavItem) =>
3944
isPrimaryButtonItem(item) || isSecondaryButtonItem(item);
4045
4146
const mainNav = computed(() => nav.value.filter((item) => !isNavButtonItem(item)));
4247
43-
const signInNavItem = computed(() =>
44-
nav.value.find((item): item is DefaultTheme.NavItemWithLink => isPrimaryButtonItem(item)),
45-
);
48+
const signInNavItem = computed(() => nav.value.find(isPrimaryButtonItem));
4649
47-
const secondaryNavItem = computed(() =>
48-
nav.value.find((item): item is DefaultTheme.NavItemWithLink => isSecondaryButtonItem(item)),
49-
);
50+
const secondaryNavItem = computed(() => nav.value.find(isSecondaryButtonItem));
5051
5152
const route = useRoute();
5253
const { localeLinks, currentLang } = useLangs({ correspondingLink: true });
@@ -68,7 +69,8 @@ const mobileMenuOpen = ref(false);
6869
const expandedAccordions = ref<Set<number>>(new Set());
6970
const languageMenuOpen = ref(false);
7071
71-
const isDropdown = (item: DefaultTheme.NavItem) => "items" in item && Array.isArray(item.items);
72+
const isDropdown = (item: AnyNavItem): item is DefaultTheme.NavItemWithChildren =>
73+
"items" in item && Array.isArray(item.items);
7274
const isExternalLink = (link: string) => EXTERNAL_URL_RE.test(link);
7375
7476
const toggleAccordion = (index: number) => {
@@ -86,23 +88,36 @@ const handleKeydown = (e: KeyboardEvent) => {
8688
}
8789
};
8890
91+
/**
92+
* `position: fixed` on <body> drops the document scroll to 0, so remember the
93+
* offset and restore it on unlock — otherwise opening the mobile menu part-way
94+
* down a page sends the reader back to the top when they close it.
95+
*/
96+
let lockedScrollY = 0;
97+
8998
const lockBodyScroll = () => {
9099
const scrollbarWidth = window.innerWidth - document.documentElement.clientWidth;
100+
lockedScrollY = window.scrollY;
91101
document.body.style.overflow = "hidden";
92102
document.body.style.position = "fixed";
93103
document.body.style.width = "100%";
94-
document.body.style.top = "0";
104+
document.body.style.top = `-${lockedScrollY}px`;
95105
if (scrollbarWidth > 0) {
96106
document.body.style.paddingRight = `${scrollbarWidth}px`;
97107
}
98108
};
99109
100110
const unlockBodyScroll = () => {
111+
const wasLocked = document.body.style.position === "fixed";
101112
document.body.style.overflow = "";
102113
document.body.style.position = "";
103114
document.body.style.width = "";
104115
document.body.style.top = "";
105116
document.body.style.paddingRight = "";
117+
if (wasLocked) {
118+
window.scrollTo(0, lockedScrollY);
119+
lockedScrollY = 0;
120+
}
106121
};
107122
108123
const toggleMobileMenu = () => {
@@ -385,7 +400,7 @@ onUnmounted(() => {
385400
>
386401
<nav class="flex-1 w-full pt-6 pb-8">
387402
<ul class="space-y-1">
388-
<li v-for="(navItem, index) in mainNav" :key="navItem.text">
403+
<li v-for="(navItem, index) in mainNav" :key="index">
389404
<template v-if="isDropdown(navItem)">
390405
<button
391406
type="button"
@@ -407,11 +422,8 @@ onUnmounted(() => {
407422
</svg>
408423
</button>
409424
<ul v-show="expandedAccordions.has(index)" class="pl-4 space-y-1">
410-
<template
411-
v-for="childItem in navItem.items"
412-
:key="childItem.link || childItem.text"
413-
>
414-
<li v-if="'link' in childItem">
425+
<template v-for="(childItem, childIndex) in navItem.items" :key="childIndex">
426+
<li v-if="isLinkItem(childItem)">
415427
<a
416428
:href="normalizeLink(childItem.link)"
417429
:target="isExternalLink(childItem.link) ? '_blank' : undefined"
@@ -429,7 +441,7 @@ onUnmounted(() => {
429441
{{ childItem.text }}
430442
</a>
431443
</li>
432-
<li v-else-if="'items' in childItem">
444+
<li v-else-if="isDropdown(childItem)">
433445
<p
434446
v-if="childItem.text"
435447
class="pt-3 pb-1 px-4 text-xs font-semibold uppercase tracking-wider text-grey/70 dark:text-white/50"
@@ -438,11 +450,11 @@ onUnmounted(() => {
438450
</p>
439451
<ul class="pl-4 space-y-1">
440452
<li
441-
v-for="nestedItem in childItem.items"
442-
:key="nestedItem.link || nestedItem.text"
453+
v-for="(nestedItem, nestedIndex) in childItem.items"
454+
:key="nestedIndex"
443455
>
444456
<a
445-
v-if="nestedItem.link"
457+
v-if="isLinkItem(nestedItem)"
446458
:href="normalizeLink(nestedItem.link)"
447459
:target="isExternalLink(nestedItem.link) ? '_blank' : undefined"
448460
:rel="isExternalLink(nestedItem.link) ? 'noreferrer' : undefined"
@@ -465,7 +477,7 @@ onUnmounted(() => {
465477
</ul>
466478
</template>
467479
<a
468-
v-else-if="navItem.link"
480+
v-else-if="isLinkItem(navItem)"
469481
:href="normalizeLink(navItem.link)"
470482
:target="isExternalLink(navItem.link) ? '_blank' : undefined"
471483
:rel="isExternalLink(navItem.link) ? 'noreferrer' : undefined"
@@ -480,8 +492,9 @@ onUnmounted(() => {
480492
>
481493
{{ navItem.text }}
482494
</a>
495+
<!-- `{ component }` nav items have no mobile representation; skip them. -->
483496
<span
484-
v-else
497+
v-else-if="'text' in navItem"
485498
class="block py-3 px-4 text-base font-sans text-primary dark:text-white"
486499
>
487500
{{ navItem.text }}

docs/.vitepress/theme/plane/index.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,16 @@ function handleTabHash() {
4848
document.querySelectorAll<HTMLElement>('[role="tab"]').forEach((button) => {
4949
const labelText = button.textContent?.trim().toLowerCase().replace(/\s+/g, "-");
5050
if (labelText === hash) {
51-
button.dispatchEvent(
52-
new MouseEvent("click", { view: window, bubbles: true, cancelable: true }),
53-
);
5451
button.click();
5552
button.focus();
5653
}
5754
});
5855
}
5956

57+
function handleHashChange() {
58+
nextTick(handleTabHash);
59+
}
60+
6061
function updateHashOnTabClick(event: Event) {
6162
const button = event.currentTarget as HTMLElement;
6263
const labelText = button.textContent?.trim().toLowerCase().replace(/\s+/g, "-");
@@ -169,7 +170,7 @@ export function createPlaneTheme(options: PlaneThemeOptions): Theme {
169170
syncHeaderTheme();
170171
}, 100);
171172

172-
window.addEventListener("hashchange", () => nextTick(handleTabHash));
173+
window.addEventListener("hashchange", handleHashChange);
173174

174175
htmlClassObserver = new MutationObserver(syncHeaderTheme);
175176
htmlClassObserver.observe(document.documentElement, {
@@ -179,6 +180,7 @@ export function createPlaneTheme(options: PlaneThemeOptions): Theme {
179180
});
180181

181182
onUnmounted(() => {
183+
window.removeEventListener("hashchange", handleHashChange);
182184
htmlClassObserver?.disconnect();
183185
zoom?.detach();
184186
});

docs/.vitepress/theme/plane/manifest.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"layout/top-banner.ts",
2828
"options.ts",
2929
"scripts/check-theme-sync.mjs",
30+
"scripts/check-vue-types.mjs",
3031
"types/shims.d.ts",
3132
"types/vitepress-augment.d.ts",
3233
"types/voidzero-theme.ts",

docs/.vitepress/theme/plane/scripts/check-theme-sync.mjs

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@
1010
* 2. https://raw.githubusercontent.com/<sibling>/<ref>/… — ref from THEME_SIBLING_REF,
1111
* then GITHUB_HEAD_REF (same-named PR branch), then master.
1212
*
13-
* Exit codes: 0 identical · 1 drift (or manifest out of date) · 2 sibling unreachable / bad args
13+
* While only one repo has adopted the shared theme the sibling has no `plane/` folder yet.
14+
* That is reported and skipped (exit 0) rather than failing, so unrelated PRs in the repo
15+
* that merged first are not blocked; the check becomes binding as soon as both sides have it.
16+
*
17+
* Exit codes: 0 identical (or sibling not adopted yet) · 1 drift (or manifest out of date)
18+
* · 2 sibling unreachable / bad args
1419
*/
1520
import { createHash } from "node:crypto";
1621
import { readdir, readFile } from "node:fs/promises";
@@ -44,32 +49,55 @@ async function readLocal(file) {
4449
return readFile(join(THEME_DIR, file));
4550
}
4651

52+
/** Present on every ref of both repos — tells "ref exists" apart from "ref has no plane/". */
53+
const ROOT_PROBE = "package.json";
54+
4755
async function makeSiblingReader() {
4856
const localPath = process.env.THEME_SIBLING_PATH;
4957
if (localPath) {
50-
const root = resolve(process.cwd(), localPath, THEME_REL);
51-
return {
52-
label: root,
53-
read: (file) => readFile(join(root, file)).catch(() => null),
54-
};
58+
const checkout = resolve(process.cwd(), localPath);
59+
const root = join(checkout, THEME_REL);
60+
const exists = (file) =>
61+
readFile(file).then(
62+
() => true,
63+
() => false,
64+
);
65+
if (await exists(join(root, "manifest.json"))) {
66+
return {
67+
status: "ok",
68+
label: root,
69+
read: (file) => readFile(join(root, file)).catch(() => null),
70+
};
71+
}
72+
return (await exists(join(checkout, ROOT_PROBE)))
73+
? { status: "not-adopted", label: checkout }
74+
: null;
5575
}
76+
5677
const refs = [process.env.THEME_SIBLING_REF, process.env.GITHUB_HEAD_REF, "master"].filter(
5778
Boolean,
5879
);
80+
let reachable = null;
5981
for (const ref of refs) {
60-
const base = `https://raw.githubusercontent.com/${siblingArg}/${ref}/${THEME_REL}/`;
82+
const root = `https://raw.githubusercontent.com/${siblingArg}/${ref}/`;
83+
const base = `${root}${THEME_REL}/`;
6184
const probe = await fetch(base + "manifest.json").catch(() => null);
6285
if (probe?.ok) {
6386
return {
87+
status: "ok",
6488
label: base,
6589
read: async (file) => {
6690
const res = await fetch(base + file).catch(() => null);
6791
return res?.ok ? Buffer.from(await res.arrayBuffer()) : null;
6892
},
6993
};
7094
}
95+
if (!reachable) {
96+
const rootProbe = await fetch(root + ROOT_PROBE).catch(() => null);
97+
if (rootProbe?.ok) reachable = `${siblingArg}@${ref}`;
98+
}
7199
}
72-
return null;
100+
return reachable ? { status: "not-adopted", label: reachable } : null;
73101
}
74102

75103
const manifest = JSON.parse(await readLocal("manifest.json"));
@@ -89,10 +117,19 @@ if (missingFromManifest.length || missingFromDisk.length) {
89117
const sibling = await makeSiblingReader();
90118
if (!sibling) {
91119
console.error(
92-
`Could not reach the sibling theme (${siblingArg}). Set THEME_SIBLING_PATH=../<repo> for a local checkout.`,
120+
`Could not reach the sibling repo (${siblingArg}). Set THEME_SIBLING_PATH=../<repo> for a local checkout.`,
93121
);
94122
process.exit(2);
95123
}
124+
125+
if (sibling.status === "not-adopted") {
126+
console.log(
127+
`${sibling.label} has no ${THEME_REL} yet — skipping the cross-repo comparison.\n` +
128+
`This is expected only until the companion PR lands; the check binds once both repos have the folder.`,
129+
);
130+
process.exit(failed ? 1 : 0);
131+
}
132+
96133
console.log(`Comparing ${THEME_REL} against ${sibling.label}`);
97134

98135
const siblingManifestRaw = await sibling.read("manifest.json");

0 commit comments

Comments
 (0)