Skip to content

feat: lazy-load dashboard chart modules with IntersectionObserver#702

Merged
pethers merged 15 commits intomainfrom
copilot/implement-lazy-loading-dashboard-charts
Mar 1, 2026
Merged

feat: lazy-load dashboard chart modules with IntersectionObserver#702
pethers merged 15 commits intomainfrom
copilot/implement-lazy-loading-dashboard-charts

Conversation

Copy link
Contributor

Copilot AI commented Mar 1, 2026

  • Create src/browser/lazy-loader.ts with IntersectionObserver-based lazy loading
  • Add .chart-skeleton CSS to styles.css
  • Update src/browser/main.ts to use lazy loading for all dashboards (except stats-loader which stays eager)
  • Create tests/lazy-loader.test.js with unit tests for the lazy loader
  • Fix fallback path in lazy-loader.ts: check DOM presence and apply skeleton class consistently
  • Add id="politician-dashboard" to politician-dashboard.html so containerId resolves correctly
  • Move initLazyDashboards() before await initStats() so IO does not delay observer setup
  • Extract loadDashboard() helper to remove repeated register-globals + init pattern
  • Fix lazy-loader.ts docstring: remove inaccurate SSR claim from fallback description
  • Remove CDN Chart.js <script> tag from politician-dashboard.html
  • Return IntersectionObserver | undefined from initLazyDashboards() to prevent GC
  • Wrap loader calls in Promise.resolve().then() for sync-throw safety
  • Store observer in module-level _lazyObserver in main.ts so GC cannot collect it (comment 2869074903)
  • Fix Cypress test expect(win.Chart).to.existcy.get('#politician-dashboard').scrollIntoView() + cy.window().its('Chart').should('exist') (CI failure fix)
  • All 20 lazy-loader Vitest tests pass; CodeQL: 0 alerts
Original prompt

This section details on the original issue you should resolve

<issue_title>Implement lazy loading for dashboard chart modules with IntersectionObserver</issue_title>
<issue_description>## 📋 Issue Type
Performance / Architecture

🎯 Objective

Implement lazy loading for dashboard chart modules to improve initial page load performance and reduce Time to Interactive (TTI).

📊 Current State

In vite.config.js, Chart.js, D3, and PapaParse are configured as manual chunks:

manualChunks: {
  'chart': ['chart.js', 'chartjs-plugin-annotation'],
  'd3': ['d3'],
  'papa': ['papaparse']
}

However, all dashboard JavaScript is loaded eagerly:

  • src/browser/main.ts imports all dashboard modules at startup
  • Dashboard pages load all visualization libraries even before user scrolls to charts
  • Chart.js (~200KB), D3 (~250KB), and PapaParse (~50KB) downloaded upfront

🚀 Desired State

  • Dashboard chart modules loaded only when they enter the viewport (IntersectionObserver)
  • Initial page load only includes critical above-the-fold content
  • Chart libraries lazy-loaded via dynamic import() on demand
  • Significant reduction in LCP and TTI metrics
  • Loading skeleton/placeholder shown while charts load

🔧 Implementation Approach

  1. Convert chart module imports to dynamic import() in dashboard initialization
  2. Use IntersectionObserver to trigger loading when chart container enters viewport
  3. Add loading skeleton CSS for chart placeholders:
.chart-skeleton {
  background: linear-gradient(90deg, var(--mid-bg) 25%, var(--dark-bg) 50%, var(--mid-bg) 75%);
  background-size: 200% 100%;
  animation: skeleton-loading 1.5s infinite;
  border-radius: 8px;
  min-height: 300px;
}
  1. Update src/browser/main.ts to lazy-init dashboards
  2. Verify Core Web Vitals improvement via Lighthouse CI

🤖 Recommended Agent

performance-engineer — Lazy loading optimization

✅ Acceptance Criteria

  • Chart modules loaded only on viewport intersection
  • Loading skeleton visible while charts initialize
  • LCP improvement of 500ms+ on dashboard pages
  • TTI improvement measurable in Lighthouse CI
  • No visual regression in chart rendering
  • Works across all 14 language dashboard variants

📚 References

  • Vite config: vite.config.js (manualChunks)
  • Main entry: src/browser/main.ts
  • Dashboard modules: src/browser/dashboards/
  • Lighthouse CI: .github/workflows/lighthouse-ci.yml
  • Budget: budget.json

🏷️ Labels

type:performance, priority:medium, component:dashboard</issue_description>

