Add Luma embed for Community Day events & facade iframes #38845
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Does this require us to set up an consent for European visitors now? ../Frenck |
|
Why would it require consent? It doesn't use cookies |
That was the question. If it doesn't I guess it is fine 👍 ../Frenck |
|
Sorry, I misread. My bad! It just waits for a scroll, touch or mouse move before loading any iframes in. |
📝 WalkthroughWalkthroughThe changes introduce a new lazy loading mechanism for iframes within articles by adding a script that sets the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant ArticleIframe
User->>Browser: Loads article page
Browser->>ArticleIframe: Iframes with data-src (no src set)
User-->>Browser: Triggers mousemove/touchmove/scroll (first time)
Browser->>ArticleIframe: Sets src = data-src, removes data-src
ArticleIframe->>Browser: Loads iframe content
sequenceDiagram
participant User
participant Browser
participant LumaCalendar
User->>Browser: Loads Community Day page
Browser->>LumaCalendar: Displays embedded interactive calendar iframe
User->>LumaCalendar: Interacts with calendar to view event details
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
source/_includes/javascripts/scripts.html (1)
14-17: Scope the contextmenu handler
Previous feedback pointed out the need to avoid hoisting top-level declarations into the global scope. Wrapping this listener logic in an IIFE or waiting forDOMContentLoadedwill prevent unintended globals and guarantee the element exists before querying.
🧹 Nitpick comments (4)
source/_includes/javascripts/scripts.html (2)
8-13: Defer DocSearch initialization to avoid race conditions
The inlinedocsearch({...})call can execute before the external DocSearch script has loaded. Consider addingdeferto the<script>tag that loads@docsearch/js@3or wrapping this block in aDOMContentLoaded/loadevent to ensure the library is available when you calldocsearch().
38-62: Enhance iframe lazy-loading robustness
querySelectorAllreturns an (empty) NodeList even if no matches exist, soif (iframes)is always truthy. Change toif (iframes.length > 0)to skip event listener registration when there are no lazy frames.- Inline scripts without
deferor DOM-ready guards can run before the DOM is parsed; wrap this block in aDOMContentLoadedor use a deferred script tag.- For a more semantic, view-based approach, you might explore the Intersection Observer API to load iframes when they scroll into view rather than on any user movement.
source/_posts/2025-04-24-community-day.markdown (2)
26-26: Clarify link branding
The link text “[Home Assistant Community Day]” points to Luma’s calendar. Consider updating it to “Luma Community Day calendar” (or similar) so users know they’re navigating to a third-party embed.
39-39: Revisit preposition for clarity
“We also have people who work full-time on Home Assistant attending these events” may read more naturally as “work full-time at Home Assistant.”🧰 Tools
🪛 LanguageTool
[uncategorized] ~39-~39: The preposition “as” seems more likely in this position.
Context: ... We also have people who work full-time on Home Assistant attending these events. ...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/_includes/javascripts/scripts.html(2 hunks)source/_posts/2025-04-24-community-day.markdown(1 hunks)
🧰 Additional context used
🪛 LanguageTool
source/_posts/2025-04-24-community-day.markdown
[uncategorized] ~39-~39: The preposition “as” seems more likely in this position.
Context: ... We also have people who work full-time on Home Assistant attending these events. ...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
| <iframe | ||
| data-src="https://lu.ma/embed/calendar/cal-QXZXEJrvB3UaI63/events?lt=light" | ||
| width="100%" | ||
| height="600" | ||
| frameborder="0" | ||
| style="border: 1px solid #bfcbda88; border-radius: 4px;" | ||
| allowfullscreen="" | ||
| aria-hidden="false" | ||
| tabindex="0" | ||
| ></iframe> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve iframe accessibility and markup hygiene
- Add a
titleattribute (e.g.title="Home Assistant Community Day Calendar") for screen readers. - Remove the deprecated
frameborder="0"—rely on CSS for styling (you already haveborder: 1px solid …). - As a progressive enhancement, you could include
loading="lazy"on the<iframe>; your JS will still swapdata-src, and browsers that support lazy loading will handle it natively.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
source/_includes/javascripts/scripts.html (4)
23-36: Enhance Scroll Listener Performance
Consider marking the scroll listener as passive to improve performance and prevent scroll jank:-window.addEventListener("scroll", () => changeNavColor()); +window.addEventListener("scroll", () => changeNavColor(), { passive: true });
41-44: Ensure DOM Readiness
The lazy-load script executes immediately, which may run before the DOM is fully parsed and lead toarticle.postbeingnull. Wrap the block in aDOMContentLoadedlistener or adddeferto the<script>tag to guarantee the element exists:-<script type="text/javascript"> - { +<script defer> +document.addEventListener('DOMContentLoaded', () => { + {
45-49: Optimize Lazy-Load Event Binding & Accessibility
Switch theiframescheck toiframes.length > 0to bind listeners only when there are iframes to load, and add akeydownlistener so keyboard-only users can trigger the load:- if (iframes) { + if (iframes.length > 0) { document.addEventListener('mousemove', loadLazyFrames, { passive: true, once: true }); document.addEventListener('touchmove', loadLazyFrames, { passive: true, once: true }); document.addEventListener('scroll', loadLazyFrames, { passive: true, once: true }); + document.addEventListener('keydown', loadLazyFrames, { passive: true, once: true });
51-62: Use Block-Scoped Function Expression
Block-level function declarations can be inconsistent across some browsers. Replacefunction loadLazyFrames()with a block-scoped arrow function:- function loadLazyFrames() { + const loadLazyFrames = () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/_includes/javascripts/scripts.html(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - home-assistant-docs
- GitHub Check: Header rules - home-assistant-docs
- GitHub Check: Pages changed - home-assistant-docs
🔇 Additional comments (1)
source/_includes/javascripts/scripts.html (1)
8-17: Review: DocSearch & Context Menu
The Algolia DocSearch setup and the contextmenu handler on.site-titleare implemented correctly. Ensure that.site-titleis present on every page to avoid a null-reference error when attaching the listener.
Proposed change
This PR adds add the Luma iframe to allow for registering for events within the blog.
This PR also adds the initial iframe facade logic to defer iframes from loading until user interaction for performance reasons. This does not include any breaking changes but requires an iframe to use
data-srcwhich will then be replaced withsrcupon interaction. This will eventually replacelite-youtubein a future PRBefore:
After:
Type of change
currentbranch).currentbranch).nextbranch).nextbranch).Additional information
Checklist
currentbranch.nextbranch.Summary by CodeRabbit
New Features
Improvements
Content Updates