fix(site): break long code snippets

[jog1t]

Changes

[jog1t]

• fix(site): break long code snippets #2603 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cloudflare-workers-and-pages]

Deploying rivet-studio with    Cloudflare Pages

Latest commit:	7df8b1e
Status:	🚫  Build failed.

View logs

[cloudflare-workers-and-pages]

Deploying rivet-hub with    Cloudflare Pages

Latest commit:	7df8b1e
Status:	🚫  Build failed.

View logs

[cloudflare-workers-and-pages]

Deploying rivet with    Cloudflare Pages

Latest commit:	7df8b1e
Status:	✅  Deploy successful!
Preview URL:	https://e2f8e24a.rivet.pages.dev
Branch Preview URL:	https://06-14-fix-site-break-long-co.rivet.pages.dev

View logs

[claude]

Claude finished @jog1t's task —— View job

---

Code Review Complete ✅

I've reviewed the changes in this PR focusing on code quality, best practices, potential issues, performance, security, and test coverage.

Todo List:

• Read and analyze both changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Evaluate test coverage needs
• Provide comprehensive feedback

Changes Summary:

1. frontend/packages/components/src/mdx/code.tsx: Added whitespace-pre-wrap break-all classes to <code> element
2. site/src/app/not-found.tsx: Replaced Header with FancyHeader and changed wrapper from <div> to <>

---

📋 Code Review Feedback

✅ Positive Aspects

• Addresses the core issue: The main change successfully addresses breaking long code snippets
• Consistent styling: Uses Tailwind CSS classes appropriately with existing codebase conventions
• Clean implementation: Simple, focused solution for the text wrapping problem
• No security concerns: Both changes are safe from a security perspective

⚠️ Issues & Concerns

1. CSS Class Choice (Medium Priority)

• File: frontend/packages/components/src/mdx/code.tsx:183
• Issue: break-all is quite aggressive and will break words in the middle, potentially making code less readable
• Recommendation: Consider break-words instead, which only breaks at word boundaries when necessary, or use overflow-wrap: anywhere for better control

// Current (aggressive breaking):
className="whitespace-pre-wrap break-all"

// Suggested (more conservative):
className="whitespace-pre-wrap break-words"

2. Scope Creep (High Priority)

• File: site/src/app/not-found.tsx
• Issue: The header component change (Header → FancyHeader) is unrelated to the PR's stated purpose
• Recommendation: This change should either be:
  • Moved to a separate PR with proper explanation
  • Documented in the PR description explaining why it was needed
  • Removed if not essential to the fix

3. Missing Context (Medium Priority)

• Issue: No explanation for why the not-found page needed header changes
• Recommendation: Add context to PR description or commit message

🧪 Test Coverage

• Missing: No tests for the text wrapping behavior
• Missing: No visual regression tests for the layout changes
• Recommendation: Consider adding tests that verify long code snippets wrap properly

🚀 Performance

• Impact: Minimal performance impact from both changes
• Note: Fragment vs div wrapper has negligible performance difference

🏗️ Best Practices

• Single Responsibility: PR mixes two unrelated changes
• Commit Messages: Could be more descriptive about the specific changes
• Documentation: Missing rationale for the header component change

💡 Suggestions

1. Split the PR: Consider moving the not-found page changes to a separate PR
2. CSS Alternative: Test break-words vs break-all for better readability
3. Add tests: Include tests for long code snippet wrapping behavior
4. Documentation: Add explanation for all changes in PR description

🎯 Overall Assessment

The core functionality (breaking long code snippets) is well-implemented and addresses the stated problem. However, the PR includes unrelated changes that should be addressed for better maintainability and review clarity.

Recommendation: ✅ Approve with minor changes - The main fix is solid, but consider the suggestions above for improved code quality.

---