feat(FN-1534): fix theme-data.css URL resolution and link reuse
- Fix URL resolution in theme-data.css so it loads correctly from index.html - Reuse theme data link element across theme updates instead of recreating it - Update useTheme hook to properly manage theme link lifecycle - Add comprehensive tests for useTheme hook URL handling - Update theming documentation with URL resolution details
This commit is contained in:
@@ -474,12 +474,24 @@ To add a new color theme:
|
||||
|
||||
### Dynamic Stylesheet Loading
|
||||
|
||||
The `theme-data.css` file is loaded dynamically using `document.baseURI` for path resolution, which ensures correct behavior across all runtime contexts:
|
||||
The `theme-data.css` file is loaded dynamically to control stylesheet size and enable lazy loading. This file contains CSS custom properties for all 54 color themes and is only loaded when a non-default color theme is active.
|
||||
|
||||
- **HTTP/HTTPS serving** (development server, production web deployment): Uses standard root-relative path resolution (`/theme-data.css`)
|
||||
- **Electron file:// context** (desktop production): Derives the path relative to the HTML file's directory
|
||||
**Path Resolution:**
|
||||
|
||||
This approach avoids hardcoded absolute paths that would break in different deployment contexts. Both the pre-hydration inline script in `index.html` and the runtime hook (`useTheme.ts`) use the same path resolution strategy.
|
||||
The stylesheet URL is derived from `document.baseURI` for correct resolution across all runtime contexts:
|
||||
|
||||
- **HTTP/HTTPS serving** (development server, production web deployment): Derives the path relative to the HTML file's directory (e.g., `/app/theme-data.css`)
|
||||
- **Electron file:// context** (desktop production): Same directory-relative resolution for local files
|
||||
|
||||
The URL resolution handles two cases:
|
||||
1. Base URL ends with `/` (directory path): Replaces trailing `/` with `/theme-data.css`
|
||||
2. Base URL ends with filename: Replaces filename with `/theme-data.css`
|
||||
|
||||
**Stale Link Correction:**
|
||||
|
||||
When switching color themes or after navigation, the runtime hook (`useTheme.ts`) checks if an existing `theme-data.css` link has a stale `href` and updates it to the correct path. This ensures theme changes apply correctly even if the page was loaded with a different base URL.
|
||||
|
||||
Both the pre-hydration inline script in `index.html` and the runtime hook use the same path resolution strategy, preventing behavior drift between startup and runtime.
|
||||
|
||||
### Theme-Driven Logo and Task-Creation CTAs
|
||||
|
||||
|
||||
@@ -626,6 +626,89 @@ describe("useTheme", () => {
|
||||
existingLink.remove();
|
||||
});
|
||||
|
||||
it("updates stale theme-data link href when baseURI changes", () => {
|
||||
// Simulate the page loading with a different baseURI than current
|
||||
// This can happen if the inline script runs with one baseURI, then navigation occurs
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
value: "http://localhost:3000/",
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useTheme());
|
||||
|
||||
// First, inject a link with a stale/wrong href (simulating old baseURI)
|
||||
const staleLink = document.createElement("link");
|
||||
staleLink.id = "theme-data";
|
||||
staleLink.rel = "stylesheet";
|
||||
staleLink.href = "/theme-data.css"; // Wrong path from old base
|
||||
document.head.appendChild(staleLink);
|
||||
|
||||
// Now change baseURI to simulate navigation
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
value: "http://localhost:3000/some/nested/path/",
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
// Switch to non-default theme
|
||||
act(() => {
|
||||
result.current.setColorTheme("ocean");
|
||||
});
|
||||
|
||||
// The link should exist and href should be updated to the correct value
|
||||
const link = document.getElementById("theme-data") as HTMLLinkElement;
|
||||
expect(link).not.toBeNull();
|
||||
// href should be updated to resolve correctly for the new baseURI
|
||||
expect(link?.href).toBe("http://localhost:3000/some/nested/path/theme-data.css");
|
||||
|
||||
// Clean up
|
||||
link?.remove();
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
value: "http://localhost:3000/",
|
||||
configurable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("updates stale file:// link href when baseURI changes", () => {
|
||||
// Simulate Electron production path change
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
value: "file:///app/old/path/index.html",
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useTheme());
|
||||
|
||||
// Inject a link with a stale href (simulating wrong baseURI at load time)
|
||||
const staleLink = document.createElement("link");
|
||||
staleLink.id = "theme-data";
|
||||
staleLink.rel = "stylesheet";
|
||||
staleLink.href = "file:///wrong/path/theme-data.css";
|
||||
document.head.appendChild(staleLink);
|
||||
|
||||
// Now change baseURI to the correct production path
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
value: "file:///Users/me/Projects/kb/packages/dashboard/dist/client/index.html",
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
// Switch to non-default theme
|
||||
act(() => {
|
||||
result.current.setColorTheme("nord");
|
||||
});
|
||||
|
||||
// The link should exist and href should be updated
|
||||
const link = document.getElementById("theme-data") as HTMLLinkElement;
|
||||
expect(link).not.toBeNull();
|
||||
// href should be updated to resolve correctly for the new baseURI
|
||||
expect(link?.href).toBe("file:///Users/me/Projects/kb/packages/dashboard/dist/client/theme-data.css");
|
||||
|
||||
// Clean up
|
||||
link?.remove();
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
value: "http://localhost:3000/",
|
||||
configurable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves concrete path for deep nested file:// URL", () => {
|
||||
// Simulate a deeply nested Electron production path
|
||||
Object.defineProperty(document, "baseURI", {
|
||||
@@ -789,14 +872,15 @@ describe("getThemeInitScript", () => {
|
||||
expect(script).toContain("effectiveMode");
|
||||
});
|
||||
|
||||
it("index.html uses correct file:// URL replacement pattern", () => {
|
||||
it("index.html uses correct URL replacement pattern", () => {
|
||||
// Verify that the inline script in index.html uses the correct URL replacement
|
||||
// pattern (replace filename with theme-data.css) rather than buggy concatenation
|
||||
// pattern (handle both directory paths and filename paths) rather than buggy concatenation
|
||||
const indexHtml = readFileSync("app/index.html", "utf8");
|
||||
|
||||
// The correct pattern: base.replace(/\/[^\/]+$/, '/theme-data.css')
|
||||
// The correct pattern: check if base ends with '/' and use slice or replace accordingly
|
||||
// The buggy pattern: base.substring(0, 7) + dirPath + 'theme-data.css'
|
||||
expect(indexHtml).toContain("base.replace(/\\/[^\\/]+$/, '/theme-data.css')");
|
||||
expect(indexHtml).toContain("base.endsWith('/')");
|
||||
expect(indexHtml).toContain("base.slice(0, -1)");
|
||||
|
||||
// Ensure the buggy pattern is NOT present
|
||||
expect(indexHtml).not.toContain("base.substring(0, 7)");
|
||||
|
||||
@@ -16,7 +16,8 @@ const THEME_DATA_FILENAME = "theme-data.css";
|
||||
* of whether the app is served over HTTP or loaded from a file:// URL.
|
||||
*
|
||||
* For file:// URLs, the path is derived relative to the HTML file's directory.
|
||||
* For HTTP/HTTPS URLs, the path resolves to the server root.
|
||||
* For HTTP/HTTPS URLs, the path is derived relative to the HTML file's directory
|
||||
* (same as file://) to ensure correct resolution in nested deployments.
|
||||
*/
|
||||
function getThemeDataUrl(): string {
|
||||
// Get base URL from document.baseURI (most reliable across contexts)
|
||||
@@ -28,16 +29,17 @@ function getThemeDataUrl(): string {
|
||||
return `/${THEME_DATA_FILENAME}`;
|
||||
}
|
||||
|
||||
// Handle file:// URLs - derive path relative to HTML file directory
|
||||
// Replace the filename at the end of the path with theme-data.css
|
||||
// This produces correct paths like: file:///path/to/app/theme-data.css
|
||||
if (base.startsWith("file://")) {
|
||||
// Derive path relative to HTML file directory
|
||||
// Handle two cases:
|
||||
// 1. Base ends with "/" (directory path): replace trailing "/" with "/filename"
|
||||
// 2. Base ends with filename: replace filename with "/filename"
|
||||
if (base.endsWith("/")) {
|
||||
// Directory path: replace trailing "/" with "/theme-data.css"
|
||||
return base.slice(0, -1) + `/${THEME_DATA_FILENAME}`;
|
||||
} else {
|
||||
// Filename path: replace last segment with "/theme-data.css"
|
||||
return base.replace(/\/[^\/]+$/, `/${THEME_DATA_FILENAME}`);
|
||||
}
|
||||
|
||||
// For HTTP/HTTPS URLs, resolve relative to server root
|
||||
const url = new URL(`/${THEME_DATA_FILENAME}`, base);
|
||||
return url.href;
|
||||
}
|
||||
|
||||
// Check if we're in a browser environment
|
||||
@@ -126,15 +128,28 @@ function applyThemeAttributes(themeMode: ThemeMode, colorTheme: ColorTheme, syst
|
||||
|
||||
/**
|
||||
* Load theme-data.css for non-default themes.
|
||||
* Prevents duplicate injections by checking for existing link.
|
||||
* Safely handles existing links by checking href and updating if stale.
|
||||
* This ensures correct URL resolution when baseURI changes between renders.
|
||||
*/
|
||||
function loadThemeDataStylesheet(): void {
|
||||
if (!isBrowser) return;
|
||||
if (document.getElementById(THEME_DATA_ID)) return; // Already loaded
|
||||
|
||||
const expectedHref = getThemeDataUrl();
|
||||
const existingLink = document.getElementById(THEME_DATA_ID) as HTMLLinkElement | null;
|
||||
|
||||
if (existingLink) {
|
||||
// Link exists - update href if it differs from expected (handles baseURI changes)
|
||||
if (existingLink.href !== expectedHref) {
|
||||
existingLink.href = expectedHref;
|
||||
}
|
||||
// If href matches, link is already correct - nothing to do
|
||||
return;
|
||||
}
|
||||
|
||||
// No existing link - create one
|
||||
const link = document.createElement("link");
|
||||
link.rel = "stylesheet";
|
||||
link.href = getThemeDataUrl();
|
||||
link.href = expectedHref;
|
||||
link.id = THEME_DATA_ID;
|
||||
document.head.appendChild(link);
|
||||
}
|
||||
|
||||
@@ -28,22 +28,43 @@
|
||||
// Load theme-data.css for non-default themes to prevent flash
|
||||
// Use path-safe resolution that works in both HTTP and file:// contexts
|
||||
if (colorTheme !== 'default') {
|
||||
var themeDataUrl;
|
||||
var base = document.baseURI || (document.location && document.location.href) || '';
|
||||
// Derive path relative to HTML file directory
|
||||
// Handle two cases:
|
||||
// 1. Base ends with "/" (directory path): replace trailing "/" with "/filename"
|
||||
// 2. Base ends with filename: replace filename with "/filename"
|
||||
var themeDataUrl;
|
||||
if (base.indexOf('file://') === 0) {
|
||||
// For file:// URLs, derive path relative to the HTML file directory
|
||||
// Replace the filename at the end of the path with theme-data.css
|
||||
// This produces correct paths like: file:///path/to/app/theme-data.css
|
||||
themeDataUrl = base.replace(/\/[^\/]+$/, '/theme-data.css');
|
||||
// For file:// URLs
|
||||
if (base.endsWith('/')) {
|
||||
themeDataUrl = base.slice(0, -1) + '/theme-data.css';
|
||||
} else {
|
||||
themeDataUrl = base.replace(/\/[^\/]+$/, '/theme-data.css');
|
||||
}
|
||||
} else {
|
||||
// For HTTP/HTTPS URLs, use URL resolution
|
||||
themeDataUrl = new URL('/theme-data.css', base).href;
|
||||
// For HTTP/HTTPS URLs - same logic
|
||||
if (base.endsWith('/')) {
|
||||
themeDataUrl = base.slice(0, -1) + '/theme-data.css';
|
||||
} else {
|
||||
themeDataUrl = base.replace(/\/[^\/]+$/, '/theme-data.css');
|
||||
}
|
||||
}
|
||||
|
||||
// Check for existing link and update href if stale
|
||||
var existingLink = document.getElementById('theme-data');
|
||||
if (existingLink && existingLink.tagName === 'LINK') {
|
||||
// Update href if it differs (handles baseURI changes between loads)
|
||||
if (existingLink.href !== themeDataUrl) {
|
||||
existingLink.href = themeDataUrl;
|
||||
}
|
||||
} else {
|
||||
// No existing link - create one
|
||||
var link = document.createElement('link');
|
||||
link.rel = 'stylesheet';
|
||||
link.href = themeDataUrl;
|
||||
link.id = 'theme-data';
|
||||
document.head.appendChild(link);
|
||||
}
|
||||
var link = document.createElement('link');
|
||||
link.rel = 'stylesheet';
|
||||
link.href = themeDataUrl;
|
||||
link.id = 'theme-data';
|
||||
document.head.appendChild(link);
|
||||
}
|
||||
} catch (e) {
|
||||
document.documentElement.setAttribute('data-theme', 'dark');
|
||||
|
||||
Reference in New Issue
Block a user