Conversation
…mismatch The Flock contributors section was blank because contributors.html loaded @iframe-resizer/child@5.3.2 from CDN via a broken SRI hash (missing sha256- prefix), while the installed parent (@iframe-resizer/vue) is 5.5.7. Both issues caused the iframe resizer handshake to fail silently. Fix: - Add prebuild/dev cp step to copy node_modules/@iframe-resizer/child/index.umd.js to public/iframe-resizer-child.js (version-matched, no CDN dependency) - Update contributors.html to load /iframe-resizer-child.js instead of CDN URL - Add public/iframe-resizer-child.js to .gitignore (generated artifact) Fixes #530 Assisted-by: Claude Sonnet 4.6 via OpenCode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On screens <=768px all nav links were hidden with no way to access them. Add a hamburger toggle button and slide-down mobile menu that lists all nav links (left + right) stacked vertically. Menu closes on link click or outside click (onClickOutside from @vueuse/core). Desktop behavior is unchanged. The toggle is display:none above 768px. Navbar height is now auto on mobile to accommodate the open drawer. Fixes #540 Assisted-by: Claude Sonnet 4.6 via OpenCode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request implements a mobile-responsive navigation menu in TopNavbar.vue and transitions the iframe-resizer-child dependency from a CDN to a local build artifact. Feedback includes replacing the non-portable cp command in package.json with a cross-platform Node.js script, improving accessibility by using rem units instead of pt for font sizes, and ensuring the mobile dropdown menu is scrollable to prevent content from being cut off on small viewports.
| "dev": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vite", | ||
| "build": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vue-tsc && vite build", |
There was a problem hiding this comment.
The cp command is not cross-platform and will fail on Windows environments (unless using a shell like Git Bash). To ensure the project can be developed across different operating systems, consider using a portable Node.js script or a tool like shx for file operations.
| "dev": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vite", | |
| "build": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vue-tsc && vite build", | |
| "dev": "node -e \"require('fs').copyFileSync('node_modules/@iframe-resizer/child/index.umd.js', 'public/iframe-resizer-child.js')\" && vite", | |
| "build": "node -e \"require('fs').copyFileSync('node_modules/@iframe-resizer/child/index.umd.js', 'public/iframe-resizer-child.js')\" && vue-tsc && vite build", |
| .navbar__mobile-menu { | ||
| display: none; | ||
| flex-direction: column; | ||
| background-color: var(--ifm-navbar-background-color); | ||
| border-top: 1px solid rgba(255, 255, 255, 0.1); | ||
| padding: 0.5rem 1rem 1rem; | ||
| } |
There was a problem hiding this comment.
Since the navbar is fixed at the top, the mobile dropdown menu might exceed the viewport height on smaller screens or in landscape orientation, making some links inaccessible. Adding a maximum height and enabling vertical scrolling ensures all menu items remain reachable.
.navbar__mobile-menu {
display: none;
flex-direction: column;
background-color: var(--ifm-navbar-background-color);
border-top: 1px solid rgba(255, 255, 255, 0.1);
padding: 0.5rem 1rem 1rem;
max-height: calc(100vh - var(--ifm-navbar-height));
overflow-y: auto;
}
| .navbar__mobile-link { | ||
| color: var(--ifm-navbar-link-color); | ||
| text-decoration: none; | ||
| font-size: 14pt; |
There was a problem hiding this comment.
Using pt (points) for font sizes is generally discouraged in web development as it is a physical unit intended for print. It can lead to accessibility issues because it doesn't scale correctly with browser zoom or user font settings. It is recommended to use relative units like rem or em instead.
font-size: 1.125rem;
There was a problem hiding this comment.
Pull request overview
This PR addresses UI and embed-related issues by adding a mobile-friendly navigation experience and switching the contributors page to use a locally served iframe-resizer child script instead of a CDN.
Changes:
- Add a hamburger toggle + mobile dropdown menu to
TopNavbar.vue, including click-outside-to-close behavior. - Serve
@iframe-resizer/childfrompublic/iframe-resizer-child.jsand update build/dev scripts to copy it intopublic/. - Ignore the generated iframe-resizer bundle in
.gitignore.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/components/TopNavbar.vue |
Implements responsive navbar behavior (hamburger + mobile menu) and adjusts navbar sizing rules. |
public/contributors.html |
Switches iframe-resizer child script loading from CDN to a local asset. |
package.json |
Copies iframe-resizer child bundle into public/ as part of dev/build. |
.gitignore |
Ignores the generated public/iframe-resizer-child.js artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| class="navbar__toggle" | ||
| :aria-expanded="menuOpen" | ||
| aria-label="Toggle navigation menu" | ||
| @click="menuOpen = !menuOpen" | ||
| > |
There was a problem hiding this comment.
The hamburger toggle button should explicitly set type="button" to avoid the default type="submit" behavior if this navbar ever ends up rendered inside a <form> (or is reused in such a context).
| "dev": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vite", | ||
| "build": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vue-tsc && vite build", |
There was a problem hiding this comment.
The dev/build scripts rely on the shell cp command, which is not available in some environments (notably default Windows shells) and can make local development/CI brittle. Consider replacing this with a cross-platform approach (e.g., a small Node copy script or a cross-platform CLI like shx) and wiring it as a pre-step.
| "dev": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vite", | ||
| "build": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vue-tsc && vite build", |
There was a problem hiding this comment.
This change copies @iframe-resizer/child’s UMD bundle into public/ and serves it to end users. @iframe-resizer/child is marked as GPL-3.0 in the lockfile, so please double-check license compatibility and any required notices/attribution now that the code is being redistributed as a site asset.
| "dev": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vite", | |
| "build": "cp node_modules/@iframe-resizer/child/index.umd.js public/iframe-resizer-child.js && vue-tsc && vite build", | |
| "dev": "vite", | |
| "build": "vue-tsc && vite build", |
No description provided.