Infrastructure/implement e2e tests - #436
Conversation
Passing Tests (@ready): ✅ Largest Contentful Paint under 2.5s ✅ First Input Delay simulation under 100ms ✅ Cumulative Layout Shift under 0.1 ✅ Time to Interactive under 3.8s ✅ First Contentful Paint under 1.8s ✅ Total Blocking Time under 200ms ✅ Speed Index under 3.4s ✅ Page load time under 3s ✅ Images load efficiently (lazy loading + size checks) ✅ No excessive render-blocking resources
- Add // environment: happy-dom at top of test cases - Update bootstrap.ts change now that it throws in dev - The IntersectionObserver mock needs to be a spy - Fixes to Cookies state mocking
The carousel dots container has role="tablist" but contains <button> elements, which violates ARIA spec. A tablist role requires tab role children, not button elements. Current code: The dots container in the carousel uses role="tablist" with button children. Proposed fix: Change the carousel dots to use the proper ARIA pattern: Change the dots container from role="tablist" to role="group" Change each dot button to have role="tab" instead of being a plain button Update the aria-selected attribute instead of aria-current for the active tab Ensure proper keyboard navigation for tab pattern (arrow keys, not just Tab key)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency ReviewThe following issues were found:
|
| name: Code Quality Check | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "22.x" | ||
| cache: 'npm' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Run TypeScript check | ||
| run: npm run check | ||
|
|
||
| - name: Run linting | ||
| run: npm run lint No newline at end of file |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive end-to-end testing infrastructure for the application. The implementation adds a BasePage object model with reusable test helpers, removes deprecated content fetchers, updates theme management to use Web Components, and refactors CSS theme variables to use Tailwind v4 patterns.
Key Changes:
- Added BasePage class with 800+ lines of test utilities for e2e testing
- Migrated ThemePicker from LoadableScript to Web Components
- Updated theme CSS to use variable references and added shadow utilities
- Removed deprecated content-fetchers and refactored state management
Reviewed Changes
Copilot reviewed 187 out of 251 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/helpers/pageObjectModels/BasePage.ts | New BasePage class with comprehensive test utilities |
| test/e2e/helpers/cookieHelper.ts | Cookie modal handling utilities for tests |
| src/components/ThemePicker/theme-picker-element.ts | New Web Component implementation for theme picker |
| src/styles/themes.css | Refactored theme variables to use Tailwind v4 patterns |
| src/components/Scripts/state/store/themes.ts | New theme state management with localStorage |
| test/e2e/fixtures/test-data.ts | Updated newsletter confirmation message |
| } | ||
|
|
||
| return response | ||
| } /** |
There was a problem hiding this comment.
Missing blank line between the closing brace of goto() method and the JSDoc comment for dismissCookieModal(). Add a blank line for consistent code formatting and readability.
| } /** | |
| } | |
| /** |
| } catch { | ||
| // Ignore errors - modal might not exist on all pages | ||
| } | ||
| } /** |
There was a problem hiding this comment.
Missing blank line between the closing brace of dismissCookieModal() method and the JSDoc comment for evaluate(). Add a blank line for consistency.
| } /** | |
| } | |
| /** |
| } | ||
|
|
||
| /** | ||
| * Verify <Contact page form "email" input is present and visible |
There was a problem hiding this comment.
Corrected malformed JSDoc comment opening. Should be * Verify Contact page form not * Verify <Contact page form.
| * Verify <Contact page form "email" input is present and visible | |
| * Verify Contact page form "email" input is present and visible |
| e.preventDefault() | ||
| this.togglePicker() | ||
| } | ||
| }) // Close button |
There was a problem hiding this comment.
Missing blank line before the comment for Close button event listeners. Add a blank line to improve code organization and readability.
| }) // Close button | |
| }) | |
| // Close button |
|
|
||
| private getCacheKey(): string { | ||
| return `${EmbedInstance.CACHE_PREFIX}${this.platform}_${btoa(this.url).substring(0, 50)}` | ||
| return `${EmbedInstance.CACHE_PREFIX}${this.platform}_${Buffer.from(this.url, 'utf8').toString('base64').substring(0, 50)}` |
There was a problem hiding this comment.
Buffer is a Node.js API and is not available in browser environments. This will cause runtime errors. Use btoa() for browser-compatible base64 encoding instead.
| const storeTheme = $theme.get() | ||
| console.log('[Theme] Init after restore - dom:', domTheme, 'stored:', storedTheme, 'store:', storeTheme) |
There was a problem hiding this comment.
Multiple debug console.log statements left in production code. These should be removed or wrapped in a development-only check to avoid polluting production console output.
|
|
||
| if (storedTheme) { | ||
| // User has explicitly chosen a theme - apply it | ||
| console.log('[Theme] Applying stored preference:', storedTheme) |
There was a problem hiding this comment.
Multiple debug console.log statements left in production code. These should be removed or wrapped in a development-only check to avoid polluting production console output.
| } else if (domTheme && domTheme !== 'default' && domTheme !== storeTheme) { | ||
| // No stored preference, but HEAD script detected system preference | ||
| // Sync store to match DOM without persisting | ||
| console.log('[Theme] Syncing to system preference:', domTheme) |
There was a problem hiding this comment.
Multiple debug console.log statements left in production code. These should be removed or wrapped in a development-only check to avoid polluting production console output.
| applyThemeToDom(domTheme) | ||
| } else { | ||
| // DOM and store are in sync, just apply current state | ||
| console.log('[Theme] Applying current theme:', storeTheme) |
There was a problem hiding this comment.
Multiple debug console.log statements left in production code. These should be removed or wrapped in a development-only check to avoid polluting production console output.
|
|
||
| // Mark initialization complete - now listen for user theme changes | ||
| isInitialized = true | ||
| console.log('[Theme] Initialization complete') |
There was a problem hiding this comment.
Multiple debug console.log statements left in production code. These should be removed or wrapped in a development-only check to avoid polluting production console output.
| console.log('[Theme] Initialization complete') | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.log('[Theme] Initialization complete') | |
| } |
No description provided.