Skip to content

Commit 29029a3

Browse files
committed
Fix timeouts and failing cases in Themepicker E2E test
1 parent 82f59b5 commit 29029a3

10 files changed

Lines changed: 96 additions & 75 deletions

File tree

‎.cache/pages.json‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,10 @@
2222
"contact",
2323
"offline",
2424
{
25-
"privacy": [
26-
"my-data"
27-
]
25+
"privacy": ["my-data"]
2826
},
2927
{
30-
"services": [
31-
"create-custom-font-sets",
32-
"overview"
33-
]
28+
"services": ["create-custom-font-sets", "overview"]
3429
},
3530
{
3631
"tags": [
@@ -45,4 +40,4 @@
4540
"typescript"
4641
]
4742
}
48-
]
43+
]

‎_TODO.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# TODO
22

3+
## Themepicker tooltips, extra themes
4+
5+
- Add additional themes
6+
- @TODO: add tooltip that makes use of the description field
7+
38
## Sentry feedback, chat bot tying into my phone and email
49

510
See note in src/components/scripts/sentry/client.ts - "User Feedback - allow users to report issues"

‎src/components/Forms/Contact/upload.astro‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<div style="padding: 2rem; text-align: center; color: var(--color-text-offset);">
1+
<div style="color: var(--color-text-offset); padding: 2rem; text-align: center;">
22
<svg
33
width="48"
44
height="48"

‎src/components/Navigation/client/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export class NavigationElement extends LitElement {
6565
this.setupFocusTrap()
6666
this.bindEvents()
6767
this.initialized = true
68+
this.setAttribute('data-nav-ready', 'true')
6869
}
6970

