Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR aims to improve the mobile experience for the Nexus interactive diagrams embedded in the Nexus sync tutorial by adding touch-friendly interactions, responsive styling, and automatic iframe height resizing.
Changes:
- Added tap-to-select / tap-to-place interactions and mobile styling tweaks for the two matching games.
- Added responsive/mobile layout adjustments for the “nexus-decouple” interactive diagram.
- Implemented iframe auto-resize via
postMessagefrom embedded HTML pages and a corresponding MDX listener; adjusted embedded iframe heights (and added a mobile CSS rule that hides two iframes).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| static/html/nexus-quick-match.html | Adds mobile tap-to-place interaction, selection styling, and iframe height postMessage. |
| static/html/nexus-match-change.html | Replaces prior touch handlers with tap-to-select/tap-to-place logic; adds mobile styles and iframe height postMessage. |
| static/html/nexus-decouple.html | Adds a mobile responsive CSS block and iframe auto-resize logic (including polling). |
| docs/tutorials/nexus/sync-nexus-tutorial.md | Adds an MDX postMessage listener to auto-resize iframes; tweaks iframe heights and adds a mobile rule that hides two interactive iframes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handler = (e) => { | ||
| if (e.data?.type === 'iframeResize' && typeof e.data.height === 'number') { | ||
| document.querySelectorAll('iframe').forEach(iframe => { | ||
| try { | ||
| if (iframe.contentWindow === e.source) { | ||
| iframe.style.setProperty('height', e.data.height + 'px', 'important'); | ||
| } | ||
| } catch(err) {} | ||
| }); |
There was a problem hiding this comment.
The iframe resize message handler accepts postMessage events from any origin. Since these embedded iframes are served from the same origin (e.g. /html/...), add an e.origin === window.location.origin (or explicit allowed-origin) check before applying the height update to avoid other iframes/windows on the page being able to force layout changes via iframeResize messages.
| @media (max-width: 768px) { | ||
| iframe[title*="Match the Change"], | ||
| iframe[title*="Quick Match"] { display: none !important; } | ||
| } |
There was a problem hiding this comment.
This new mobile CSS rule hides the “Quick Match” and “Match the Change” iframes entirely for screens <= 768px. That prevents mobile users from accessing the interactive exercises, and it also contradicts the PR goal of fixing mobile issues. Consider removing this rule (or replacing it with a mobile-friendly layout/tap-support fallback message instead of display: none).
| function notifyHeight() { | ||
| var h = document.documentElement.scrollHeight; | ||
| if (window.parent && window.parent !== window) { | ||
| window.parent.postMessage({ type: 'iframeResize', height: h }, '*'); |
There was a problem hiding this comment.
postMessage(..., '*') sends resize events to any parent origin. Since this iframe is embedded same-origin in the docs site, set targetOrigin to window.location.origin (or another explicit allowed origin) to avoid leaking messages to an unexpected parent and to pair with stricter origin checks on the parent listener.
| window.parent.postMessage({ type: 'iframeResize', height: h }, '*'); | |
| window.parent.postMessage({ type: 'iframeResize', height: h }, window.location.origin); |
| function notifyHeight() { | ||
| var h = document.documentElement.scrollHeight; | ||
| if (window.parent && window.parent !== window) { | ||
| window.parent.postMessage({ type: 'iframeResize', height: h }, '*'); |
There was a problem hiding this comment.
postMessage(..., '*') sends resize events to any parent origin. Since this iframe is embedded same-origin in the docs site, set targetOrigin to window.location.origin (or another explicit allowed origin) to avoid leaking messages to an unexpected parent and to pair with stricter origin checks on the parent listener.
| window.parent.postMessage({ type: 'iframeResize', height: h }, '*'); | |
| window.parent.postMessage({ type: 'iframeResize', height: h }, window.location.origin); |
| var h = document.body.offsetHeight; | ||
| if (h !== lastH && window.parent && window.parent !== window) { | ||
| lastH = h; | ||
| window.parent.postMessage({ type: 'iframeResize', height: h }, '*'); |
There was a problem hiding this comment.
This auto-resize code posts messages with targetOrigin='*'. Since the parent page is expected to be same-origin, set an explicit targetOrigin (e.g. window.location.origin) to avoid sending messages to an unexpected embedding parent and to support origin validation on the receiver.
| window.parent.postMessage({ type: 'iframeResize', height: h }, '*'); | |
| window.parent.postMessage({ type: 'iframeResize', height: h }, window.location.origin); |
| setInterval(notifyHeight, 300); | ||
| if (typeof ResizeObserver !== 'undefined') { | ||
| new ResizeObserver(notifyHeight).observe(document.body); |
There was a problem hiding this comment.
setInterval(notifyHeight, 300) will wake up and post messages ~3x/sec even when nothing changes (and even though a ResizeObserver is also registered). Consider using the interval only as a fallback when ResizeObserver is unavailable, or throttling/debouncing notifyHeight to reduce unnecessary work and postMessage traffic (especially on mobile).
| setInterval(notifyHeight, 300); | |
| if (typeof ResizeObserver !== 'undefined') { | |
| new ResizeObserver(notifyHeight).observe(document.body); | |
| if (typeof ResizeObserver !== 'undefined') { | |
| new ResizeObserver(notifyHeight).observe(document.body); | |
| } else { | |
| setInterval(notifyHeight, 300); |
Fixed mobile issues of interactive diagrams.