Skip to content

v2.2.4 - #10

Merged
byteful merged 14 commits into
mainfrom
dev
May 22, 2026
Merged

v2.2.4#10
byteful merged 14 commits into
mainfrom
dev

Conversation

@byteful

@byteful byteful commented May 21, 2026

Copy link
Copy Markdown
Member

Confidence Score: 3/5

I would fix the build and registry issues before merging.

  • The new SEO HTML generator is not connected to the production build.
  • CI overwrites the committed npm registry setting, so installs can diverge between environments.
  • The remaining issues are contained to SEO correctness and homepage performance.

package.json, .github/workflows/build.yml, and client/src/components/home/FeaturesSection.tsx need the most follow-up.

Important Files Changed

Filename Overview
client/src/components/home/FeaturesSection.tsx Large feature-card redesign with expandable media previews and eager media preloading.
scripts/postbuild-seo.mjs Generates route-specific static HTML, but it is not invoked by the build script.
client/src/components/SeoHead.tsx Updates route-level meta tags in the SPA while leaving JSON-LD route schema unchanged.
.github/workflows/build.yml Build workflow still rewrites npm registry settings to GitHub Packages.

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 5 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 5
package.json:6-11
**SEO script unused**

This PR adds `scripts/postbuild-seo.mjs` to generate `/register/index.html`, `/privacy/index.html`, and `/terms/index.html`, but the build still only runs `vite build`. A production build will not create those route-specific HTML files, so direct route requests and crawlers will keep getting the generic SPA HTML or a host-level fallback instead of the metadata this script is meant to produce.

```suggestion
  "scripts": {
    "dev": "vite",
    "build": "vite build && node scripts/postbuild-seo.mjs",
    "preview": "vite preview",
    "check": "tsc"
  },
```

### Issue 2 of 5
client/src/components/SeoHead.tsx:13-24
**Route schema stays stale**

`SeoHead` updates the document title and meta tags for each SPA route, but it leaves the `data-route-schema` JSON-LD block from `client/index.html` unchanged. After navigating to `/register`, `/privacy`, or `/terms`, the page can have the correct canonical/meta tags while structured data still describes the home page URL and description, producing inconsistent SEO metadata.

### Issue 3 of 5
.github/workflows/build.yml:28-34
**Registry config overwritten**

The committed `.npmrc` now points `@modl-gg` packages at the Nexus registry, but this workflow still deletes that file and rewrites it to GitHub Packages before `npm ci`. CI no longer tests the package registry configuration that developers and deployments get from the repository, and installs can fail or diverge if `@modl-gg/shared-web` is expected to come from Nexus.

### Issue 4 of 5
client/src/components/home/FeaturesSection.tsx:871-884
**Images preload immediately**

This hook creates an `Image` for every feature media asset as soon as the homepage mounts, including large GIFs and screenshots that are below the fold or only shown after expanding a card. That defeats the `loading="lazy"` behavior on the visible cards and can make the initial landing page download much more media than the visitor needs.

### Issue 5 of 5
client/src/components/home/FeaturesSection.tsx:662-665
**Wheel zoom drops updates**

The wheel handler calculates the next zoom from the `zoom` value captured by the current render. Trackpads and mouse wheels can fire several events before React renders again, so multiple events reuse the same stale value and some zoom steps are lost. The modal can feel stuck or jumpy when users scroll to zoom.

```suggestion
      onWheel={(event) => {
        event.preventDefault();
        setZoom((value) => {
          const constrained = Math.min(4, Math.max(1, value + (event.deltaY < 0 ? 0.2 : -0.2)));
          if (constrained === 1) setPosition({ x: 0, y: 0 });
          return constrained;
        });
      }}
```

Reviews (2): Last reviewed commit: "Merge branch 'main' into dev" | Re-trigger Greptile

Greptile also left 5 inline comments on this PR.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 22, 2026

Copy link
Copy Markdown

Deploying modl-landing with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4939eb7
Status: ✅  Deploy successful!
Preview URL: https://bdd9ea0b.modl-landing.pages.dev
Branch Preview URL: https://dev.modl-landing.pages.dev

View logs

@theobong
theobong marked this pull request as ready for review May 22, 2026 01:35
@theobong
theobong self-requested a review May 22, 2026 01:35

@theobong theobong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code is ass cheeks!!!