7071
private setupFocusTrap(): void {

‎src/components/ThemePicker/client/__tests__/index.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ vi.mock(
1515
type ThemePickerModule = WebComponentModule<ThemePickerElement>
1616

1717
type QueryRoot = ParentNode & {
18-
querySelector: (selectors: string) => Element | null
19-
querySelectorAll: (selectors: string) => NodeListOf<Element>
18+
querySelector: (_selectors: string) => Element | null
19+
querySelectorAll: (_selectors: string) => NodeListOf<Element>
2020
}
2121

2222
const queryElement = <T extends Element>(root: QueryRoot, selector: string): T => {

‎src/components/ThemePicker/client/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ export class ThemePickerElement extends LitElement {
9797
this.bindEvents()
9898
this.setViewTransitionHandlers()
9999
this.isInitialized = true
100+
this.setAttribute('data-theme-picker-ready', 'true')
100101

101102
// Initial update (Lit + StoreController will handle all reactivity automatically)
102103
this.requestUpdate()

‎src/components/scripts/store/themes.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,34 @@ import { handleScriptError } from '@components/scripts/errors/handler'
1919

2020
export type ThemeId = 'light' | 'dark' | 'holiday' | 'a11y'
2121

22+
const validThemes: ThemeId[] = ['light', 'dark', 'holiday', 'a11y']
23+
const fallbackTheme: ThemeId = 'light'
24+
25+
const getInitialThemePreference = (): ThemeId => {
26+
if (typeof window === 'undefined' || typeof document === 'undefined') {
27+
return fallbackTheme
28+
}
29+
30+
try {
31+
const stored = window.localStorage?.getItem('theme')
32+
if (stored && validThemes.includes(stored as ThemeId)) {
33+
return stored as ThemeId
34+
}
35+
} catch {
36+
// Ignore localStorage access issues (Safari private mode, etc.)
37+
}
38+
39+
try {
40+
if (typeof window.matchMedia === 'function' && window.matchMedia('(prefers-color-scheme: dark)').matches) {
41+
return 'dark'
42+
}
43+
} catch {
44+
// matchMedia unsupported - fall through to default
45+
}
46+
47+
return fallbackTheme
48+
}
49+
2250
// ============================================================================
2351
// STORES
2452
// ============================================================================
@@ -28,13 +56,12 @@ export type ThemeId = 'light' | 'dark' | 'holiday' | 'a11y'
2856
* Persisted to localStorage automatically via nanostores/persistent
2957
* Classified as 'necessary' - no consent required
3058
*/
31-
export const $theme = persistentAtom<ThemeId>('theme', 'light', {
32-
encode: (value) => value,
33-
decode: (value: string) => {
34-
// Handle both JSON-stringified and plain strings
35-
const validThemes: ThemeId[] = ['light', 'dark', 'holiday', 'a11y']
36-
return validThemes.includes(value as ThemeId) ? (value as ThemeId) : 'light'
37-
},
59+
export const $theme = persistentAtom<ThemeId>('theme', getInitialThemePreference(), {
60+
encode: (value) => value,
61+
decode: (value: string) => {
62+
// Handle both JSON-stringified and plain strings
63+
return validThemes.includes(value as ThemeId) ? (value as ThemeId) : fallbackTheme
64+
},
3865
})
3966

4067
/**

‎test/e2e/helpers/cookieHelper.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ export async function selectTheme(page: Page, themeId: string): Promise<void> {
116116
// Check if modal is already open (modal state now persists)
117117
const modal = page.locator('[data-theme-modal]')
118118
const toggleButton = getThemePickerToggle(page)
119+
await modal.waitFor({ state: 'attached' })
120+
await toggleButton.waitFor({ state: 'visible' })
119121
const isOpen = await modal.evaluate((el) => !el.hasAttribute('hidden'))
120122

121123
// Only click toggle if modal is not already open

‎test/e2e/specs/04-components/THEMEPICKER_TEST_NOTES.md‎

Lines changed: 0 additions & 40 deletions
This file was deleted.

‎test/e2e/specs/04-components/theme-picker.spec.ts‎

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,52 @@ import {
1313
test,
1414
} from '@test/e2e/helpers'
1515

16+
type PlaywrightPage = import('@playwright/test').Page
17+
18+
const THEME_PICKER_READY_ATTR = 'data-theme-picker-ready'
19+
const NAVIGATION_READY_ATTR = 'data-nav-ready'
20+
21+
const waitForThemePickerReady = async (page: PlaywrightPage) => {
22+
await page.waitForFunction((attr) => {
23+
const host = document.querySelector('theme-picker')
24+
return host?.getAttribute(attr) === 'true'
25+
}, THEME_PICKER_READY_ATTR)
26+
}
27+
28+
const waitForNavigationReady = async (page: PlaywrightPage) => {
29+
await page.waitForFunction((attr) => {
30+
const nav = document.querySelector('site-navigation')
31+
return nav?.getAttribute(attr) === 'true'
32+
}, NAVIGATION_READY_ATTR)
33+
}
34+
35+
const waitForHeaderComponents = async (page: PlaywrightPage) => {
36+
await Promise.all([waitForThemePickerReady(page), waitForNavigationReady(page)])
37+
}
38+
39+
const getDefaultNavigationHref = (basePage: BasePage) => basePage.navigationItems[0]?.url ?? '/about'
40+
1641
/**
1742
* Helper function to handle mobile navigation
1843
*/
19-
async function navigateWithMobileSupport(page: import('@playwright/test').Page, selector: string) {
44+
async function navigateWithMobileSupport(basePage: BasePage, href: string = getDefaultNavigationHref(basePage)) {
45+
const page = basePage.page
2046
const viewport = page.viewportSize()
2147
const isMobile = viewport && viewport.width < 768
2248
const navToggle = page.locator('button[aria-label="toggle menu"]')
2349
const header = page.locator('#header')
24-
const targetLink = page.locator(selector).first()
2550

2651
if (isMobile) {
2752
// Open mobile menu first
53+
await navToggle.waitFor({ state: 'visible' })
2854
await navToggle.click()
2955
await expect(navToggle).toHaveAttribute('aria-expanded', 'true')
3056
await expect(header).toHaveClass(/aria-expanded-true/)
31-
await targetLink.waitFor({ state: 'visible', timeout: 2000 })
3257
}
3358

34-
// Click the first navigation link
35-
await targetLink.click()
36-
37-
// Wait for navigation to complete
38-
await page.waitForLoadState('networkidle')
59+
// Trigger client-side navigation (Astro View Transitions)
60+
await basePage.navigateToPage(href)
61+
await basePage.waitForPageLoad()
3962

4063
// On mobile, the menu should automatically close after navigation
4164
// But let's ensure it's closed by checking and closing if needed
@@ -65,13 +88,10 @@ test.describe('Theme Picker Component', () => {
6588
* - Cookie modal blocking theme picker UI interactions
6689
* - Stale View Transitions state causing inconsistent behavior
6790
*/
68-
test.beforeEach(async ({ page: playwrightPage }, testInfo) => {
91+
test.beforeEach(async ({ page: playwrightPage }) => {
6992
const page = await BasePage.init(playwrightPage)
70-
// Skip setup for tests that need custom media emulation
71-
if (testInfo.title.includes('prefers-color-scheme')) {
72-
return
73-
}
7493
await setupCleanTestPage(page.page)
94+
await waitForHeaderComponents(page.page)
7595
})
7696

7797
test('@ready theme picker is visible', async ({ page: playwrightPage }) => {
@@ -124,6 +144,7 @@ test.describe('Theme Picker Component', () => {
124144
// Expected: Theme picker modal should show active state for current theme
125145
const themePicker = getThemePickerToggle(page.page)
126146
const modal = page.locator('[data-theme-modal]')
147+
const closeBtn = page.locator('.themepicker__closeBtn').first()
127148

128149
// Open the theme picker modal
129150
await themePicker.click()
@@ -137,11 +158,16 @@ test.describe('Theme Picker Component', () => {
137158
// Verify light theme is initially active
138159
await expect(lightParentLi).toHaveClass(/is-active/)
139160

140-
// Select dark theme (which closes the modal)
161+
// Select dark theme - modal stays open for additional actions
141162
const darkThemeButton = page.locator('button[data-theme="dark"]')
142163
await darkThemeButton.click()
143164
await expect(page.locator('html')).toHaveAttribute('data-theme', 'dark')
144165

166+
// Close the modal using the dedicated close button
167+
await closeBtn.click()
168+
await expect(themePicker).toHaveAttribute('aria-expanded', 'false')
169+
await expect(modal).not.toHaveClass(/is-open/)
170+
145171
// Reopen modal
146172
await themePicker.click()
147173
await expect(themePicker).toHaveAttribute('aria-expanded', 'true')
@@ -172,6 +198,7 @@ test.describe('Theme Picker Component', () => {
172198
test.beforeEach(async ({ page: playwrightPage }) => {
173199
const page = await BasePage.init(playwrightPage)
174200
await setupCleanTestPage(page.page)
201+
await waitForHeaderComponents(page.page)
175202
})
176203

177204
test('@ready theme preference persists across page reloads', async ({ page: playwrightPage }) => {
@@ -198,6 +225,7 @@ test.describe('Theme Picker Component', () => {
198225

199226
// This test needs its own setup without localStorage clearing
200227
await setupTestPage(page.page)
228+
await waitForHeaderComponents(page.page)
201229

202230
// Select dark theme using helper
203231
await selectTheme(page.page, 'dark')
@@ -294,6 +322,7 @@ test.describe('Theme Picker Component', () => {
294322
await setupTestPage(page.page, '/')
295323
await page.evaluate(() => localStorage.clear())
296324
await page.reload()
325+
await waitForHeaderComponents(page.page)
297326
})
298327

299328
test('theme picker button works after View Transition navigation', async ({ page: playwrightPage }) => {
@@ -316,7 +345,7 @@ test.describe('Theme Picker Component', () => {
316345
await expect(themeToggleBtn).toHaveAttribute('aria-expanded', 'false')
317346

318347
// 2. Navigate to another page using an internal link
319-
await navigateWithMobileSupport(page.page, '.main-nav-item a')
348+
await navigateWithMobileSupport(page)
320349

321350
// 3. Verify theme picker button still works after navigation
322351
const themeToggleBtnAfterNav = page.locator('.theme-toggle-btn').first()
@@ -334,7 +363,7 @@ test.describe('Theme Picker Component', () => {
334363
test('theme picker can select themes after View Transition navigation', async ({ page: playwrightPage }) => {
335364
const page = await BasePage.init(playwrightPage)
336365
// Navigate to another page first using mobile-aware helper
337-
await navigateWithMobileSupport(page.page, '.main-nav-item a')
366+
await navigateWithMobileSupport(page)
338367

339368
// 2. Open theme picker
340369
const themeToggleBtn = page.locator('.theme-toggle-btn').first()
@@ -366,7 +395,7 @@ test.describe('Theme Picker Component', () => {
366395
await expect(page.locator('html')).toHaveAttribute('data-theme', 'dark')
367396

368397
// 2. Navigate to another page using mobile-aware helper
369-
await navigateWithMobileSupport(page.page, '.main-nav-item a')
398+
await navigateWithMobileSupport(page)
370399

371400
// 3. Verify dark theme is still applied
372401
const htmlElement = page.locator('html')
@@ -400,6 +429,7 @@ test.describe('Theme Picker Component', () => {
400429
await setupTestPage(page.page, '/')
401430
await page.evaluate(() => localStorage.clear())
402431
await page.reload()
432+
await waitForHeaderComponents(page.page)
403433
})
404434

405435
test('preserves lang attribute across navigation', async ({ page: playwrightPage }) => {

0 commit comments

Comments
 (0)