Copilot commented on code in PR #1372:
URL: https://github.com/apache/airflow-site/pull/1372#discussion_r2659064493
##########
landing-pages/site/assets/js/dark-mode.js:
##########
@@ -103,23 +103,15 @@
}
})
- const init = () => {
- showActiveTheme(getPreferredTheme())
-
- document.querySelectorAll('[data-bs-theme-value]')
- .forEach(toggle => {
- toggle.addEventListener('click', () => {
- const theme = toggle.getAttribute('data-bs-theme-value')
- setStoredTheme(theme)
- setTheme(theme)
- showActiveTheme(theme, true)
- })
+ showActiveTheme(getPreferredTheme())
+
+ document.querySelectorAll('[data-bs-theme-value]')
+ .forEach(toggle => {
+ toggle.addEventListener('click', () => {
+ const theme = toggle.getAttribute('data-bs-theme-value')
+ setStoredTheme(theme)
+ setTheme(theme)
+ showActiveTheme(theme, true)
})
- }
-
- if (document.readyState === 'loading') {
- window.addEventListener('DOMContentLoaded', init)
- } else {
- init()
- }
+ })
Review Comment:
Removing the DOM ready check causes the dark mode functionality to fail when
the script loads before DOM elements are available.
The code at line 106 calls `showActiveTheme()` which queries for
`.theme-switcher` elements (line 57), and lines 108-116 query for
`[data-bs-theme-value]` elements. If this script executes before these DOM
elements exist (e.g., if loaded in the `<head>`), these queries will return
empty NodeLists and the event listeners won't be attached, breaking the theme
toggle functionality.
The original code properly handled this by checking `document.readyState`
and waiting for `DOMContentLoaded` if needed. This safeguard should be restored
to ensure the script works reliably regardless of when it's loaded.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