@byteful

byteful commented May 22, 2026

Copy link
Copy Markdown
Member Author

@greptileai lemme sniff ur bombastic crack!

Comment thread package.json
"license": "AGPL-3.0-only",
"scripts": {
"dev": "vite",
"build": "vite build",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 SEO generation skipped The new scripts/postbuild-seo.mjs writes route-specific HTML for /register, /privacy, and /terms, but the build command still only runs vite build. A production build will not generate those per-route files, so direct crawls and social unfurls for those routes keep the generic SPA HTML instead of the route metadata this PR adds.

Suggested change
"build": "vite build",
"build": "vite build && node scripts/postbuild-seo.mjs",
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 8

Comment:
**SEO generation skipped** The new `scripts/postbuild-seo.mjs` writes route-specific HTML for `/register`, `/privacy`, and `/terms`, but the build command still only runs `vite build`. A production build will not generate those per-route files, so direct crawls and social unfurls for those routes keep the generic SPA HTML instead of the route metadata this PR adds.

```suggestion
    "build": "vite build && node scripts/postbuild-seo.mjs",
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +13 to +25
useEffect(() => {
document.title = route.title;
setMeta('meta[name="description"]', "content", route.description);
setMeta('link[rel="canonical"]', "href", route.url);
setMeta('meta[property="og:site_name"]', "content", siteName);
setMeta('meta[property="og:title"]', "content", route.title);
setMeta('meta[property="og:description"]', "content", route.description);
setMeta('meta[property="og:image"]', "content", defaultImageUrl);
setMeta('meta[property="og:url"]', "content", route.url);
setMeta('meta[name="twitter:title"]', "content", route.title);
setMeta('meta[name="twitter:description"]', "content", route.description);
setMeta('meta[name="twitter:image"]', "content", defaultImageUrl);
}, [route]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Schema stays stale This effect updates the regular meta tags when the SPA route changes, but it leaves the script[data-route-schema] JSON-LD from index.html unchanged. Navigating to /register, /privacy, or /terms can show the right title and canonical while still exposing the home page WebPage schema, which gives crawlers inconsistent structured data.

Prompt To Fix With AI
This is a comment left during a code review.
Path: client/src/components/SeoHead.tsx
Line: 13-25

Comment:
**Schema stays stale** This effect updates the regular meta tags when the SPA route changes, but it leaves the `script[data-route-schema]` JSON-LD from `index.html` unchanged. Navigating to `/register`, `/privacy`, or `/terms` can show the right title and canonical while still exposing the home page `WebPage` schema, which gives crawlers inconsistent structured data.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +30 to +34
- name: Configure npm for GitHub Packages
run: |
rm .npmrc
echo "@modl-gg:registry=https://npm.pkg.github.com" >> .npmrc
echo "//npm.pkg.github.com/:_authToken=${{ secrets.GITHUB_TOKEN }}" >> .npmrc

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Registry override mismatch The repository .npmrc now points @modl-gg packages at the Nexus registry, and the lockfile resolves @modl-gg/shared-web from Nexus. This workflow deletes that config and rewrites the scope back to GitHub Packages before npm ci, so CI can resolve the private package from the wrong registry whenever the lockfile is refreshed or the package is no longer mirrored there.

Suggested change
- name: Configure npm for GitHub Packages
run: |
rm .npmrc
echo "@modl-gg:registry=https://npm.pkg.github.com" >> .npmrc
echo "//npm.pkg.github.com/:_authToken=${{ secrets.GITHUB_TOKEN }}" >> .npmrc
- name: Configure npm for Nexus
run: |
rm .npmrc
echo "@modl-gg:registry=https://nexus.modl.gg/repository/npm-releases/" >> .npmrc
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/build.yml
Line: 30-34

Comment:
**Registry override mismatch** The repository `.npmrc` now points `@modl-gg` packages at the Nexus registry, and the lockfile resolves `@modl-gg/shared-web` from Nexus. This workflow deletes that config and rewrites the scope back to GitHub Packages before `npm ci`, so CI can resolve the private package from the wrong registry whenever the lockfile is refreshed or the package is no longer mirrored there.

```suggestion
    - name: Configure npm for Nexus
      run: |
        rm .npmrc
        echo "@modl-gg:registry=https://nexus.modl.gg/repository/npm-releases/" >> .npmrc
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines 901 to 917
<motion.div
className="text-center mb-12"
initial={{ opacity: 0, y: 20 }}
whileInView={{ opacity: 1, y: 0 }}
viewport={{ once: true, margin: "-100px" }}
transition={{ duration: 0.5 }}
className="glass-bar rounded-2xl px-6 py-4 sm:px-8 sm:py-5 flex items-center gap-6"
initial={{ opacity: 0, y: -20 }}
animate={{ opacity: 1, y: 0 }}
transition={{ duration: 0.8, ease }}
>
<h2 className="text-3xl sm:text-4xl font-bold mb-4">Powerful Moderation Tools</h2>
<p className="text-xl text-slate-400 max-w-2xl mx-auto">
Everything you need to keep your Minecraft community safe, engaged, and supported
<h1 className="font-brand text-2xl sm:text-3xl tracking-tight shrink-0">
<span className="text-primary">modl</span>
<span className="text-foreground/70">.gg</span>
</h1>
<span className="w-px h-6 bg-white/10 shrink-0 hidden sm:block" />
<p className="text-sm text-muted-foreground/70 leading-relaxed">
The comprehensive moderation and support suite for Minecraft
servers: smart punishments, web replays, efficient ticketing, robust
analytics, and a full web dashboard.
</p>
</motion.div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Primary CTA removed Home now starts with this feature bar after deleting the old navbar and hero, but the replacement only renders brand text and a description. The previous above-the-fold Register Free and documentation links are gone, so new visitors have to scroll through the feature grid before finding the registration action in pricing.

Prompt To Fix With AI
This is a comment left during a code review.
Path: client/src/components/home/FeaturesSection.tsx
Line: 901-917

Comment:
**Primary CTA removed** `Home` now starts with this feature bar after deleting the old navbar and hero, but the replacement only renders brand text and a description. The previous above-the-fold `Register Free` and documentation links are gone, so new visitors have to scroll through the feature grid before finding the registration action in pricing.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Comment thread package.json
Comment on lines +6 to +11
"scripts": {
"dev": "vite",
"build": "vite build",
"preview": "vite preview",
"check": "tsc"
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 SEO script unused

This PR adds scripts/postbuild-seo.mjs to generate /register/index.html, /privacy/index.html, and /terms/index.html, but the build still only runs vite build. A production build will not create those route-specific HTML files, so direct route requests and crawlers will keep getting the generic SPA HTML or a host-level fallback instead of the metadata this script is meant to produce.

Suggested change
"scripts": {
"dev": "vite",
"build": "vite build",
"preview": "vite preview",
"check": "tsc"
},
"scripts": {
"dev": "vite",
"build": "vite build && node scripts/postbuild-seo.mjs",
"preview": "vite preview",
"check": "tsc"
},
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 6-11

Comment:
**SEO script unused**

This PR adds `scripts/postbuild-seo.mjs` to generate `/register/index.html`, `/privacy/index.html`, and `/terms/index.html`, but the build still only runs `vite build`. A production build will not create those route-specific HTML files, so direct route requests and crawlers will keep getting the generic SPA HTML or a host-level fallback instead of the metadata this script is meant to produce.

```suggestion
  "scripts": {
    "dev": "vite",
    "build": "vite build && node scripts/postbuild-seo.mjs",
    "preview": "vite preview",
    "check": "tsc"
  },
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +13 to +24
useEffect(() => {
document.title = route.title;
setMeta('meta[name="description"]', "content", route.description);
setMeta('link[rel="canonical"]', "href", route.url);
setMeta('meta[property="og:site_name"]', "content", siteName);
setMeta('meta[property="og:title"]', "content", route.title);
setMeta('meta[property="og:description"]', "content", route.description);
setMeta('meta[property="og:image"]', "content", defaultImageUrl);
setMeta('meta[property="og:url"]', "content", route.url);
setMeta('meta[name="twitter:title"]', "content", route.title);
setMeta('meta[name="twitter:description"]', "content", route.description);
setMeta('meta[name="twitter:image"]', "content", defaultImageUrl);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Route schema stays stale

SeoHead updates the document title and meta tags for each SPA route, but it leaves the data-route-schema JSON-LD block from client/index.html unchanged. After navigating to /register, /privacy, or /terms, the page can have the correct canonical/meta tags while structured data still describes the home page URL and description, producing inconsistent SEO metadata.

Prompt To Fix With AI
This is a comment left during a code review.
Path: client/src/components/SeoHead.tsx
Line: 13-24

Comment:
**Route schema stays stale**

`SeoHead` updates the document title and meta tags for each SPA route, but it leaves the `data-route-schema` JSON-LD block from `client/index.html` unchanged. After navigating to `/register`, `/privacy`, or `/terms`, the page can have the correct canonical/meta tags while structured data still describes the home page URL and description, producing inconsistent SEO metadata.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +28 to +34
scope: '@modl-gg'
- name: Configure npm for GitHub Packages
run: |
rm .npmrc
echo "@modl-gg:registry=https://npm.pkg.github.com" >> .npmrc
echo "//npm.pkg.github.com/:_authToken=${{ secrets.GITHUB_TOKEN }}" >> .npmrc

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Registry config overwritten

The committed .npmrc now points @modl-gg packages at the Nexus registry, but this workflow still deletes that file and rewrites it to GitHub Packages before npm ci. CI no longer tests the package registry configuration that developers and deployments get from the repository, and installs can fail or diverge if @modl-gg/shared-web is expected to come from Nexus.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/build.yml
Line: 28-34

Comment:
**Registry config overwritten**

The committed `.npmrc` now points `@modl-gg` packages at the Nexus registry, but this workflow still deletes that file and rewrites it to GitHub Packages before `npm ci`. CI no longer tests the package registry configuration that developers and deployments get from the repository, and installs can fail or diverge if `@modl-gg/shared-web` is expected to come from Nexus.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +871 to +884
// Preload all expanded-view images so they appear instantly on click
function usePreloadImages() {
useEffect(() => {
for (const f of features) {
if (f.media) {
const img = new Image();
img.src = f.media;
}
for (const src of f.extraMedia ?? []) {
const img = new Image();
img.src = src;
}
}
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Images preload immediately

This hook creates an Image for every feature media asset as soon as the homepage mounts, including large GIFs and screenshots that are below the fold or only shown after expanding a card. That defeats the loading="lazy" behavior on the visible cards and can make the initial landing page download much more media than the visitor needs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: client/src/components/home/FeaturesSection.tsx
Line: 871-884

Comment:
**Images preload immediately**

This hook creates an `Image` for every feature media asset as soon as the homepage mounts, including large GIFs and screenshots that are below the fold or only shown after expanding a card. That defeats the `loading="lazy"` behavior on the visible cards and can make the initial landing page download much more media than the visitor needs.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +662 to +665
onWheel={(event) => {
event.preventDefault();
setConstrainedZoom(zoom + (event.deltaY < 0 ? 0.2 : -0.2));
}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Wheel zoom drops updates

The wheel handler calculates the next zoom from the zoom value captured by the current render. Trackpads and mouse wheels can fire several events before React renders again, so multiple events reuse the same stale value and some zoom steps are lost. The modal can feel stuck or jumpy when users scroll to zoom.

Suggested change
onWheel={(event) => {
event.preventDefault();
setConstrainedZoom(zoom + (event.deltaY < 0 ? 0.2 : -0.2));
}}
onWheel={(event) => {
event.preventDefault();
setZoom((value) => {
const constrained = Math.min(4, Math.max(1, value + (event.deltaY < 0 ? 0.2 : -0.2)));
if (constrained === 1) setPosition({ x: 0, y: 0 });
return constrained;
});
}}
Prompt To Fix With AI
This is a comment left during a code review.
Path: client/src/components/home/FeaturesSection.tsx
Line: 662-665

Comment:
**Wheel zoom drops updates**

The wheel handler calculates the next zoom from the `zoom` value captured by the current render. Trackpads and mouse wheels can fire several events before React renders again, so multiple events reuse the same stale value and some zoom steps are lost. The modal can feel stuck or jumpy when users scroll to zoom.

```suggestion
      onWheel={(event) => {
        event.preventDefault();
        setZoom((value) => {
          const constrained = Math.min(4, Math.max(1, value + (event.deltaY < 0 ? 0.2 : -0.2)));
          if (constrained === 1) setPosition({ x: 0, y: 0 });
          return constrained;
        });
      }}
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

@byteful
byteful merged commit eb7ca5f into main May 22, 2026
3 checks passed
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