<agent_instructions>Implement lazy loading for dashboard chart modules using IntersectionObserver. Convert chart imports to dynamic import() in src/browser/main.ts. Add loading skeleton CSS. Verify with Lighthouse CI.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Add src/browser/lazy-loader.ts with IntersectionObserver-based lazy loading
- Update src/browser/main.ts: stats-loader stays eager; all 11 chart dashboards
  converted to dynamic import() triggered on viewport intersection
- Add .chart-skeleton CSS to styles.css using existing design tokens
- Add tests/lazy-loader.test.js (14 tests, all passing)

Reduces initial JS parse cost by deferring Chart.js (~200KB), D3 (~250KB),
and PapaParse (~50KB) until their dashboard sections scroll into view.

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement lazy loading for dashboard chart modules feat: lazy-load dashboard chart modules with IntersectionObserver Mar 1, 2026
@github-actions github-actions bot added html-css HTML/CSS changes javascript JavaScript code changes testing Test coverage size-l Large change (250-1000 lines) labels Mar 1, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Copy link
Contributor

Copilot AI left a 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 introduces viewport-triggered lazy loading for chart-heavy dashboard modules to reduce initial JS download/parse cost and improve TTI, while keeping above-the-fold “stats” eager.

Changes:

  • Added a new initLazyDashboards() helper that uses IntersectionObserver to dynamically import() dashboard modules as their containers near the viewport (with a no-IntersectionObserver fallback).
  • Refactored src/browser/main.ts to remove eager/static imports of Chart.js/D3/PapaParse-dependent dashboards and register them as lazy entries.
  • Added .chart-skeleton styling and new Vitest unit tests for the lazy loader behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/browser/lazy-loader.ts Adds IntersectionObserver-based lazy dashboard loader with skeleton lifecycle handling and fallback behavior.
src/browser/main.ts Switches chart-heavy dashboards from eager init to lazy dynamic imports; keeps stats eager.
styles.css Adds .chart-skeleton shimmer placeholder styling (with reduced-motion support).
tests/lazy-loader.test.js Adds unit tests covering IO-triggered loading, skeleton lifecycle, dedupe, skipping, and fallback.

Comment on lines 48 to 55
if (typeof IntersectionObserver === 'undefined') {
// Graceful fallback: load everything immediately
for (const { containerId, loader } of dashboards) {
loader().catch((err: unknown) =>
logger.error(`Lazy load failed for #${containerId}:`, err),
);
}
return;
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

initLazyDashboards() fallback path (when IntersectionObserver is unavailable) calls every dashboard loader() unconditionally. This bypasses the DOM presence check used in the observer path and can trigger dashboard init on pages where that container does not exist (e.g., different routes), causing avoidable errors. Suggestion: mirror the normal path by checking document.getElementById(containerId) before invoking loader() (and optionally apply/remove the skeleton class consistently).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 29043a3. The fallback path now mirrors the observer path: it calls document.getElementById(containerId) before invoking the loader (skipping and logging when the element is absent), and applies/removes the chart-skeleton class around the loader promise — matching the observer path's skeleton lifecycle.

Comment on lines 116 to 121
containerId: 'politician-dashboard',
loader: async () => {
await import('./shared/register-globals.js');
const { init } = await import('./dashboards/politician-dashboard.js');
await init();
},
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The lazy entry for the politician dashboard uses containerId: 'politician-dashboard', but there is no element with id="politician-dashboard" in politician-dashboard.html (the page uses id="main-content"). With this change the Politician Dashboard module will never be imported/initialized on that page. Update the containerId to an ID that actually exists on the politician dashboard page (or add the missing ID in the HTML as part of this PR).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 29043a3. Added id="politician-dashboard" to the <div class="dashboard-container"> element in politician-dashboard.html. Using id="main-content" wasn't viable since that ID exists on all 14 language home pages as well, which would have triggered the politician dashboard module on every page.

@pethers
Copy link
Member

pethers commented Mar 1, 2026

@copilot apply changes based on the comments in this thread

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

…containerId

- lazy-loader.ts: fallback path now checks getElementById before invoking
  loader (mirrors IO path), applies/removes chart-skeleton class consistently
- politician-dashboard.html: add id="politician-dashboard" to dashboard-container
  div so the IntersectionObserver can find it (page used id="main-content" on
  <main> which exists on all pages)
- tests/lazy-loader.test.js: add tests for fallback DOM-skip and skeleton
  lifecycle; add loader call assertion to skeleton test

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
@pethers pethers requested a review from Copilot March 1, 2026 11:21
@github-actions github-actions bot added the dashboard Interactive dashboards (Chart.js/D3.js) label Mar 1, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment on lines 134 to 143
// Eager: stats loader populates hero metrics — no chart libraries needed
try {
await initStats();
logger.debug('✓ stats initialized');
} catch (error) {
logger.error('✗ stats failed:', error);
}

const succeeded = results.filter((r) => r.status === 'fulfilled').length;
const failed = results.filter((r) => r.status === 'rejected').length;
const elapsed = (performance.now() - start).toFixed(0);
// Lazy: chart-heavy dashboards load only when their section enters the viewport
initLazyDashboards(LAZY_DASHBOARDS);
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

initLazyDashboards(LAZY_DASHBOARDS) is only registered after awaiting initStats(). Since initStats() does async I/O (CSV fetch/parse), this can delay setting up the IntersectionObserver and postpone (or miss) early lazy-load triggers for dashboards that are already in/near the viewport on initial render. Consider registering initLazyDashboards() before awaiting initStats() (or running them in parallel) so lazy dashboards can start prefetching as soon as DOMContentLoaded fires.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b166ec6. initLazyDashboards(LAZY_DASHBOARDS) is now called before await initStats(), so the IntersectionObserver is live before any async I/O can delay it.

Comment on lines 34 to 41
const LAZY_DASHBOARDS: LazyDashboard[] = [
{
containerId: 'coalition-status',
loader: async () => {
await import('./shared/register-globals.js');
const { init } = await import('./dashboards/coalition-loader.js');
await init();
},
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The LAZY_DASHBOARDS registry repeats the same await import('./shared/register-globals.js') + const { init } = await import(...) pattern in every loader. This duplication makes it easy for future edits to drift (e.g., forgetting to load globals for one dashboard). Consider extracting a small helper (e.g., a wrapper that loads globals once and then imports/runs init) to centralize the pattern.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b166ec6. Extracted a loadDashboard(moduleLoader) helper that handles the register-globals import and init() call. All 11 lazy entries now use it — the repeated 3-line pattern is gone.

@pethers
Copy link
Member

pethers commented Mar 1, 2026

@copilot apply changes based on the comments in this thread

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment on lines 70 to 107
// Map element → loader for O(1) lookup inside the observer callback
const pending = new Map<Element, () => Promise<void>>();

const observer = new IntersectionObserver((entries: IntersectionObserverEntry[]) => {
for (const entry of entries) {
if (!entry.isIntersecting) continue;

const el = entry.target as HTMLElement;
observer.unobserve(el);

const loaderFn = pending.get(el);
if (!loaderFn) continue;
pending.delete(el);

// Show skeleton while the module is downloading / initialising
el.classList.add(CHART_SKELETON_CLASS);

loaderFn()
.then(() => {
el.classList.remove(CHART_SKELETON_CLASS);
logger.debug(`✓ lazy loaded #${el.id}`);
})
.catch((err: unknown) => {
el.classList.remove(CHART_SKELETON_CLASS);
logger.error(`✗ lazy load failed #${el.id}:`, err);
});
}
}, options);

for (const { containerId, loader } of dashboards) {
const el = document.getElementById(containerId);
if (!el) {
logger.debug(`Lazy loader: #${containerId} not in DOM, skipping`);
continue;
}
pending.set(el, loader);
observer.observe(el);
}
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

initLazyDashboards() creates an IntersectionObserver but doesn’t keep any reference to it outside the function. After initLazyDashboards() returns, the observer can be garbage-collected in some browsers/conditions, which would stop observations and prevent dashboards from ever loading. Consider retaining the observer (e.g., module-level variable) and/or returning a handle (observer or cleanup function) so its lifetime is guaranteed and it can be disconnected if needed.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9341bd1. initLazyDashboards() now returns IntersectionObserver | undefined (undefined in the no-IO fallback path). The caller holds the reference, preventing GC. The docstring documents both the return type and when undefined is returned.

Comment on lines 84 to 95
// Show skeleton while the module is downloading / initialising
el.classList.add(CHART_SKELETON_CLASS);

loaderFn()
.then(() => {
el.classList.remove(CHART_SKELETON_CLASS);
logger.debug(`✓ lazy loaded #${el.id}`);
})
.catch((err: unknown) => {
el.classList.remove(CHART_SKELETON_CLASS);
logger.error(`✗ lazy load failed #${el.id}:`, err);
});
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

In both the IntersectionObserver path and the fallback path, loader()/loaderFn() is assumed to always return a Promise and never throw synchronously. If a loader throws before returning (e.g., due to a coding error), the skeleton class won’t be removed and the error won’t be logged by the .catch() handler. Wrapping the invocation in Promise.resolve() (or a try/catch that rethrows as a rejected Promise) would make the skeleton lifecycle and error logging robust to sync exceptions.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9341bd1. Both the IO path and fallback path now wrap loader invocations in Promise.resolve().then(() => loader()). A synchronous throw inside a loader becomes a rejected promise, which is caught by the existing .catch() handler — ensuring the skeleton class is always removed and the error is always logged.

@pethers
Copy link
Member

pethers commented Mar 1, 2026

@copilot apply changes based on the comments in this thread

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

- lazy-loader.ts: return IntersectionObserver | undefined from
  initLazyDashboards() so callers hold a reference preventing GC.
  Wrap both IO-path and fallback loader calls in Promise.resolve().then()
  so synchronous throws become rejections, guaranteeing the skeleton
  class is always removed and errors are always logged.
- tests/lazy-loader.test.js: add flushPromises() helper (setTimeout
  macro-task boundary) to fully drain the microtask queue regardless
  of promise chain depth. Update 5 existing skeleton-lifecycle tests
  that were checking synchronous loader dispatch (now async). Add 4
  new tests: observer return value (IO + fallback paths), sync-throw
  skeleton cleanup (IO + fallback paths).

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 1, 2026

@copilot apply changes based on the comments in this thread

Done in 9341bd1. Both issues from the review are addressed: observer GC prevention (return value) and sync-throw robustness (Promise.resolve().then()). Tests updated with a flushPromises() helper and 4 new tests covering the new behaviour — all 20 pass.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment on lines 99 to 103
// Register lazy dashboards immediately — IntersectionObserver must be live
// before initStats()'s async I/O so containers already in/near the viewport
// on initial render are not missed.
initLazyDashboards(LAZY_DASHBOARDS);

Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

initLazyDashboards() returns the created IntersectionObserver specifically so the caller can retain a reference (preventing GC). Here the return value is ignored, which can allow the observer to be garbage-collected and stop firing, leaving lazy dashboards never loading in some environments. Store the returned observer (even if unused) for the lifetime of the page, or keep it in module scope and optionally disconnect it on teardown/navigation.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 21e2600. Added a module-level _lazyObserver variable in main.ts; initLazyDashboards() return value is now assigned to it, keeping the IntersectionObserver alive for the page lifetime.

@pethers
Copy link
Member

pethers commented Mar 1, 2026

@copilot apply changes based on the comments in this thread and Test Dashboard / Dashboard Tests (cypress/e2e/dashboards.cy.js) (pull_request)
Test Dashboard / Dashboard Tests (cypress/e2e/dashboards.cy.js) (pull_request)Cancelled after 1m
Test Dashboard / Dashboard Tests (cypress/e2e/politician-dashboard.cy.js) (pull_request)
Test Dashboard / Dashboard Tests (cypress/e2e/politician-dashboard.cy.js) (pull_request)Failing after 1m

…ess test

- main.ts: add module-level `_lazyObserver` variable so the
  IntersectionObserver returned by initLazyDashboards() is retained for
  the page lifetime, preventing GC from stopping observations (comment
  2869074903).
- cypress/e2e/politician-dashboard.cy.js: replace one-shot
  `cy.window().then()` check (which could never see a lazily-loaded
  window.Chart) with `cy.get('#politician-dashboard').scrollIntoView()`
  + `cy.window().its('Chart').should('exist')`, which scrolls the
  container into the IntersectionObserver viewport trigger zone and then
  retries until register-globals sets window.Chart — fixing the CI
  failure in Test Dashboard / Dashboard Tests
  (cypress/e2e/politician-dashboard.cy.js).

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 1, 2026

@copilot apply changes based on the comments in this thread and [Test Dashboard / Dashboard Tests (cypress/e2e/dashbo...

Done in 21e2600. Two fixes applied:

  • Observer GC (main.ts): assigned initLazyDashboards() return value to module-level _lazyObserver so the observer lives for the page lifetime.
  • CI failure (politician-dashboard.cy.js): replaced one-shot cy.window().then((win) => expect(win.Chart).to.exist) with cy.get('#politician-dashboard').scrollIntoView() + cy.window().its('Chart').should('exist'). The scroll ensures the IO fires, and .its().should() retries until register-globals sets window.Chart.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@pethers pethers marked this pull request as ready for review March 1, 2026 14:24
@pethers pethers merged commit b0455cc into main Mar 1, 2026
28 checks passed
@pethers pethers deleted the copilot/implement-lazy-loading-dashboard-charts branch March 1, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dashboard Interactive dashboards (Chart.js/D3.js) html-css HTML/CSS changes javascript JavaScript code changes size-l Large change (250-1000 lines) testing Test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement lazy loading for dashboard chart modules with IntersectionObserver

3 participants