Skip to content

Fix theme toggle broken by shared DOMContentLoaded scope#2

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-theme-toggle-functionality
Draft

Fix theme toggle broken by shared DOMContentLoaded scope#2
Copilot wants to merge 2 commits intomainfrom
copilot/fix-theme-toggle-functionality

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 19, 2026

A return inside the theme guard and shared function scope with the fetch chain meant any early exit in ride-loading could silently kill theme initialization.

Changes

  • assets/js/main.js — Extract loadRides() and initTheme() as independent top-level functions; DOMContentLoaded calls both unconditionally. The return in the missing-element guard now only exits initTheme().
document.addEventListener("DOMContentLoaded", function () {
  loadRides();   // fetch failure cannot affect theme
  initTheme();   // missing element only skips theme, nothing else
});
  • assets/css/styles.css — Extend body transition to include color so text smoothly follows background on theme switch.
/* before */
transition: background 0.3s ease;
/* after */
transition: background 0.3s ease, color 0.3s ease;
Original prompt

Problem

The dark/light theme toggle is not working — clicking the 🌙 button does not switch between dark and light mode.

Root Cause

In assets/js/main.js, the entire script is inside a single DOMContentLoaded callback. The theme system code (lines 43-75) runs after the fetch() Promise chain (lines 4-41). The problem is that if fetch("data/rides.json") fails (which it does when served as a static file:// page or if the JSON path is wrong), the .catch() block on line 35-41 runs, but since fetch is async, the theme code still runs synchronously.

However, there's a subtle issue: the return statement on line 49 (return; inside the if (!toggleBtn || !icon) guard) exits the entire DOMContentLoaded callback function. If for any reason the elements aren't found at that point, the theme system never initializes.

But the real fix needed is: The theme toggle code and the ride-loading code should be completely independent of each other. Currently they share the same function scope, and any uncaught error in the fetch chain or the rides rendering could prevent the theme system from initializing.

Required Fix

In assets/js/main.js, restructure the code so that:

  1. Wrap the ride-loading fetch in its own function (e.g., loadRides()) called from DOMContentLoaded
  2. Wrap the theme system in its own function (e.g., initTheme()) called from DOMContentLoaded
  3. Both functions should be called independently so a failure in one doesn't affect the other
  4. Change the return; in the theme guard to just skip the theme init (not exit the whole callback)

Here's the current file:

document.addEventListener("DOMContentLoaded", function () {

  /* ===== LOAD RIDES ===== */
  fetch("data/rides.json")
    .then(res => {
      if (!res.ok) {
        throw new Error("Failed to load rides.json");
      }
      return res.json();
    })
    .then(rides => {
      const container = document.getElementById("rides-grid");
      if (!container) return;

      rides.forEach(ride => {
        const card = document.createElement("div");
        card.className = "card";

        card.innerHTML = `
          <a href="rides/ride.html?id=${ride.id || ''}">
            ${ride.cover ? `<img src="${ride.cover}" loading="lazy" alt="${ride.title}">` : ''}
          </a>
          <div class="card-content">
            <h2>${ride.title}</h2>
            <div class="stats">
              ${ride.distance} km • ${ride.elevation} m<br>
              ${ride.date}
            </div>
          </div>
        `;

        container.appendChild(card);
      });
    })
    .catch(err => {
      console.error("Ride loading error:", err);
      const container = document.getElementById("rides-grid");
      if (container) {
        container.innerHTML = `<p class="error-message">Failed to load rides. Please try again later.</p>`;
      }
    });

  /* ===== THEME SYSTEM ===== */
  const toggleBtn = document.getElementById("themeToggle");
  const icon = document.getElementById("themeIcon");

  if (!toggleBtn || !icon) {
    console.warn("Theme toggle elements not found.");
    return;
  }

  const savedTheme = localStorage.getItem("theme");

  if (savedTheme === "dark") {
    document.documentElement.setAttribute("data-theme", "dark");
    icon.textContent = "☀️";
  } else {
    icon.textContent = "🌙";
  }

  toggleBtn.addEventListener("click", function () {
    const current = document.documentElement.getAttribute("data-theme");

    if (current === "dark") {
      document.documentElement.removeAttribute("data-theme");
      localStorage.setItem("theme", "light");
      icon.textContent = "🌙";
    } else {
      document.documentElement.setAttribute("data-theme", "dark");
      localStorage.setItem("theme", "dark");
      icon.textContent = "☀️";
    }
  });

});

The fixed version should look like this:

document.addEventListener("DOMContentLoaded", function () {
  loadRides();
  initTheme();
});

function loadRides() {
  fetch("data/rides.json")
    .then(res => {
      if (!res.ok) {
        throw new Error("Failed to load rides.json");
      }
      return res.json();
    })
    .then(rides => {
      const container = document.getElementById("rides-grid");
      if (!container) return;

      rides.forEach(ride => {
        const card = document.createElement("div");
        card.className = "card";

        card.innerHTML = `
          <a href="rides/ride.html?id=${ride.id || ''}">
            ${ride.cover ? `<img src="${ride.cover}" loading="lazy" alt="${ride.title}">` : ''}
          </a>
          <div class="card-content">
            <h2>${ride.title}</h2>
            <div class="stats">
              ${ride.distance} km • ${ride.elevation} m<br>
              ${ride.date}
            </div>
          </div>
        `;

        container.appendChild(card);
      });
    })
    .catch(err => {
      console.error("Ride loading error:", err);
      const containe...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 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](https://gh.io/copilot-coding-agent-tips) in the docs.

Co-authored-by: bacanijesse <66404384+bacanijesse@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix dark/light theme toggle functionality Fix theme toggle broken by shared DOMContentLoaded scope Feb 19, 2026
Copilot AI requested a review from bacanijesse February 19, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants