Skip to content

Bugfix/issue 530 540#569

Open
castrojo wants to merge 2 commits intomainfrom
bugfix/issue-530-540
Open

Bugfix/issue 530 540#569
castrojo wants to merge 2 commits intomainfrom
bugfix/issue-530-540

Conversation

@castrojo
Copy link
Copy Markdown
Contributor

No description provided.

castrojo and others added 2 commits March 31, 2026 14:40
…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>
Copilot AI review requested due to automatic review settings March 31, 2026 18:43
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +7 to +8
"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",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
"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",

Comment on lines +292 to +298
.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;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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;

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/child from public/iframe-resizer-child.js and update build/dev scripts to copy it into public/.
  • 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.

Comment on lines +91 to +96
<button
class="navbar__toggle"
:aria-expanded="menuOpen"
aria-label="Toggle navigation menu"
@click="menuOpen = !menuOpen"
>
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +8
"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",
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +8
"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",
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
"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",

Copilot uses AI. Check for mistakes.
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