Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions docs/.vitepress/theme/banner.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
.jdx-banner {
position: relative;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Using position: relative for the banner while offsetting the fixed navigation bar via --vp-layout-top-height causes a visual gap at the top of the viewport when scrolling down. As the banner scrolls away, the navigation bar remains fixed at its offset position. Changing the banner to position: fixed ensures it stays at the top of the viewport along with the navigation bar, providing a consistent user experience.

Suggested change
position: relative;
position: fixed;
top: 0;
left: 0;
right: 0;

z-index: 60;
display: flex;
gap: 0.75rem;
align-items: center;
padding: 0.5rem 1rem;
background: var(--vp-c-brand-1, #3451b2);
color: #fff;
font-size: 0.9rem;
line-height: 1.4;
}
Comment thread
cursor[bot] marked this conversation as resolved.
.jdx-banner a {
color: #fff;
text-decoration: underline;
font-weight: 500;
}
.jdx-banner button {
margin-left: auto;
background: transparent;
border: 0;
color: #fff;
font-size: 1.25rem;
cursor: pointer;
line-height: 1;
padding: 0 0.25rem;
opacity: 0.85;
}
.jdx-banner button:hover {
opacity: 1;
}
@media (max-width: 640px) {
.jdx-banner {
font-size: 0.85rem;
padding: 0.4rem 0.75rem;
}
}
73 changes: 73 additions & 0 deletions docs/.vitepress/theme/banner.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import "./banner.css";

interface BannerData {
id: string;
enabled: boolean;
message: string;
link?: string;
linkText?: string;
}

const ENDPOINT = "https://jdx.dev/banner.json";
const STORAGE_KEY = "jdx-banner-dismissed";

export function initBanner(): void {
if (typeof window === "undefined") return;
Comment on lines +14 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The initBanner function should be idempotent to prevent multiple fetches or redundant banner injections if the function is called multiple times (e.g., during development with HMR or if enhanceApp is triggered again).

let initialized = false;
export function initBanner(): void {
  if (typeof window === "undefined" || initialized) return;
  initialized = true;

fetch(ENDPOINT, { cache: "no-cache" })
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
.then((r) => (r.ok ? (r.json() as Promise<BannerData>) : null))
.then((b) => {
if (!b || !b.enabled) return;
if (localStorage.getItem(STORAGE_KEY) === b.id) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Strict equality breaks dismissal for non-string banner IDs

Medium Severity

The BannerData.id is typed as string, but since the data comes from an external JSON endpoint with only a type assertion (as Promise<BannerData>), a numeric id (e.g., 1) won't be caught at runtime. localStorage.setItem coerces the value to "1", but the subsequent localStorage.getItem(STORAGE_KEY) === b.id performs strict equality between "1" and 1, which is always false. This makes the banner undismissable whenever the upstream JSON uses a numeric id.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 07edbff. Configure here.

render(b);
})
.catch(() => {});
}

function isHttpUrl(value: string): boolean {
try {
const u = new URL(value, window.location.href);
return u.protocol === "http:" || u.protocol === "https:";
} catch {
return false;
}
}

function render(b: BannerData): void {
const el = document.createElement("div");
el.className = "jdx-banner";
el.setAttribute("role", "region");
el.setAttribute("aria-label", "Site announcement");

const msg = document.createElement("span");
msg.textContent = b.message;
el.appendChild(msg);

if (b.link && isHttpUrl(b.link)) {
const a = document.createElement("a");
a.href = b.link;
a.target = "_blank";
a.rel = "noopener noreferrer";
a.textContent = b.linkText || "Learn more";
el.appendChild(a);
}

const btn = document.createElement("button");
btn.type = "button";
btn.setAttribute("aria-label", "Dismiss");
btn.textContent = "\u00d7";
btn.addEventListener("click", () => {
localStorage.setItem(STORAGE_KEY, b.id);
el.remove();
document.documentElement.style.removeProperty("--vp-layout-top-height");
});
el.appendChild(btn);

document.body.prepend(el);

requestAnimationFrame(() => {
document.documentElement.style.setProperty(
"--vp-layout-top-height",
`${el.offsetHeight}px`,
);
});
Comment on lines +58 to +72

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The banner's height can change if the window is resized (e.g., text wrapping on smaller screens). Using a ResizeObserver ensures the --vp-layout-top-height CSS variable stays in sync with the actual height of the banner. This prevents layout issues where the navigation bar might overlap the banner or leave a gap.

  const observer = new ResizeObserver(() => {
    document.documentElement.style.setProperty(
      "--vp-layout-top-height",
      `${el.offsetHeight}px`,
    );
  });

  btn.addEventListener("click", () => {
    localStorage.setItem(STORAGE_KEY, b.id);
    observer.disconnect();
    el.remove();
    document.documentElement.style.removeProperty("--vp-layout-top-height");
  });
  el.appendChild(btn);

  document.body.prepend(el);
  observer.observe(el);

}
2 changes: 2 additions & 0 deletions docs/.vitepress/theme/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Theme } from "vitepress";
import DefaultTheme from "vitepress/theme";
import { enhanceAppWithTabs } from "vitepress-plugin-tabs/client";
import { initBanner } from "./banner";
import "virtual:group-icons.css";
import "./custom.css";
import Layout from "./Layout.vue";
Expand All @@ -12,6 +13,7 @@ export default {
Layout,
enhanceApp({ app }) {
enhanceAppWithTabs(app);
initBanner();
Comment on lines 14 to +16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 initBanner() called in enhanceApp rather than onMounted

enhanceApp runs synchronously during Vue app bootstrap, before the component tree is mounted. All other imperative DOM work in this file (star count) is deferred to onMounted() for exactly this reason. While the async fetch means render() only runs after the DOM is ready in practice, the call site pattern is inconsistent and could break if an eager document.body access is ever added. Moving initBanner() inside the existing onMounted would align it with the established pattern.

Fix in Claude Code

},
setup() {
onMounted(() => {
Expand Down
Loading