-
Notifications
You must be signed in to change notification settings - Fork 1
fix: use base-dependent local storage key for banner state #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: use base-dependent local storage key for banner state #32
Conversation
✅ Deploy Preview for typst-docs-web ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the SiteNoticeBanner component to make the localStorage key dynamic based on the basePath configuration, replacing the hardcoded 'typst-jp-banner-hidden' key. This ensures that different deployments with different base paths maintain separate banner visibility states.
Key Changes
- Import
basePathfrom metadata to make the storage key configurable - Generate a dynamic storage key by replacing forward slashes in
basePathwith hyphens - Update the Alpine.js
x-dataattribute from a static string to a template literal to interpolate the storage key
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { InfoCircleIcon } from "../../icons"; | ||
|
|
||
| export const SiteNoticeBanner = () => { | ||
| const storageKey = `typst-docs-web${basePath.replace(/\//g, "-")}banner-hidden`; |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The storage key will have consecutive hyphens when basePath is /docs/ (e.g., typst-docs-web-docs--banner-hidden). Consider using .replace(/\//g, '-').replace(/-+/g, '-').replace(/^-|-$/g, '') to normalize the key and avoid leading/trailing/duplicate hyphens.
| const storageKey = `typst-docs-web${basePath.replace(/\//g, "-")}banner-hidden`; | |
| const storageKey = `typst-docs-web${basePath.replace(/\//g, "-").replace(/-+/g, "-").replace(/^-|-$/g, "")}-banner-hidden`; |
| export const SiteNoticeBanner = () => { | ||
| const storageKey = `typst-docs-web${basePath.replace(/\//g, "-")}banner-hidden`; |
Copilot
AI
Nov 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The hardcoded prefix typst-docs-web in the storage key should be extracted as a constant at the module level or imported from a configuration file to improve maintainability and consistency across the codebase.
| export const SiteNoticeBanner = () => { | |
| const storageKey = `typst-docs-web${basePath.replace(/\//g, "-")}banner-hidden`; | |
| const STORAGE_KEY_PREFIX = "typst-docs-web"; | |
| export const SiteNoticeBanner = () => { | |
| const storageKey = `${STORAGE_KEY_PREFIX}${basePath.replace(/\//g, "-")}banner-hidden`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good. I confirmed that the banner on each sub-website in the deploy preview is controlled independently.
close #19